Skip to content

Conversation

@mehanana
Copy link
Contributor

ℹ️ 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

nourshoreibah
nourshoreibah previously approved these changes Oct 30, 2025
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.

Great work!!

const normalizedPath = rawPath.replace(/\/$/, '');
const method = (event.requestContext?.http?.method || event.httpMethod || 'GET').toUpperCase();

console.log('DEBUG - rawPath:', rawPath, 'normalizedPath:', normalizedPath, 'method:', method);
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe remove if not needed anymore

@@ -0,0 +1,5 @@
module.exports = {
Copy link
Contributor

Choose a reason for hiding this comment

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

wait why was i able to run jest tests without this

Copy link
Contributor

Choose a reason for hiding this comment

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

i got an error when i tried to test and it worked when i deleted it

@denniwang
Copy link
Contributor

Business logic is all correct, I think my update test cases are screwing yours over.

Copy link
Contributor

@denniwang denniwang left a comment

Choose a reason for hiding this comment

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

i approve this pr

@mehanana mehanana merged commit f97bb60 into main Nov 6, 2025
4 checks passed
@mehanana mehanana deleted the 49-get-all-users-route branch November 6, 2025 23:26
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)

4 participants