-
Notifications
You must be signed in to change notification settings - Fork 12
fix: stfc remove roles bug #1605
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
Changes from all commits
cb973cd
9a2a9f5
91bbcff
f8cca63
674b8f4
a6c9353
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -151,6 +151,12 @@ export default class PostgresUserDataSource implements UserDataSource { | |
| }); | ||
| } | ||
|
|
||
| async removeUserRoles(id: number): Promise<void> { | ||
| return database.transaction(async (trx) => { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We may be missing await |
||
| await trx<RoleUserRecord>('role_user').where('user_id', id).del(); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This may leave some one with out any assigned role do not we need to leave at least the user role
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This will only remove the roles assigned in the role)user table all UOWS assigned roles will still be there and we give everyone at least one role from UOWS right? |
||
| }); | ||
| } | ||
|
|
||
| async me(id: number): Promise<User | null> { | ||
| return database | ||
| .select() | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -415,7 +415,11 @@ export class StfcUserDataSource implements UserDataSource { | |
| (roleId) => !assignedRoleIds.has(roleId) && roleId !== userRole?.id | ||
| ); | ||
|
|
||
| if (newRolesToAssign.length > 0) { | ||
| if (newRolesToAssign.length == 0) { | ||
| this.removeUserRoles(id); | ||
| this.stfcRolesCache.remove(String(id)); | ||
| this.uopRolesCache.remove(String(id)); | ||
| } else if (newRolesToAssign.length > 0) { | ||
| await postgresUserDataSource.setUserRoles(id, newRolesToAssign); | ||
| this.stfcRolesCache.remove(String(id)); | ||
| this.uopRolesCache.remove(String(id)); | ||
|
|
@@ -522,6 +526,12 @@ export class StfcUserDataSource implements UserDataSource { | |
| throw new Error('Method not implemented.'); | ||
| } | ||
|
|
||
| async removeUserRoles(id: number): Promise<void> { | ||
| await postgresUserDataSource.removeUserRoles(id); | ||
|
|
||
| return; | ||
| } | ||
|
Comment on lines
+529
to
+533
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We may need to pass this to the super class or implement it. |
||
|
|
||
| async me(id: number) { | ||
| return this.getUser(id); | ||
| } | ||
|
|
||
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.
We are not mocking any thing here