Skip to content

Conversation

@mehanana
Copy link

ℹ️ Issue

Closes #49

📝 Description

Added GET /users endpoint to retrieve users from the database. Optional pagination with page number and limit implemented.

Briefly list the changes made to the code:

  1. Added Get /users endpoint through lambda cli tool
  2. Created a local database for testing

✔️ Verification

Wrote Jest tests as well as checked different cases on Swagger to ensure it was working.
Screenshot 2025-10-23 233815
Screenshot 2025-10-23 233846

🏕️ (Optional) Future Work / Notes

Perhaps having user authentication for who can get users?

Copy link
Collaborator

@nourshoreibah nourshoreibah left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome work! Just a couple quick suggestions to make sure tests pass. also sorry for all the merge commits

@@ -1 +1 @@
DATABASE_URL=postgresql://branch_dev:password@localhost:5432/branch_db
DATABASE_URL=postgresql://branch_dev:password@localhost:5433/branch_db
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch!


const users = await db
.selectFrom('branch.users')
.selectAll()
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
.selectAll()
.selectAll()
.orderBy('user_id', 'asc')

Need this to make sure tests pass since it depends on users being ordered by id

expect(body.users.length).toBe(3);

const firstUser = body.users[0];
expect(firstUser.created_at).toBe("2025-10-20T21:19:52.978Z");
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
expect(firstUser.created_at).toBe("2025-10-20T21:19:52.978Z");

Created at is a little too brittle of a test since we all set up our DBs at different times

expect(body.users.length).toBe(1);

const firstUser = body.users[0];
expect(firstUser.created_at).toBe("2025-10-20T21:19:52.978Z");
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
expect(firstUser.created_at).toBe("2025-10-20T21:19:52.978Z");

expect(secondUser.user_id).toBe(2);

const thirdUser = body.users[2];
expect(thirdUser.created_at).toBe("2025-10-20T21:19:52.978Z");
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
expect(thirdUser.created_at).toBe("2025-10-20T21:19:52.978Z");

expect(firstUser.user_id).toBe(1);

const secondUser = body.users[1];
expect(secondUser.created_at).toBe("2025-10-20T21:19:52.978Z");
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
expect(secondUser.created_at).toBe("2025-10-20T21:19:52.978Z");

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Users] Add Get All Users Route (GET /users)

2 participants