From 33bff5a4b0883b0b8e97be1818f5c8bd348100e6 Mon Sep 17 00:00:00 2001 From: Luka Waymouth <155040393+lukawaay@users.noreply.github.com> Date: Tue, 26 Nov 2024 04:00:21 -0500 Subject: [PATCH] fix(console): bug fixes for ListProjectRoles and general pagination (#8938) # Which Problems Are Solved A number of small problems are fixed relating to the project roles listed in various places in the UI: - Fixes issue #8460 - Fixes an issue where the "Master checkbox" that's supposed to check and uncheck all list items breaks when there's multiple pages of results. Demonstration images are attached at the end of the PR. - Fixes an issue where the "Edit Role" dialog opened by clicking on a role in the list will not save any changes if the role's group is empty even though empty groups are allowed during creation. - Fixes issues where the list does not properly update after the user modifies or deletes some of its entries. - Fixes an issue for all paginated lists where the page number information (like "0-25" specifying that items 0 through 25 are shown on screen) was inaccurate, as described in #8460. # How the Problems Are Solved - Fixes buggy handling of pre-selected roles while editing a grant so that all selected roles are saved instead of only the ones on the current page. - Triggers the entire page to be reloaded when a user modifies or deletes a role to easily ensure the information on the screen is accurate. - Revises checkbox logic so that the "Master checkbox" will apply only to rows on the current page. I think this is the correct behavior but tell me if it should be changed. - Other fixes to faulty logic. # Additional Changes - I made clicking on a group name toggle all the rows in that group on the screen, instead of just turning them on. Tell me if this should be changed back to what it was before. # Additional Context - Closes #8460 ## An example of the broken checkboxes: ![2024-11-20_03-11-1732091377](https://github.com/user-attachments/assets/9f01f529-aac9-4669-92df-2abbe67e4983) ![2024-11-20_03-11-1732091365](https://github.com/user-attachments/assets/e7b8bed6-5cef-4c9f-9ecf-45ed41640dc6) ![2024-11-20_03-11-1732091357](https://github.com/user-attachments/assets/d404bc78-68fd-472d-b450-6578658f48ab) ![2024-11-20_03-11-1732091348](https://github.com/user-attachments/assets/a5976816-802b-4eab-bc61-58babc0b68f7) --------- Co-authored-by: Max Peintner --- .../paginator/paginator.component.html | 6 +- .../modules/paginator/paginator.component.ts | 15 ++++ .../project-role-detail-dialog.component.ts | 4 +- .../project-roles-table.component.html | 8 +- .../project-roles-table.component.ts | 83 ++++++++++--------- 5 files changed, 65 insertions(+), 51 deletions(-) diff --git a/console/src/app/modules/paginator/paginator.component.html b/console/src/app/modules/paginator/paginator.component.html index 913803843..e74a3e388 100644 --- a/console/src/app/modules/paginator/paginator.component.html +++ b/console/src/app/modules/paginator/paginator.component.html @@ -8,12 +8,10 @@

- {{ pageIndex * pageSize }} - {{ pageIndex * pageSize + pageSize }} - + {{ startIndex }} - {{ endIndex }}
- + {{ sizeOption }} diff --git a/console/src/app/modules/paginator/paginator.component.ts b/console/src/app/modules/paginator/paginator.component.ts index b4b406acd..4fd62fb56 100644 --- a/console/src/app/modules/paginator/paginator.component.ts +++ b/console/src/app/modules/paginator/paginator.component.ts @@ -50,6 +50,15 @@ export class PaginatorComponent { return temp <= this.length / this.pageSize; } + get startIndex(): number { + return this.pageIndex * this.pageSize; + } + + get endIndex(): number { + const max = this.startIndex + this.pageSize; + return this.length < max ? this.length : max; + } + public emitChange(): void { this.page.emit({ length: this.length, @@ -58,4 +67,10 @@ export class PaginatorComponent { pageSizeOptions: this.pageSizeOptions, }); } + + public updatePageSize(newSize: number): void { + this.pageSize = newSize; + this.pageIndex = 0; + this.emitChange(); + } } diff --git a/console/src/app/modules/project-role-detail-dialog/project-role-detail-dialog.component.ts b/console/src/app/modules/project-role-detail-dialog/project-role-detail-dialog.component.ts index 24c700277..f62ccc54a 100644 --- a/console/src/app/modules/project-role-detail-dialog/project-role-detail-dialog.component.ts +++ b/console/src/app/modules/project-role-detail-dialog/project-role-detail-dialog.component.ts @@ -31,9 +31,9 @@ export class ProjectRoleDetailDialogComponent { } submitForm(): void { - if (this.formGroup.valid && this.key?.value && this.group?.value && this.displayName?.value) { + if (this.formGroup.valid && this.key?.value && this.displayName?.value) { this.mgmtService - .updateProjectRole(this.projectId, this.key.value, this.displayName.value, this.group.value) + .updateProjectRole(this.projectId, this.key.value, this.displayName.value, this.group?.value) .then(() => { this.toast.showInfo('PROJECT.TOAST.ROLECHANGED', true); this.dialogRef.close(true); diff --git a/console/src/app/modules/project-roles-table/project-roles-table.component.html b/console/src/app/modules/project-roles-table/project-roles-table.component.html index 851e6848a..48ad2851b 100644 --- a/console/src/app/modules/project-roles-table/project-roles-table.component.html +++ b/console/src/app/modules/project-roles-table/project-roles-table.component.html @@ -35,8 +35,8 @@ [disabled]="disabled" color="primary" (change)="$event ? masterToggle() : null" - [checked]="selection.hasValue() && isAllSelected()" - [indeterminate]="selection.hasValue() && !isAllSelected()" + [checked]="isAnySelected() && isAllSelected()" + [indeterminate]="isAnySelected() && !isAllSelected()" >
@@ -76,7 +76,7 @@ class="role state" [ngClass]="{ 'no-selection': !selectionAllowed }" *ngIf="role.group" - (click)="selectionAllowed ? selectAllOfGroup(role.group) : openDetailDialog(role)" + (click)="selectionAllowed ? groupMasterToggle(role.group) : openDetailDialog(role)" [matTooltip]="selectionAllowed ? ('PROJECT.ROLE.SELECTGROUPTOOLTIP' | translate: role) : null" >{{ role.group }} @@ -135,7 +135,7 @@ #paginator [timestamp]="dataSource.viewTimestamp" [length]="dataSource.totalResult" - [pageSize]="50" + [pageSize]="INITIAL_PAGE_SIZE" (page)="changePage()" [pageSizeOptions]="[25, 50, 100, 250]" > diff --git a/console/src/app/modules/project-roles-table/project-roles-table.component.ts b/console/src/app/modules/project-roles-table/project-roles-table.component.ts index ae8633d15..215978241 100644 --- a/console/src/app/modules/project-roles-table/project-roles-table.component.ts +++ b/console/src/app/modules/project-roles-table/project-roles-table.component.ts @@ -18,6 +18,7 @@ import { ProjectRolesDataSource } from './project-roles-table-datasource'; styleUrls: ['./project-roles-table.component.scss'], }) export class ProjectRolesTableComponent implements OnInit { + public INITIAL_PAGE_SIZE: number = 50; @Input() public projectId: string = ''; @Input() public grantId: string = ''; @Input() public disabled: boolean = false; @@ -43,41 +44,58 @@ export class ProjectRolesTableComponent implements OnInit { } public ngOnInit(): void { - this.dataSource.loadRoles(this.projectId, this.grantId, 0, 25, 'asc'); - - this.dataSource.rolesSubject.subscribe((roles) => { - const selectedRoles: Role.AsObject[] = roles.filter((role) => this.selectedKeys.includes(role.key)); - this.selection.select(...selectedRoles.map((r) => r.key)); - }); + this.loadRolesPage(); + this.selection.select(...this.selectedKeys); this.selection.changed.subscribe(() => { this.changedSelection.emit(this.selection.selected); }); } - public selectAllOfGroup(group: string): void { - const groupRoles: Role.AsObject[] = this.dataSource.rolesSubject.getValue().filter((role) => role.group === group); - this.selection.select(...groupRoles.map((r) => r.key)); - } - private loadRolesPage(): void { - this.dataSource.loadRoles(this.projectId, this.grantId, this.paginator?.pageIndex ?? 0, this.paginator?.pageSize ?? 25); + this.dataSource.loadRoles( + this.projectId, + this.grantId, + this.paginator?.pageIndex ?? 0, + this.paginator?.pageSize ?? this.INITIAL_PAGE_SIZE, + ); } public changePage(): void { this.loadRolesPage(); } - public isAllSelected(): boolean { - const numSelected = this.selection.selected.length; - const numRows = this.dataSource.totalResult; - return numSelected === numRows; + private listIsAllSelected(list: string[]): boolean { + return list.findIndex((key) => !this.selection.isSelected(key)) == -1; + } + + private listIsAnySelected(list: string[]): boolean { + return list.findIndex((key) => this.selection.isSelected(key)) != -1; + } + + private listMasterToggle(list: string[]): void { + if (this.listIsAllSelected(list)) this.selection.deselect(...list); + else this.selection.select(...list); + } + + private compilePageKeys(): string[] { + return this.dataSource.rolesSubject.value.map((role) => role.key); } public masterToggle(): void { - this.isAllSelected() - ? this.selection.clear() - : this.dataSource.rolesSubject.value.forEach((row: Role.AsObject) => this.selection.select(row.key)); + this.listMasterToggle(this.compilePageKeys()); + } + + public isAllSelected(): boolean { + return this.listIsAllSelected(this.compilePageKeys()); + } + + public isAnySelected(): boolean { + return this.listIsAnySelected(this.compilePageKeys()); + } + + public groupMasterToggle(group: string): void { + this.listMasterToggle(this.dataSource.rolesSubject.value.filter((role) => role.group == group).map((role) => role.key)); } public deleteRole(role: Role.AsObject): void { @@ -93,45 +111,28 @@ export class ProjectRolesTableComponent implements OnInit { dialogRef.afterClosed().subscribe((resp) => { if (resp) { - const index = this.dataSource.rolesSubject.value.findIndex((iter) => iter.key === role.key); - this.mgmtService.removeProjectRole(this.projectId, role.key).then(() => { this.toast.showInfo('PROJECT.TOAST.ROLEREMOVED', true); - - if (index > -1) { - this.dataSource.rolesSubject.value.splice(index, 1); - this.dataSource.rolesSubject.next(this.dataSource.rolesSubject.value); - } + this.loadRolesPage(); }); } }); } - public removeRole(role: Role.AsObject, index: number): void { - this.mgmtService - .removeProjectRole(this.projectId, role.key) - .then(() => { - this.toast.showInfo('PROJECT.TOAST.ROLEREMOVED', true); - this.dataSource.rolesSubject.value.splice(index, 1); - this.dataSource.rolesSubject.next(this.dataSource.rolesSubject.value); - }) - .catch((error) => { - this.toast.showError(error); - }); - } - public openDetailDialog(role: Role.AsObject): void { - this.dialog.open(ProjectRoleDetailDialogComponent, { + const dialogRef = this.dialog.open(ProjectRoleDetailDialogComponent, { data: { role, projectId: this.projectId, }, width: '400px', }); + + dialogRef.afterClosed().subscribe(() => this.loadRolesPage()); } public refreshPage(): void { - this.dataSource.loadRoles(this.projectId, this.grantId, this.paginator?.pageIndex ?? 0, this.paginator?.pageSize ?? 25); + this.loadRolesPage(); } public get selectionAllowed(): boolean {