-
Notifications
You must be signed in to change notification settings - Fork 4
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
nsc-events_3_39_update-user-dto-refactor #42
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@CourtneyHoppus Some checks are failing here. I also auto-updated your branch. The checks were failing before the update, too, FYI. Click the "details" link next to the failed check for more details. Some of them can be automatically fixed but I didn't go through and do that in case you wanted to check them out.
Thanks @brinkbrink. When referring to the automatic fix, is there a linter we are using that I can run on my branch or is it the third-party CodeFactor that wants permission to access GitHub? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks great, nice job! All the api routes work as expected once I have signed up and signed in and used the provided token.
The update route returns helpful messages when updating a user. The delete api route doesn't seem to work though "/users/delete/:id", but this route works "/users/remove/:id"
Also, in the 4th bullet point under To test, I think it'd be helpful to add the connection link documentation Tin created.
All endpoints are functional. It would be helpful to have a success message when removing a user. |
@Robel-003 I added that link and updated the testing directions. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great and it's working perfectly, thank you for making those changes! If you want to create issues for the comment Tin made, please feel free to do so. I'll go ahead and merge this into main
Resolves #39
I added a DTO for updating the user and refactored the controller and service to utilize this.
I removed the unused imports and the non-typed request parameters.
Removed redundant add User methods, the Auth module handles this.
When running
npm install
to be able to run the app with it's dependencies, it found a critical vulnerability:I resolved this by running
npm audit fix
Here is a document about connecting to a MongoDB Compass (local) instance. This is also the basic setup to connect to an Atlas (cloud) instance.
To test:
npm install
/auth/login
routeOR:
/auth/signup
route/users
route with GET call/users/find/:id
route with GET call/users/email/:email
route with GET call/users/update/:id
route with PATCH call/users/remove/:id
route with DELETE call