[AC-2676] Remove paging logic from GroupsComponent (#9705)

* remove infinite scroll, use virtual scroll instead
* use TableDataSource for search
* allow sorting by name
* replacing PlatformUtilsService.showToast with ToastService
* misc FIXMEs
This commit is contained in:
Thomas Rittson 2024-07-04 06:04:16 +10:00 committed by GitHub
parent d7a0510ef9
commit 3e7f8f5384
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
6 changed files with 389 additions and 8 deletions

View File

@ -71,6 +71,11 @@ type GroupDetailsRow = {
collectionNames?: string[];
};
/**
* @deprecated To be replaced with NewGroupsComponent which significantly refactors this component.
* The GroupsComponentRefactor flag switches between the old and new components; this component will be removed when
* the feature flag is removed.
*/
@Component({
selector: "app-org-groups",
templateUrl: "groups.component.html",

View File

@ -0,0 +1,109 @@
<app-header>
<bit-search
[placeholder]="'searchGroups' | i18n"
[formControl]="searchControl"
class="tw-w-80"
></bit-search>
<button bitButton type="button" buttonType="primary" (click)="add()">
<i class="bwi bwi-plus bwi-fw" aria-hidden="true"></i>
{{ "newGroup" | i18n }}
</button>
</app-header>
<ng-container *ngIf="loading">
<i
class="bwi bwi-spinner bwi-spin text-muted"
title="{{ 'loading' | i18n }}"
aria-hidden="true"
></i>
<span class="sr-only">{{ "loading" | i18n }}</span>
</ng-container>
<ng-container *ngIf="!loading">
<p *ngIf="!dataSource.filteredData.length">{{ "noGroupsInList" | i18n }}</p>
<!-- The padding on the bottom of the cdk-virtual-scroll-viewport element is required to prevent table row content
from overflowing the <main> element. -->
<cdk-virtual-scroll-viewport scrollWindow [itemSize]="rowHeight" class="tw-pb-8">
<bit-table *ngIf="dataSource.filteredData.length" [dataSource]="dataSource">
<ng-container header>
<tr>
<th bitCell class="tw-w-20">
<input
type="checkbox"
bitCheckbox
class="tw-mr-2"
(change)="toggleAllVisible($event)"
id="selectAll"
/>
<label class="tw-mb-0 !tw-font-bold !tw-text-muted" for="selectAll">{{
"all" | i18n
}}</label>
</th>
<th bitCell bitSortable="name" default>{{ "name" | i18n }}</th>
<th bitCell>{{ "collections" | i18n }}</th>
<th bitCell class="tw-w-10">
<button
[bitMenuTriggerFor]="headerMenu"
type="button"
bitIconButton="bwi-ellipsis-v"
size="small"
appA11yTitle="{{ 'options' | i18n }}"
></button>
<bit-menu #headerMenu>
<button type="button" bitMenuItem (click)="deleteAllSelected()">
<span class="tw-text-danger"
><i aria-hidden="true" class="bwi bwi-trash"></i> {{ "delete" | i18n }}</span
>
</button>
</bit-menu>
</th>
</tr>
</ng-container>
<ng-template body let-rows$>
<tr bitRow *cdkVirtualFor="let g of rows$" [ngClass]="rowHeightClass">
<td bitCell (click)="check(g)" class="tw-cursor-pointer">
<input type="checkbox" bitCheckbox [(ngModel)]="g.checked" />
</td>
<td bitCell class="tw-cursor-pointer tw-font-bold" (click)="edit(g)">
<button type="button" bitLink>
{{ g.details.name }}
</button>
</td>
<td bitCell (click)="edit(g, ModalTabType.Collections)" class="tw-cursor-pointer">
<bit-badge-list
[items]="g.collectionNames"
[maxItems]="3"
variant="secondary"
></bit-badge-list>
</td>
<td bitCell>
<button
[bitMenuTriggerFor]="rowMenu"
type="button"
bitIconButton="bwi-ellipsis-v"
size="small"
appA11yTitle="{{ 'options' | i18n }}"
></button>
<bit-menu #rowMenu>
<button type="button" bitMenuItem (click)="edit(g)">
<i aria-hidden="true" class="bwi bwi-pencil-square"></i> {{ "editInfo" | i18n }}
</button>
<button type="button" bitMenuItem (click)="edit(g, ModalTabType.Members)">
<i aria-hidden="true" class="bwi bwi-user"></i> {{ "members" | i18n }}
</button>
<button type="button" bitMenuItem (click)="edit(g, ModalTabType.Collections)">
<i aria-hidden="true" class="bwi bwi-collection"></i> {{ "collections" | i18n }}
</button>
<button type="button" bitMenuItem (click)="delete(g)">
<span class="tw-text-danger"
><i aria-hidden="true" class="bwi bwi-trash"></i> {{ "delete" | i18n }}</span
>
</button>
</bit-menu>
</td>
</tr>
</ng-template>
</bit-table>
</cdk-virtual-scroll-viewport>
</ng-container>

View File

@ -0,0 +1,255 @@
import { Component } from "@angular/core";
import { takeUntilDestroyed } from "@angular/core/rxjs-interop";
import { FormControl } from "@angular/forms";
import { ActivatedRoute } from "@angular/router";
import {
BehaviorSubject,
combineLatest,
concatMap,
from,
lastValueFrom,
map,
switchMap,
tap,
} from "rxjs";
import { debounceTime, first } from "rxjs/operators";
import { ApiService } from "@bitwarden/common/abstractions/api.service";
import { ListResponse } from "@bitwarden/common/models/response/list.response";
import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service";
import { LogService } from "@bitwarden/common/platform/abstractions/log.service";
import { CollectionService } from "@bitwarden/common/vault/abstractions/collection.service";
import { CollectionData } from "@bitwarden/common/vault/models/data/collection.data";
import { Collection } from "@bitwarden/common/vault/models/domain/collection";
import {
CollectionDetailsResponse,
CollectionResponse,
} from "@bitwarden/common/vault/models/response/collection.response";
import { CollectionView } from "@bitwarden/common/vault/models/view/collection.view";
import { DialogService, TableDataSource, ToastService } from "@bitwarden/components";
import { InternalGroupService as GroupService, GroupView } from "../core";
import {
GroupAddEditDialogResultType,
GroupAddEditTabType,
openGroupAddEditDialog,
} from "./group-add-edit.component";
type GroupDetailsRow = {
/**
* Details used for displaying group information
*/
details: GroupView;
/**
* True if the group is selected in the table
*/
checked?: boolean;
/**
* A list of collection names the group has access to
*/
collectionNames?: string[];
};
/**
* Custom filter predicate that filters the groups table by id and name only.
* This is required because the default implementation searches by all properties, which can unintentionally match
* with members' names (who are assigned to the group) or collection names (which the group has access to).
*/
const groupsFilter = (filter: string) => {
const transformedFilter = filter.trim().toLowerCase();
return (data: GroupDetailsRow) => {
const group = data.details;
return (
group.id.toLowerCase().indexOf(transformedFilter) != -1 ||
group.name.toLowerCase().indexOf(transformedFilter) != -1
);
};
};
@Component({
templateUrl: "new-groups.component.html",
})
export class NewGroupsComponent {
loading = true;
organizationId: string;
protected dataSource = new TableDataSource<GroupDetailsRow>();
protected searchControl = new FormControl("");
// Fixed sizes used for cdkVirtualScroll
protected rowHeight = 46;
protected rowHeightClass = `tw-h-[46px]`;
protected ModalTabType = GroupAddEditTabType;
private refreshGroups$ = new BehaviorSubject<void>(null);
constructor(
private apiService: ApiService,
private groupService: GroupService,
private route: ActivatedRoute,
private i18nService: I18nService,
private dialogService: DialogService,
private logService: LogService,
private collectionService: CollectionService,
private toastService: ToastService,
) {
this.route.params
.pipe(
tap((params) => (this.organizationId = params.organizationId)),
switchMap(() =>
combineLatest([
// collectionMap
from(this.apiService.getCollections(this.organizationId)).pipe(
concatMap((response) => this.toCollectionMap(response)),
),
// groups
this.refreshGroups$.pipe(
switchMap(() => this.groupService.getAll(this.organizationId)),
),
]),
),
map(([collectionMap, groups]) => {
return groups.map<GroupDetailsRow>((g) => ({
id: g.id,
name: g.name,
details: g,
checked: false,
collectionNames: g.collections
.map((c) => collectionMap[c.id]?.name)
.sort(this.i18nService.collator?.compare),
}));
}),
takeUntilDestroyed(),
)
.subscribe((groups) => {
this.dataSource.data = groups;
this.loading = false;
});
// Connect the search input to the table dataSource filter input
this.searchControl.valueChanges
.pipe(debounceTime(200), takeUntilDestroyed())
.subscribe((v) => (this.dataSource.filter = groupsFilter(v)));
this.route.queryParams.pipe(first(), takeUntilDestroyed()).subscribe((qParams) => {
this.searchControl.setValue(qParams.search);
});
}
async edit(
group: GroupDetailsRow,
startingTabIndex: GroupAddEditTabType = GroupAddEditTabType.Info,
) {
const dialogRef = openGroupAddEditDialog(this.dialogService, {
data: {
initialTab: startingTabIndex,
organizationId: this.organizationId,
groupId: group != null ? group.details.id : null,
},
});
const result = await lastValueFrom(dialogRef.closed);
if (result == GroupAddEditDialogResultType.Saved) {
this.refreshGroups$.next();
} else if (result == GroupAddEditDialogResultType.Deleted) {
this.removeGroup(group);
}
}
async add() {
await this.edit(null);
}
async delete(groupRow: GroupDetailsRow) {
const confirmed = await this.dialogService.openSimpleDialog({
title: groupRow.details.name,
content: { key: "deleteGroupConfirmation" },
type: "warning",
});
if (!confirmed) {
return false;
}
try {
await this.groupService.delete(this.organizationId, groupRow.details.id);
this.toastService.showToast({
variant: "success",
title: null,
message: this.i18nService.t("deletedGroupId", groupRow.details.name),
});
this.removeGroup(groupRow);
} catch (e) {
this.logService.error(e);
}
}
async deleteAllSelected() {
const groupsToDelete = this.dataSource.data.filter((g) => g.checked);
if (groupsToDelete.length == 0) {
return;
}
const deleteMessage = groupsToDelete.map((g) => g.details.name).join(", ");
const confirmed = await this.dialogService.openSimpleDialog({
title: {
key: "deleteMultipleGroupsConfirmation",
placeholders: [groupsToDelete.length.toString()],
},
content: deleteMessage,
type: "warning",
});
if (!confirmed) {
return false;
}
try {
await this.groupService.deleteMany(
this.organizationId,
groupsToDelete.map((g) => g.details.id),
);
this.toastService.showToast({
variant: "success",
title: null,
message: this.i18nService.t("deletedManyGroups", groupsToDelete.length.toString()),
});
groupsToDelete.forEach((g) => this.removeGroup(g));
} catch (e) {
this.logService.error(e);
}
}
check(groupRow: GroupDetailsRow) {
groupRow.checked = !groupRow.checked;
}
toggleAllVisible(event: Event) {
this.dataSource.filteredData.forEach(
(g) => (g.checked = (event.target as HTMLInputElement).checked),
);
}
private removeGroup(groupRow: GroupDetailsRow) {
// Assign a new array to dataSource.data to trigger the setters and update the table
this.dataSource.data = this.dataSource.data.filter((g) => g !== groupRow);
}
private async toCollectionMap(response: ListResponse<CollectionResponse>) {
const collections = response.data.map(
(r) => new Collection(new CollectionData(r as CollectionDetailsResponse)),
);
const decryptedCollections = await this.collectionService.decryptMany(collections);
// Convert to an object using collection Ids as keys for faster name lookups
const collectionMap: Record<string, CollectionView> = {};
decryptedCollections.forEach((c) => (collectionMap[c.id] = c));
return collectionMap;
}
}

View File

@ -2,6 +2,7 @@ import { NgModule } from "@angular/core";
import { RouterModule, Routes } from "@angular/router";
import { AuthGuard } from "@bitwarden/angular/auth/guards";
import { featureFlaggedRoute } from "@bitwarden/angular/platform/utils/feature-flagged-route";
import {
canAccessOrgAdmin,
canAccessGroupsTab,
@ -11,11 +12,13 @@ import {
canAccessSettingsTab,
} from "@bitwarden/common/admin-console/abstractions/organization/organization.service.abstraction";
import { Organization } from "@bitwarden/common/admin-console/models/domain/organization";
import { FeatureFlag } from "@bitwarden/common/enums/feature-flag.enum";
import { organizationPermissionsGuard } from "../../admin-console/organizations/guards/org-permissions.guard";
import { organizationRedirectGuard } from "../../admin-console/organizations/guards/org-redirect.guard";
import { OrganizationLayoutComponent } from "../../admin-console/organizations/layouts/organization-layout.component";
import { GroupsComponent } from "../../admin-console/organizations/manage/groups.component";
import { NewGroupsComponent } from "../../admin-console/organizations/manage/new-groups.component";
import { deepLinkGuard } from "../../auth/guards/deep-link.guard";
import { VaultModule } from "../../vault/org-vault/vault.module";
@ -46,14 +49,18 @@ const routes: Routes = [
path: "members",
loadChildren: () => import("./members").then((m) => m.MembersModule),
},
{
...featureFlaggedRoute({
defaultComponent: GroupsComponent,
flaggedComponent: NewGroupsComponent,
featureFlag: FeatureFlag.GroupsComponentRefactor,
routeOptions: {
path: "groups",
component: GroupsComponent,
canActivate: [organizationPermissionsGuard(canAccessGroupsTab)],
data: {
titleId: "groups",
},
},
}),
{
path: "reporting",
loadChildren: () =>

View File

@ -1,3 +1,4 @@
import { ScrollingModule } from "@angular/cdk/scrolling";
import { NgModule } from "@angular/core";
import { LooseComponentsModule } from "../../shared";
@ -5,6 +6,7 @@ import { LooseComponentsModule } from "../../shared";
import { CoreOrganizationModule } from "./core";
import { GroupAddEditComponent } from "./manage/group-add-edit.component";
import { GroupsComponent } from "./manage/groups.component";
import { NewGroupsComponent } from "./manage/new-groups.component";
import { OrganizationsRoutingModule } from "./organization-routing.module";
import { SharedOrganizationModule } from "./shared";
import { AccessSelectorModule } from "./shared/components/access-selector";
@ -16,7 +18,8 @@ import { AccessSelectorModule } from "./shared/components/access-selector";
CoreOrganizationModule,
OrganizationsRoutingModule,
LooseComponentsModule,
ScrollingModule,
],
declarations: [GroupsComponent, GroupAddEditComponent],
declarations: [GroupsComponent, NewGroupsComponent, GroupAddEditComponent],
})
export class OrganizationModule {}

View File

@ -21,6 +21,7 @@ export enum FeatureFlag {
InlineMenuFieldQualification = "inline-menu-field-qualification",
MemberAccessReport = "ac-2059-member-access-report",
EnableTimeThreshold = "PM-5864-dollar-threshold",
GroupsComponentRefactor = "groups-component-refactor",
}
export type AllowedFeatureFlagTypes = boolean | number | string;
@ -52,6 +53,7 @@ export const DefaultFeatureFlagValue = {
[FeatureFlag.InlineMenuFieldQualification]: FALSE,
[FeatureFlag.MemberAccessReport]: FALSE,
[FeatureFlag.EnableTimeThreshold]: FALSE,
[FeatureFlag.GroupsComponentRefactor]: FALSE,
} satisfies Record<FeatureFlag, AllowedFeatureFlagTypes>;
export type DefaultFeatureFlagValueType = typeof DefaultFeatureFlagValue;