Modify Users' Access Types #357
Conversation
nik-dange
requested review from
sumeet-bansal,
shravanhariharan2 and
dowhep
as code owners
|
Thanks for contributing! If you've made changes to the API's functionality, please make sure to bump the package version—see this guide to semantic versioning for details—and document those changes as appropriate. |
|
|
| ] | ||
| }, standard); | ||
| }).rejects.toThrow(ForbiddenError); | ||
| }); |
There was a problem hiding this comment.
Let's also add another test making sure the role of the user is not updated
There was a problem hiding this comment.
The role of the current user making the request?
There was a problem hiding this comment.
the role of all users in the input of the changes requested
There was a problem hiding this comment.
the goal is that when an error is thrown, the request should not make changes to the database
There was a problem hiding this comment.
let me know if there is any questions regarding for what I requested
|
services/UserAccountService.ts
Outdated
| } | ||
| // Prevent anyone from demoting user with current admin status | ||
| if (currUser.accessType === UserAccessType.ADMIN && newAccess !== UserAccessType.ADMIN) { | ||
| throw new ForbiddenError('You cannot demote an admin.'); |
There was a problem hiding this comment.
I know there's been some use-case of this e.g. at end of school year where we want to remove ADMIN from users who no longer need admin perms. Do we plan on doing this manually? If so, should we also only grand ADMIN permissions manually as well?
| ] | ||
| }, standard); | ||
| }).rejects.toThrow(ForbiddenError); | ||
| }); |
There was a problem hiding this comment.
the role of all users in the input of the changes requested
| ] | ||
| }, standard); | ||
| }).rejects.toThrow(ForbiddenError); | ||
| }); |
There was a problem hiding this comment.
the goal is that when an error is thrown, the request should not make changes to the database
…into nikhil/admin-role-management merging main.
| ] | ||
| }, standard); | ||
| }).rejects.toThrow(ForbiddenError); | ||
| }); |
There was a problem hiding this comment.
let me know if there is any questions regarding for what I requested
services/UserAccountService.ts
Outdated
| @@ -234,7 +234,6 @@ export default class UserAccountService { | |||
|
|
|||
| const updatedUser = await userRepository.upsertUser(currUser, { accessType }); | |||
|
|
|||
| // log the activity of changing a user's access type | |||
There was a problem hiding this comment.
I will be honest this specific comment is fine as it explains a decent size of code
|
Also make sure to fix linting |
…into nikhil/admin-role-management merging from master
services/UserAccountService.ts
Outdated
| }); | ||
| } | ||
|
|
||
| public async getAllUsersWithAccessLevels(): Promise<PrivateProfile[]> { |
There was a problem hiding this comment.
Just double checking, are you thinking that we might have a type for just the user with accesslevels in the future? If that is not the case, we can just change the name of the function to getAllFullUserProfile()
…into nikhil/admin-role-management merging from master.
Closes #342.
Added
PATCHroute to update user access types.Finalized
GETroute to obtain all users and access types.