-
Notifications
You must be signed in to change notification settings - Fork 10
Role rework #638
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
base: development_3.0
Are you sure you want to change the base?
Role rework #638
Conversation
that check privileges
Untested so far
dynamically obtained from the serr
got user creation/editing to use configured roles
diagnostic logs
added a method to auth.service to allow checking a set of privileges (any that match yields a true response)
privs) are invisible
src/app/core/admin/user-management/user-edit-dialog/user-edit-dialog.component.ts
Outdated
Show resolved
Hide resolved
src/app/core/admin/user-management/user-edit-dialog/user-edit-dialog.component.ts
Outdated
Show resolved
Hide resolved
| console.log(' has priv to configure system'); | ||
| return true; | ||
| } else { | ||
| this.router.parseUrl('/home'); |
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.router.parseUrl('/home'); | |
| return this.router.parseUrl('/home'); |
| console.log('user CAN'); | ||
| return true; | ||
| } else { | ||
| this.router.parseUrl('/home'); |
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.router.parseUrl('/home'); | |
| return this.router.parseUrl('/home'); |
| <a class = "mat-icon-button" aria-label = "edit substance" mat-icon-button [routerLink]="['/substances', substance.uuid, 'edit']" *ngIf="isAdmin" | ||
| matTooltip='edit record'> | ||
| <a class = "mat-icon-button" aria-label = "edit substance" mat-icon-button [routerLink]="['/substances', substance.uuid, 'edit']" | ||
| *ngIf="canImportData" matTooltip='edit record'> |
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.
| *ngIf="canImportData" matTooltip='edit record'> | |
| *ngIf="canUserImportData" matTooltip='edit record'> |
| console.log(`in admin.component ngOnInit`); | ||
| await this.checkPrivileges(); | ||
|
|
||
| this.activatedRoute.params.subscribe(routeParams => { |
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.
Should we unsubscribe here?
src/app/core/auth/auth.service.ts
Outdated
| const privs = await firstValueFrom(this.fetchPrivs()); | ||
| return privs.some(p => p.privilege === 'Edit'); | ||
| } | ||
| return this._privileges != null && this._privileges.some(p=>p.privilege=="Edit"); |
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.
| return this._privileges != null && this._privileges.some(p=>p.privilege=="Edit"); | |
| return this._privileges != null && this._privileges.some(p=>p.privilege=="Edit"); | |
| const auth = this.authService.getAuth(); | ||
| if (auth) { | ||
| console.log(` got auth `); | ||
| const canImporNow = await this.authService.hasSpecificPrivilege("Import Data"); |
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.
| const canImporNow = await this.authService.hasSpecificPrivilege("Import Data"); | |
| const canImportNow = await this.authService.hasSpecificPrivilege("Import Data"); |
| response: any; | ||
| isError: boolean = false; | ||
| availableRoleNames: string[]; | ||
| assignableRoles: any[]; |
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.
type safety imporvement e.g.
interface AssignableRole {
roleName: string;
assigned: boolean;
}
assignableRoles: AssignableRole[];
Also, removed a duplicate from config.json
|
|
||
| default: this.activeTab = 0; break; | ||
| } | ||
| this.subscriptions.push(routeSub); |
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.
You're pushing subscription inside subscribe callback.That's wrong
| if( this.activeTab <= -1) { | ||
| this.router.navigate(['/home' ] ); | ||
| } | ||
| }); |
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.
it should be after callback here.
this.subscriptions.push(routeSub);
When you fix that you can simplify ngOnDestroy back to:
ngOnDestroy() {
this.subscriptions.forEach(sub => sub?.unsubscribe());
}
And test it.
Ready to merge now