From dac06316028b37d96efed2bf1e674e6d682dfed4 Mon Sep 17 00:00:00 2001 From: Vijay Oommen Date: Tue, 11 Nov 2025 09:05:38 -0600 Subject: [PATCH] [PM-23375] Replace drawer with dialog (#17176) --- .../models/drawer-models.types.ts | 17 +-- .../risk-insights.component.html | 108 ------------------ .../risk-insights.component.ts | 78 ++++++------- ...risk-insights-drawer-dialog.component.html | 98 ++++++++++++++++ ...k-insights-drawer-dialog.component.spec.ts | 96 ++++++++++++++++ .../risk-insights-drawer-dialog.component.ts | 23 ++++ 6 files changed, 262 insertions(+), 158 deletions(-) create mode 100644 bitwarden_license/bit-web/src/app/dirt/access-intelligence/shared/risk-insights-drawer-dialog.component.html create mode 100644 bitwarden_license/bit-web/src/app/dirt/access-intelligence/shared/risk-insights-drawer-dialog.component.spec.ts create mode 100644 bitwarden_license/bit-web/src/app/dirt/access-intelligence/shared/risk-insights-drawer-dialog.component.ts diff --git a/bitwarden_license/bit-common/src/dirt/reports/risk-insights/models/drawer-models.types.ts b/bitwarden_license/bit-common/src/dirt/reports/risk-insights/models/drawer-models.types.ts index dffb22af3ee..fc500f6fd1f 100644 --- a/bitwarden_license/bit-common/src/dirt/reports/risk-insights/models/drawer-models.types.ts +++ b/bitwarden_license/bit-common/src/dirt/reports/risk-insights/models/drawer-models.types.ts @@ -1,14 +1,15 @@ import { MemberDetails } from "./report-models"; // -------------------- Drawer and UI Models -------------------- -// FIXME: update to use a const object instead of a typescript enum -// eslint-disable-next-line @bitwarden/platform/no-enums -export enum DrawerType { - None = 0, - AppAtRiskMembers = 1, - OrgAtRiskMembers = 2, - OrgAtRiskApps = 3, -} + +export const DrawerType = { + None: 0, + AppAtRiskMembers: 1, + OrgAtRiskMembers: 2, + OrgAtRiskApps: 3, +} as const; + +export type DrawerType = (typeof DrawerType)[keyof typeof DrawerType]; export type DrawerDetails = { open: boolean; diff --git a/bitwarden_license/bit-web/src/app/dirt/access-intelligence/risk-insights.component.html b/bitwarden_license/bit-web/src/app/dirt/access-intelligence/risk-insights.component.html index 4b7d51af174..9dbfe582ac9 100644 --- a/bitwarden_license/bit-web/src/app/dirt/access-intelligence/risk-insights.component.html +++ b/bitwarden_license/bit-web/src/app/dirt/access-intelligence/risk-insights.component.html @@ -109,112 +109,4 @@ } } - - @if (dataService.drawerDetails$ | async; as drawerDetails) { - - - - - - {{ - (drawerDetails.atRiskMemberDetails.length > 0 - ? "atRiskMembersDescription" - : "atRiskMembersDescriptionNone" - ) | i18n - }} - - -
-
- {{ "email" | i18n }} -
-
- {{ "atRiskPasswords" | i18n }} -
-
- -
-
{{ member.email }}
-
{{ member.atRiskPasswordCount }}
-
- - - - - - @if (dataService.isActiveDrawerType(drawerTypes.AppAtRiskMembers)) { - - - -
- {{ "atRiskMembersWithCount" | i18n: drawerDetails.appAtRiskMembers.members.length }} -
-
- {{ - (drawerDetails.appAtRiskMembers.members.length > 0 - ? "atRiskMembersDescriptionWithApp" - : "atRiskMembersDescriptionWithAppNone" - ) | i18n: drawerDetails.appAtRiskMembers.applicationName - }} -
-
- -
{{ member.email }}
- -
- - } - - @if (dataService.isActiveDrawerType(drawerTypes.OrgAtRiskApps)) { - - - - - {{ - (drawerDetails.atRiskAppDetails.length > 0 - ? "atRiskApplicationsDescription" - : "atRiskApplicationsDescriptionNone" - ) | i18n - }} - - -
-
- {{ "application" | i18n }} -
-
- {{ "atRiskPasswords" | i18n }} -
-
- -
-
{{ app.applicationName }}
-
{{ app.atRiskPasswordCount }}
-
- - - - } - - } diff --git a/bitwarden_license/bit-web/src/app/dirt/access-intelligence/risk-insights.component.ts b/bitwarden_license/bit-web/src/app/dirt/access-intelligence/risk-insights.component.ts index 5a5efa8225d..eddc26cbc77 100644 --- a/bitwarden_license/bit-web/src/app/dirt/access-intelligence/risk-insights.component.ts +++ b/bitwarden_license/bit-web/src/app/dirt/access-intelligence/risk-insights.component.ts @@ -1,10 +1,17 @@ import { animate, style, transition, trigger } from "@angular/animations"; import { CommonModule } from "@angular/common"; -import { Component, DestroyRef, OnDestroy, OnInit, inject } from "@angular/core"; +import { + Component, + DestroyRef, + OnDestroy, + OnInit, + inject, + ChangeDetectionStrategy, +} from "@angular/core"; import { takeUntilDestroyed } from "@angular/core/rxjs-interop"; import { ActivatedRoute, Router } from "@angular/router"; import { combineLatest, EMPTY, firstValueFrom } from "rxjs"; -import { map, tap } from "rxjs/operators"; +import { distinctUntilChanged, map, tap } from "rxjs/operators"; import { JslibModule } from "@bitwarden/angular/jslib.module"; import { @@ -21,9 +28,8 @@ import { OrganizationId } from "@bitwarden/common/types/guid"; import { AsyncActionsModule, ButtonModule, - DrawerBodyComponent, - DrawerComponent, - DrawerHeaderComponent, + DialogRef, + DialogService, TabsModule, } from "@bitwarden/components"; import { ExportHelper } from "@bitwarden/vault-export-core"; @@ -36,11 +42,11 @@ import { CriticalApplicationsComponent } from "./critical-applications/critical- import { EmptyStateCardComponent } from "./empty-state-card.component"; import { RiskInsightsTabType } from "./models/risk-insights.models"; import { PageLoadingComponent } from "./shared/page-loading.component"; +import { RiskInsightsDrawerDialogComponent } from "./shared/risk-insights-drawer-dialog.component"; import { ApplicationsLoadingComponent } from "./shared/risk-insights-loading.component"; -// FIXME(https://bitwarden.atlassian.net/browse/CL-764): Migrate to OnPush -// eslint-disable-next-line @angular-eslint/prefer-on-push-component-change-detection @Component({ + changeDetection: ChangeDetectionStrategy.OnPush, templateUrl: "./risk-insights.component.html", imports: [ AllApplicationsComponent, @@ -52,9 +58,6 @@ import { ApplicationsLoadingComponent } from "./shared/risk-insights-loading.com JslibModule, HeaderModule, TabsModule, - DrawerComponent, - DrawerBodyComponent, - DrawerHeaderComponent, AllActivityComponent, ApplicationsLoadingComponent, PageLoadingComponent, @@ -70,7 +73,6 @@ import { ApplicationsLoadingComponent } from "./shared/risk-insights-loading.com }) export class RiskInsightsComponent implements OnInit, OnDestroy { private destroyRef = inject(DestroyRef); - private _isDrawerOpen: boolean = false; protected ReportStatusEnum = ReportStatus; tabIndex: RiskInsightsTabType = RiskInsightsTabType.AllApps; @@ -94,6 +96,7 @@ export class RiskInsightsComponent implements OnInit, OnDestroy { protected emptyStateVideoSrc: string | null = "/videos/risk-insights-mark-as-critical.mp4"; protected IMPORT_ICON = "bwi bwi-download"; + protected currentDialogRef: DialogRef | null = null; // TODO: See https://github.com/bitwarden/clients/pull/16832#discussion_r2474523235 @@ -103,6 +106,7 @@ export class RiskInsightsComponent implements OnInit, OnDestroy { private configService: ConfigService, protected dataService: RiskInsightsDataService, protected i18nService: I18nService, + protected dialogService: DialogService, private fileDownloadService: FileDownloadService, private logService: LogService, ) { @@ -151,14 +155,32 @@ export class RiskInsightsComponent implements OnInit, OnDestroy { // Subscribe to drawer state changes this.dataService.drawerDetails$ - .pipe(takeUntilDestroyed(this.destroyRef)) + .pipe( + distinctUntilChanged( + (prev, curr) => + prev.activeDrawerType === curr.activeDrawerType && prev.invokerId === curr.invokerId, + ), + takeUntilDestroyed(this.destroyRef), + ) .subscribe((details) => { - this._isDrawerOpen = details.open; + if (details.activeDrawerType !== DrawerType.None) { + this.currentDialogRef = this.dialogService.openDrawer(RiskInsightsDrawerDialogComponent, { + data: details, + }); + } else { + this.currentDialogRef?.close(); + } }); + + // if any dialogs are open close it + // this happens when navigating between orgs + // or just navigating away from the page and back + this.currentDialogRef?.close(); } ngOnDestroy(): void { this.dataService.destroy(); + this.currentDialogRef?.close(); } /** @@ -179,35 +201,7 @@ export class RiskInsightsComponent implements OnInit, OnDestroy { }); // close drawer when tabs are changed - this.dataService.closeDrawer(); - } - - // Get a list of drawer types - get drawerTypes(): typeof DrawerType { - return DrawerType; - } - - /** - * Special case getter for syncing drawer state from service to component. - * This allows the template to use two-way binding while staying reactive. - */ - get isDrawerOpen() { - return this._isDrawerOpen; - } - - /** - * Special case setter for syncing drawer state from component to service. - * When the drawer component closes the drawer, this syncs the state back to the service. - */ - set isDrawerOpen(value: boolean) { - if (this._isDrawerOpen !== value) { - this._isDrawerOpen = value; - - // Close the drawer in the service if the drawer component closed the drawer - if (!value) { - this.dataService.closeDrawer(); - } - } + this.currentDialogRef?.close(); } // Empty state methods diff --git a/bitwarden_license/bit-web/src/app/dirt/access-intelligence/shared/risk-insights-drawer-dialog.component.html b/bitwarden_license/bit-web/src/app/dirt/access-intelligence/shared/risk-insights-drawer-dialog.component.html new file mode 100644 index 00000000000..d4ab4c5e98f --- /dev/null +++ b/bitwarden_license/bit-web/src/app/dirt/access-intelligence/shared/risk-insights-drawer-dialog.component.html @@ -0,0 +1,98 @@ +@if (isActiveDrawerType(drawerTypes.OrgAtRiskMembers)) { + + + {{ + "atRiskMembersWithCount" | i18n: drawerDetails.atRiskMemberDetails?.length ?? 0 + }} + + + {{ + (drawerDetails.atRiskMemberDetails?.length > 0 + ? "atRiskMembersDescription" + : "atRiskMembersDescriptionNone" + ) | i18n + }} + + @if (drawerDetails.atRiskMemberDetails?.length > 0) { + +
+
+ {{ "email" | i18n }} +
+
+ {{ "atRiskPasswords" | i18n }} +
+
+ @for (member of drawerDetails.atRiskMemberDetails; track member.email) { +
+
{{ member.email }}
+
{{ member.atRiskPasswordCount }}
+
+ } + + } + + +} + +@if (isActiveDrawerType(drawerTypes.AppAtRiskMembers)) { + + + {{ drawerDetails.appAtRiskMembers?.applicationName }} + + +
+ {{ "atRiskMembersWithCount" | i18n: drawerDetails.appAtRiskMembers?.members.length }} +
+
+ {{ + (drawerDetails.appAtRiskMembers?.members.length > 0 + ? "atRiskMembersDescriptionWithApp" + : "atRiskMembersDescriptionWithAppNone" + ) | i18n: drawerDetails.appAtRiskMembers?.applicationName + }} +
+
+ @for (member of drawerDetails.appAtRiskMembers?.members; track $index) { +
{{ member.email }}
+ } +
+ + +} + +@if (isActiveDrawerType(drawerTypes.OrgAtRiskApps)) { + + + {{ + "atRiskApplicationsWithCount" | i18n: drawerDetails.atRiskAppDetails?.length ?? 0 + }} + + + {{ + (drawerDetails.atRiskAppDetails?.length > 0 + ? "atRiskApplicationsDescription" + : "atRiskApplicationsDescriptionNone" + ) | i18n + }} + @if (drawerDetails.atRiskAppDetails?.length > 0) { + +
+
+ {{ "application" | i18n }} +
+
+ {{ "atRiskPasswords" | i18n }} +
+
+ @for (app of drawerDetails.atRiskAppDetails; track app.applicationName) { +
+
{{ app.applicationName }}
+
{{ app.atRiskPasswordCount }}
+
+ } + + } + + +} diff --git a/bitwarden_license/bit-web/src/app/dirt/access-intelligence/shared/risk-insights-drawer-dialog.component.spec.ts b/bitwarden_license/bit-web/src/app/dirt/access-intelligence/shared/risk-insights-drawer-dialog.component.spec.ts new file mode 100644 index 00000000000..2b5910ed99e --- /dev/null +++ b/bitwarden_license/bit-web/src/app/dirt/access-intelligence/shared/risk-insights-drawer-dialog.component.spec.ts @@ -0,0 +1,96 @@ +import { ComponentFixture, TestBed } from "@angular/core/testing"; +import { BrowserAnimationsModule } from "@angular/platform-browser/animations"; +import { mock } from "jest-mock-extended"; + +import { DrawerDetails, DrawerType } from "@bitwarden/bit-common/dirt/reports/risk-insights"; +import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; +import { DIALOG_DATA } from "@bitwarden/components"; +import { I18nPipe } from "@bitwarden/ui-common"; + +import { RiskInsightsDrawerDialogComponent } from "./risk-insights-drawer-dialog.component"; + +beforeAll(() => { + // Mock element.animate for jsdom + // the animate function is not available in jsdom, so we provide a mock implementation + // This is necessary for tests that rely on animations + // This mock does not perform any actual animations, it just provides a structure that allows tests + // to run without throwing errors related to missing animate function + if (!HTMLElement.prototype.animate) { + HTMLElement.prototype.animate = function () { + return { + play: () => {}, + pause: () => {}, + finish: () => {}, + cancel: () => {}, + reverse: () => {}, + addEventListener: () => {}, + removeEventListener: () => {}, + dispatchEvent: () => false, + onfinish: null, + oncancel: null, + startTime: 0, + currentTime: 0, + playbackRate: 1, + playState: "idle", + replaceState: "active", + effect: null, + finished: Promise.resolve(), + id: "", + remove: () => {}, + timeline: null, + ready: Promise.resolve(), + } as unknown as Animation; + }; + } +}); + +describe("RiskInsightsDrawerDialogComponent", () => { + let component: RiskInsightsDrawerDialogComponent; + let fixture: ComponentFixture; + const mockI18nService = mock(); + const drawerDetails: DrawerDetails = { + open: true, + invokerId: "test-invoker", + activeDrawerType: DrawerType.None, + atRiskMemberDetails: [], + appAtRiskMembers: null, + atRiskAppDetails: null, + }; + + beforeEach(async () => { + await TestBed.configureTestingModule({ + imports: [RiskInsightsDrawerDialogComponent, BrowserAnimationsModule], + providers: [ + { provide: DIALOG_DATA, useValue: drawerDetails }, + { provide: I18nPipe, useValue: mock() }, + { provide: I18nService, useValue: mockI18nService }, + ], + }).compileComponents(); + + fixture = TestBed.createComponent(RiskInsightsDrawerDialogComponent); + component = fixture.componentInstance; + fixture.detectChanges(); + }); + + it("should create", () => { + expect(component).toBeTruthy(); + }); + + describe("drawerTypes getter", () => { + it("should return DrawerType enum", () => { + expect(component.drawerTypes).toBe(DrawerType); + }); + }); + + describe("isActiveDrawerType", () => { + it("should return true if type matches activeDrawerType", () => { + component.drawerDetails.activeDrawerType = DrawerType.None; + expect(component.isActiveDrawerType(DrawerType.None)).toBeTruthy(); + }); + + it("should return false if type does not match activeDrawerType", () => { + component.drawerDetails.activeDrawerType = DrawerType.None; + expect(component.isActiveDrawerType(DrawerType.AppAtRiskMembers)).toBeFalsy(); + }); + }); +}); diff --git a/bitwarden_license/bit-web/src/app/dirt/access-intelligence/shared/risk-insights-drawer-dialog.component.ts b/bitwarden_license/bit-web/src/app/dirt/access-intelligence/shared/risk-insights-drawer-dialog.component.ts new file mode 100644 index 00000000000..82cddda542c --- /dev/null +++ b/bitwarden_license/bit-web/src/app/dirt/access-intelligence/shared/risk-insights-drawer-dialog.component.ts @@ -0,0 +1,23 @@ +import { Component, ChangeDetectionStrategy, Inject } from "@angular/core"; + +import { DrawerDetails, DrawerType } from "@bitwarden/bit-common/dirt/reports/risk-insights"; +import { DIALOG_DATA } from "@bitwarden/components"; +import { SharedModule } from "@bitwarden/web-vault/app/shared"; + +@Component({ + imports: [SharedModule], + templateUrl: "./risk-insights-drawer-dialog.component.html", + changeDetection: ChangeDetectionStrategy.OnPush, +}) +export class RiskInsightsDrawerDialogComponent { + constructor(@Inject(DIALOG_DATA) public drawerDetails: DrawerDetails) {} + + // Get a list of drawer types + get drawerTypes(): typeof DrawerType { + return DrawerType; + } + + isActiveDrawerType(type: DrawerType): boolean { + return this.drawerDetails.activeDrawerType === type; + } +}