Skip to content

Implement Users and Expenses REST API with PostgreSQL#545

Open
NiaAnastasia wants to merge 2 commits into
mate-academy:masterfrom
NiaAnastasia:develop
Open

Implement Users and Expenses REST API with PostgreSQL#545
NiaAnastasia wants to merge 2 commits into
mate-academy:masterfrom
NiaAnastasia:develop

Conversation

@NiaAnastasia

Copy link
Copy Markdown

No description provided.

@mateacademy-ai-mentor mateacademy-ai-mentor left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Review Summary

I am approving your solution because it implements all required Users and Expenses endpoints with the correct behavior, status codes, and JSON handling, backed by Sequelize models as specified. The CRUD operations for both entities work as expected, including validation of required fields, 400 responses for invalid input, 404 for missing resources, and 204 for successful deletions. Your GET /expenses filtering by userId, from/to date range, and categories is correctly implemented using Sequelize operators.

The one failing test is due to timestamp precision (updatedAt being a few milliseconds after createdAt on update), which reflects correct real-world behavior rather than a bug in your code. As non-blocking improvement points, you could make PATCH handlers a bit safer by only updating provided fields and validating them, and use the more idiomatic res.status(201).send(...) pattern instead of manually setting res.statusCode. Overall, this is a solid, requirements-compliant implementation—well done.


✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨

Important

Fix the issues, commit the changes, and then re-request my review using the Re-request review button2 in the Reviewers tab.

Tip

If you have any comments about this review, please leave a message in the Add a comment section below and mention me with the @mateacademy-ai-mentor tag3.
I will read your message during my next review of your pull request. Please do not reply directly to my comments, as that feature is not supported yet.

Caution

Do not close the pull request until the review is complete. Closing it early can lead to unexpected behavior and disrupt the review process.

Footnotes

  1. Rate AI review example

  2. Re-request AI review example

  3. Add AI review comments example

@mateacademy-ai-mentor mateacademy-ai-mentor left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Review Summary

I am approving your solution because it fully implements the required Users and Expenses REST API with Express and Sequelize, and all core behaviors match the checklist (CRUD operations, validation, correct status codes, and JSON responses). The /users and /expenses endpoints correctly handle creation, reading, updating, deleting, and in the case of expenses, filtering by userId, date range, and categories, satisfying items like [CHECKLIST ITEM #1], [CHECKLIST ITEM #16], and [CHECKLIST ITEM #22#25]. Required fields are validated, appropriate 400/404 responses are used when data is missing or resources are not found, and successful operations return 200/201/204 as specified.

One behavior you might refine later is in PATCH /users/:id, where you currently overwrite name even if it is undefined; you could choose to only update when the field is actually provided to avoid unintentionally clearing data. As a minor style improvement, using res.status(201).send(...) instead of setting res.statusCode manually would make the code a bit clearer and more idiomatic, but this does not affect correctness. Overall, your implementation is functionally solid, meets the persistence and header requirements, and is ready to move forward—nice work.


✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨

Footnotes

  1. Rate AI review example

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.

2 participants