Skip to content
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

Feature/shreyagupta112/user routes #45

Merged
merged 31 commits into from
Feb 23, 2022

Conversation

shreyagupta112
Copy link
Contributor

@shreyagupta112 shreyagupta112 commented Feb 18, 2022

Administrative Info

#38
Make sure your branch name conforms to: <feature/staging/hotfix/...>/[username]/[3-4 word description separated by dashes]. Otherwise, please rename your branch and create a new PR.

Changes

I created the auth_header.js file and modified the UserRoutes.js file to include new user routes. I created the auth_header.js file to used by a few user routes. I created routes to upgrade users to admin and verified by id, a route to get a user's profile by id, and a route to update user information. The route to upgrade a user to admin requires the logged-in user to be an admin and the route to update user information requires either the logged-in user to be an admin or to be the same user as the one who's information is being edited.

Testing

How did you confirm your changes worked?
I used Postman to run each of the user routes.

Confirmation of Change

I created the auth_header.js file. Everything that isn't the secure and admin routes has been changed.

[TODO Screenshot Title]
[TODO Screenshot Link]

shreyagupta112 and others added 22 commits January 13, 2022 19:42
User Schema and get route for admin and non-admin users
Get route which retrieves a list of all admin users or a list of all non-admin users.
Removing unused exports and fixing incorrect path name
Modified the get route for admin/ non-admin users to check if req.params.admin is a valid input
…sportJS-and-made-admin-get-route' of https://github.com/TritonSE/MAW-Volunteer-Hub into feature/shreyagupta112/created-user-model-integrted-PassportJS-and-made-admin-get-route
…odel-integrted-PassportJS-and-made-admin-get-route
…gupta112/created-user-model-integrted-PassportJS-and-made-admin-get-route
…gupta112/created-user-model-integrted-PassportJS-and-made-admin-get-route
…sportJS-and-made-admin-get-route' of https://github.com/TritonSE/MAW-Volunteer-Hub into feature/shreyagupta112/User-Routes
@shreyagupta112 shreyagupta112 requested a review from a team as a code owner February 18, 2022 15:48
Copy link
Contributor

@mohakvni mohakvni left a comment

Choose a reason for hiding this comment

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

Just to sanitize a bit, we can remove the "/secure" route in UserRoutes otherwise everything else, LGTM

@mohakvni mohakvni dismissed their stale review February 21, 2022 20:18

Nmv, was a small change, made it myself

@mohakvni mohakvni self-requested a review February 21, 2022 20:18
mohakvni
mohakvni previously approved these changes Feb 21, 2022
Copy link
Contributor

@mohakvni mohakvni left a comment

Choose a reason for hiding this comment

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

LGTM

@mohakvni mohakvni dismissed their stale review February 21, 2022 22:11

Found some errors on further testing

@mohakvni
Copy link
Contributor

So as of now I have found the following errors:
1. When we try to find user details of user whose Id does not exist in the database, server crashes
2. When we try to verify a non existent user (ID not in database), does not display appropriate error but also server does not crash
3. Server crashed when trying to promote admin of User whose ID not in database
4. Edit info automatically sets everything to null, if none of the information is passed in request body
5. Sever crashed when we try to edit information fo non existent ID

Copy link
Contributor

@mohakvni mohakvni left a comment

Choose a reason for hiding this comment

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

Tested All possible edge cases, work right about fine!
LGTM

@PatrickBrown1 PatrickBrown1 merged commit b6948d0 into development Feb 23, 2022
@delete-merged-branch delete-merged-branch bot deleted the feature/shreyagupta112/User-Routes branch February 23, 2022 18:56
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.

3 participants