diff --git a/bitwarden_license/bit-web/src/app/dirt/access-intelligence/all-applications/applications.component.html b/bitwarden_license/bit-web/src/app/dirt/access-intelligence/all-applications/applications.component.html index 743f8ff1b68..ec73c4f47e6 100644 --- a/bitwarden_license/bit-web/src/app/dirt/access-intelligence/all-applications/applications.component.html +++ b/bitwarden_license/bit-web/src/app/dirt/access-intelligence/all-applications/applications.component.html @@ -22,7 +22,7 @@
- @if (selectedUrls().size > 0) { + @if (visibleSelectedApps().size > 0) { @if (allSelectedAppsAreCritical()) { } @else { } } @@ -79,8 +79,9 @@ [dataSource]="dataSource" [selectedUrls]="selectedUrls()" [openApplication]="drawerDetails.invokerId || ''" - [checkboxChange]="onCheckboxChange" [showAppAtRiskMembers]="showAppAtRiskMembers" + (checkboxChange)="onCheckboxChange($event)" + (selectAllChange)="onSelectAllChange($event)" class="tw-mb-10" > diff --git a/bitwarden_license/bit-web/src/app/dirt/access-intelligence/all-applications/applications.component.spec.ts b/bitwarden_license/bit-web/src/app/dirt/access-intelligence/all-applications/applications.component.spec.ts index b4cbbc5c436..b79f5160bf7 100644 --- a/bitwarden_license/bit-web/src/app/dirt/access-intelligence/all-applications/applications.component.spec.ts +++ b/bitwarden_license/bit-web/src/app/dirt/access-intelligence/all-applications/applications.component.spec.ts @@ -1,8 +1,9 @@ +import { Signal, WritableSignal } from "@angular/core"; import { ComponentFixture, TestBed } from "@angular/core/testing"; import { ReactiveFormsModule } from "@angular/forms"; -import { ActivatedRoute } from "@angular/router"; +import { ActivatedRoute, convertToParamMap } from "@angular/router"; import { mock, MockProxy } from "jest-mock-extended"; -import { BehaviorSubject } from "rxjs"; +import { BehaviorSubject, of } from "rxjs"; import { DrawerDetails, @@ -11,6 +12,7 @@ import { ReportStatus, RiskInsightsDataService, } from "@bitwarden/bit-common/dirt/reports/risk-insights"; +import { createNewSummaryData } from "@bitwarden/bit-common/dirt/reports/risk-insights/helpers"; import { RiskInsightsEnrichedData } from "@bitwarden/bit-common/dirt/reports/risk-insights/models/report-data-service.types"; import { FileDownloadService } from "@bitwarden/common/platform/abstractions/file-download/file-download.service"; import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; @@ -23,9 +25,18 @@ import { AccessIntelligenceSecurityTasksService } from "../shared/security-tasks import { ApplicationsComponent } from "./applications.component"; +// Mock ResizeObserver +global.ResizeObserver = class ResizeObserver { + observe() {} + unobserve() {} + disconnect() {} +}; + // Helper type to access protected members in tests type ComponentWithProtectedMembers = ApplicationsComponent & { dataSource: TableDataSource; + selectedUrls: WritableSignal>; + filteredTableData: Signal; }; describe("ApplicationsComponent", () => { @@ -83,7 +94,10 @@ describe("ApplicationsComponent", () => { { provide: RiskInsightsDataService, useValue: mockDataService }, { provide: ActivatedRoute, - useValue: { snapshot: { paramMap: { get: (): string | null => null } } }, + useValue: { + paramMap: of(convertToParamMap({})), + snapshot: { paramMap: convertToParamMap({}) }, + }, }, { provide: AccessIntelligenceSecurityTasksService, useValue: mockSecurityTasksService }, ], @@ -91,6 +105,7 @@ describe("ApplicationsComponent", () => { fixture = TestBed.createComponent(ApplicationsComponent); component = fixture.componentInstance; + fixture.detectChanges(); }); afterEach(() => { @@ -247,4 +262,185 @@ describe("ApplicationsComponent", () => { expect(capturedBlobData).not.toContain("Slack"); }); }); + + describe("checkbox selection", () => { + const mockApplicationData: ApplicationTableDataSource[] = [ + { + applicationName: "GitHub", + passwordCount: 10, + atRiskPasswordCount: 3, + memberCount: 5, + atRiskMemberCount: 2, + isMarkedAsCritical: true, + atRiskCipherIds: ["cipher1" as CipherId], + memberDetails: [] as MemberDetails[], + atRiskMemberDetails: [] as MemberDetails[], + cipherIds: ["cipher1" as CipherId], + iconCipher: undefined, + }, + { + applicationName: "Slack", + passwordCount: 8, + atRiskPasswordCount: 1, + memberCount: 4, + atRiskMemberCount: 1, + isMarkedAsCritical: false, + atRiskCipherIds: ["cipher2" as CipherId], + memberDetails: [] as MemberDetails[], + atRiskMemberDetails: [] as MemberDetails[], + cipherIds: ["cipher2" as CipherId], + iconCipher: undefined, + }, + { + applicationName: "Jira", + passwordCount: 12, + atRiskPasswordCount: 4, + memberCount: 6, + atRiskMemberCount: 3, + isMarkedAsCritical: false, + atRiskCipherIds: ["cipher3" as CipherId], + memberDetails: [] as MemberDetails[], + atRiskMemberDetails: [] as MemberDetails[], + cipherIds: ["cipher3" as CipherId], + iconCipher: undefined, + }, + ]; + + beforeEach(() => { + // Emit mock data through the data service observable to populate the table + enrichedReportData$.next({ + reportData: mockApplicationData, + summaryData: createNewSummaryData(), + applicationData: [], + creationDate: new Date(), + }); + }); + + describe("onCheckboxChange", () => { + it("should add application to selectedUrls when checked is true", () => { + // act + component.onCheckboxChange({ applicationName: "GitHub", checked: true }); + + // assert + const selectedUrls = (component as ComponentWithProtectedMembers).selectedUrls(); + expect(selectedUrls.has("GitHub")).toBe(true); + expect(selectedUrls.size).toBe(1); + }); + + it("should remove application from selectedUrls when checked is false", () => { + // arrange + (component as ComponentWithProtectedMembers).selectedUrls.set(new Set(["GitHub", "Slack"])); + + // act + component.onCheckboxChange({ applicationName: "GitHub", checked: false }); + + // assert + const selectedUrls = (component as ComponentWithProtectedMembers).selectedUrls(); + expect(selectedUrls.has("GitHub")).toBe(false); + expect(selectedUrls.has("Slack")).toBe(true); + expect(selectedUrls.size).toBe(1); + }); + + it("should handle multiple applications being selected", () => { + // act + component.onCheckboxChange({ applicationName: "GitHub", checked: true }); + component.onCheckboxChange({ applicationName: "Slack", checked: true }); + component.onCheckboxChange({ applicationName: "Jira", checked: true }); + + // assert + const selectedUrls = (component as ComponentWithProtectedMembers).selectedUrls(); + expect(selectedUrls.has("GitHub")).toBe(true); + expect(selectedUrls.has("Slack")).toBe(true); + expect(selectedUrls.has("Jira")).toBe(true); + expect(selectedUrls.size).toBe(3); + }); + }); + + describe("onSelectAllChange", () => { + it("should add all visible applications to selectedUrls when checked is true", () => { + // act + component.onSelectAllChange(true); + + // assert + const selectedUrls = (component as ComponentWithProtectedMembers).selectedUrls(); + expect(selectedUrls.has("GitHub")).toBe(true); + expect(selectedUrls.has("Slack")).toBe(true); + expect(selectedUrls.has("Jira")).toBe(true); + expect(selectedUrls.size).toBe(3); + }); + + it("should remove all applications from selectedUrls when checked is false", () => { + // arrange + (component as ComponentWithProtectedMembers).selectedUrls.set(new Set(["GitHub", "Slack"])); + + // act + component.onSelectAllChange(false); + + // assert + const selectedUrls = (component as ComponentWithProtectedMembers).selectedUrls(); + expect(selectedUrls.size).toBe(0); + }); + + it("should only add visible filtered applications when filter is applied", () => { + // arrange - apply filter to only show critical apps + (component as ComponentWithProtectedMembers).dataSource.filter = ( + app: ApplicationTableDataSource, + ) => app.isMarkedAsCritical; + fixture.detectChanges(); + + // act + component.onSelectAllChange(true); + + // assert - only GitHub is critical + const selectedUrls = (component as ComponentWithProtectedMembers).selectedUrls(); + expect(selectedUrls.has("GitHub")).toBe(true); + expect(selectedUrls.has("Slack")).toBe(false); + expect(selectedUrls.has("Jira")).toBe(false); + expect(selectedUrls.size).toBe(1); + }); + + it("should only remove visible filtered applications when unchecking with filter applied", () => { + // arrange - select all apps first, then apply filter to only show non-critical apps + (component as ComponentWithProtectedMembers).selectedUrls.set( + new Set(["GitHub", "Slack", "Jira"]), + ); + + (component as ComponentWithProtectedMembers).dataSource.filter = ( + app: ApplicationTableDataSource, + ) => !app.isMarkedAsCritical; + fixture.detectChanges(); + + // act - uncheck with filter applied + component.onSelectAllChange(false); + + // assert - GitHub (critical) should still be selected + const selectedUrls = (component as ComponentWithProtectedMembers).selectedUrls(); + expect(selectedUrls.has("GitHub")).toBe(true); + expect(selectedUrls.has("Slack")).toBe(false); + expect(selectedUrls.has("Jira")).toBe(false); + expect(selectedUrls.size).toBe(1); + }); + + it("should preserve existing selections when checking select all with filter", () => { + // arrange - select a non-visible app + (component as ComponentWithProtectedMembers).selectedUrls.set(new Set(["GitHub"])); + + // apply filter to hide GitHub + (component as ComponentWithProtectedMembers).dataSource.filter = ( + app: ApplicationTableDataSource, + ) => !app.isMarkedAsCritical; + fixture.detectChanges(); + + // act - select all visible (non-critical apps) + component.onSelectAllChange(true); + + // assert - GitHub should still be selected + visible apps + const selectedUrls = (component as ComponentWithProtectedMembers).selectedUrls(); + expect(selectedUrls.has("GitHub")).toBe(true); + expect(selectedUrls.has("Slack")).toBe(true); + expect(selectedUrls.has("Jira")).toBe(true); + expect(selectedUrls.size).toBe(3); + }); + }); + }); }); diff --git a/bitwarden_license/bit-web/src/app/dirt/access-intelligence/all-applications/applications.component.ts b/bitwarden_license/bit-web/src/app/dirt/access-intelligence/all-applications/applications.component.ts index 962628584d3..659e099641c 100644 --- a/bitwarden_license/bit-web/src/app/dirt/access-intelligence/all-applications/applications.component.ts +++ b/bitwarden_license/bit-web/src/app/dirt/access-intelligence/all-applications/applications.component.ts @@ -89,6 +89,9 @@ export class ApplicationsComponent implements OnInit { // Standard properties protected readonly dataSource = new TableDataSource(); protected readonly searchControl = new FormControl("", { nonNullable: true }); + protected readonly filteredTableData = toSignal(this.dataSource.connect(), { + initialValue: [], + }); // Template driven properties protected readonly selectedUrls = signal(new Set()); @@ -118,13 +121,35 @@ export class ApplicationsComponent implements OnInit { }, ]); + // Computed property that returns only selected applications that are currently visible in filtered data + readonly visibleSelectedApps = computed(() => { + const filteredData = this.filteredTableData(); + const selected = this.selectedUrls(); + + if (!filteredData || selected.size === 0) { + return new Set(); + } + + const visibleSelected = new Set(); + filteredData.forEach((row) => { + if (selected.has(row.applicationName)) { + visibleSelected.add(row.applicationName); + } + }); + + return visibleSelected; + }); + readonly allSelectedAppsAreCritical = computed(() => { - if (!this.dataSource.filteredData || this.selectedUrls().size == 0) { + const visibleSelected = this.visibleSelectedApps(); + const filteredData = this.filteredTableData(); + + if (!filteredData || visibleSelected.size === 0) { return false; } - return this.dataSource.filteredData - .filter((row) => this.selectedUrls().has(row.applicationName)) + return filteredData + .filter((row) => visibleSelected.has(row.applicationName)) .every((row) => row.isMarkedAsCritical); }); @@ -202,15 +227,6 @@ export class ApplicationsComponent implements OnInit { this.dataSource.filter = (app) => filterFunction(app) && app.applicationName.toLowerCase().includes(searchText.toLowerCase()); - - // filter selectedUrls down to only applications showing with active filters - const filteredUrls = new Set(); - this.dataSource.filteredData?.forEach((row) => { - if (this.selectedUrls().has(row.applicationName)) { - filteredUrls.add(row.applicationName); - } - }); - this.selectedUrls.set(filteredUrls); }); } @@ -218,12 +234,13 @@ export class ApplicationsComponent implements OnInit { this.selectedFilter.set(value); } - markAppsAsCritical = async () => { + async markAppsAsCritical() { this.updatingCriticalApps.set(true); - const count = this.selectedUrls().size; + const visibleSelected = this.visibleSelectedApps(); + const count = visibleSelected.size; this.dataService - .saveCriticalApplications(Array.from(this.selectedUrls())) + .saveCriticalApplications(Array.from(visibleSelected)) .pipe(takeUntilDestroyed(this.destroyRef)) .subscribe({ next: (response) => { @@ -246,11 +263,11 @@ export class ApplicationsComponent implements OnInit { }); }, }); - }; + } - unmarkAppsAsCritical = async () => { + async unmarkAppsAsCritical() { this.updatingCriticalApps.set(true); - const appsToUnmark = this.selectedUrls(); + const appsToUnmark = this.visibleSelectedApps(); this.dataService .removeCriticalApplications(appsToUnmark) @@ -278,7 +295,7 @@ export class ApplicationsComponent implements OnInit { }); }, }); - }; + } async requestPasswordChange() { const orgId = this.organizationId(); @@ -310,24 +327,38 @@ export class ApplicationsComponent implements OnInit { } } - showAppAtRiskMembers = async (applicationName: string) => { + async showAppAtRiskMembers(applicationName: string) { await this.dataService.setDrawerForAppAtRiskMembers(applicationName); - }; + } - onCheckboxChange = (applicationName: string, event: Event) => { - const isChecked = (event.target as HTMLInputElement).checked; + onCheckboxChange({ applicationName, checked }: { applicationName: string; checked: boolean }) { this.selectedUrls.update((selectedUrls) => { const nextSelected = new Set(selectedUrls); - if (isChecked) { + if (checked) { nextSelected.add(applicationName); } else { nextSelected.delete(applicationName); } return nextSelected; }); - }; + } - downloadApplicationsCSV = () => { + onSelectAllChange(checked: boolean) { + const filteredData = this.filteredTableData(); + if (!filteredData) { + return; + } + + this.selectedUrls.update((selectedUrls) => { + const nextSelected = new Set(selectedUrls); + filteredData.forEach((row) => + checked ? nextSelected.add(row.applicationName) : nextSelected.delete(row.applicationName), + ); + return nextSelected; + }); + } + + downloadApplicationsCSV() { try { const data = this.dataSource.filteredData; if (!data || data.length === 0) { @@ -360,5 +391,5 @@ export class ApplicationsComponent implements OnInit { } catch (error) { this.logService.error("Failed to download applications CSV", error); } - }; + } } diff --git a/bitwarden_license/bit-web/src/app/dirt/access-intelligence/shared/app-table-row-scrollable-m11.component.html b/bitwarden_license/bit-web/src/app/dirt/access-intelligence/shared/app-table-row-scrollable-m11.component.html index 05dec048328..ddbc977fc13 100644 --- a/bitwarden_license/bit-web/src/app/dirt/access-intelligence/shared/app-table-row-scrollable-m11.component.html +++ b/bitwarden_license/bit-web/src/app/dirt/access-intelligence/shared/app-table-row-scrollable-m11.component.html @@ -33,7 +33,7 @@ bitCheckbox type="checkbox" [checked]="selectedUrls().has(row.applicationName)" - (change)="checkboxChange()(row.applicationName, $event)" + (change)="checkboxChanged($event.target, row.applicationName)" /> { selectAllCheckboxEl = fixture.debugElement.query(By.css('[data-testid="selectAll"]')); }); - it("should check all rows in table when checked", () => { + it("should emit selectAllChange event with true when checked", () => { // arrange const selectedUrls = new Set(); const dataSource = new TableDataSource(); @@ -121,18 +121,19 @@ describe("AppTableRowScrollableM11Component", () => { fixture.componentRef.setInput("dataSource", dataSource); fixture.detectChanges(); + const selectAllChangeSpy = jest.fn(); + fixture.componentInstance.selectAllChange.subscribe(selectAllChangeSpy); + // act selectAllCheckboxEl.nativeElement.click(); fixture.detectChanges(); // assert - expect(selectedUrls.has("google.com")).toBe(true); - expect(selectedUrls.has("facebook.com")).toBe(true); - expect(selectedUrls.has("twitter.com")).toBe(true); - expect(selectedUrls.size).toBe(3); + expect(selectAllChangeSpy).toHaveBeenCalledWith(true); + expect(selectAllChangeSpy).toHaveBeenCalledTimes(1); }); - it("should uncheck all rows in table when unchecked", () => { + it("should emit selectAllChange event with false when unchecked", () => { // arrange const selectedUrls = new Set(["google.com", "facebook.com", "twitter.com"]); const dataSource = new TableDataSource(); @@ -142,12 +143,16 @@ describe("AppTableRowScrollableM11Component", () => { fixture.componentRef.setInput("dataSource", dataSource); fixture.detectChanges(); + const selectAllChangeSpy = jest.fn(); + fixture.componentInstance.selectAllChange.subscribe(selectAllChangeSpy); + // act selectAllCheckboxEl.nativeElement.click(); fixture.detectChanges(); // assert - expect(selectedUrls.size).toBe(0); + expect(selectAllChangeSpy).toHaveBeenCalledWith(false); + expect(selectAllChangeSpy).toHaveBeenCalledTimes(1); }); it("should become checked when all rows in table are checked", () => { @@ -178,4 +183,59 @@ describe("AppTableRowScrollableM11Component", () => { expect(selectAllCheckboxEl.nativeElement.checked).toBe(false); }); }); + + describe("individual row checkbox", () => { + it("should emit checkboxChange event with correct parameters when checkboxChanged is called", () => { + // arrange + const checkboxChangeSpy = jest.fn(); + fixture.componentInstance.checkboxChange.subscribe(checkboxChangeSpy); + + const mockTarget = { checked: true } as HTMLInputElement; + + // act + fixture.componentInstance.checkboxChanged(mockTarget, "google.com"); + + // assert + expect(checkboxChangeSpy).toHaveBeenCalledWith({ + applicationName: "google.com", + checked: true, + }); + expect(checkboxChangeSpy).toHaveBeenCalledTimes(1); + }); + + it("should emit checkboxChange with checked=false when checkbox is unchecked", () => { + // arrange + const checkboxChangeSpy = jest.fn(); + fixture.componentInstance.checkboxChange.subscribe(checkboxChangeSpy); + + const mockTarget = { checked: false } as HTMLInputElement; + + // act + fixture.componentInstance.checkboxChanged(mockTarget, "google.com"); + + // assert + expect(checkboxChangeSpy).toHaveBeenCalledWith({ + applicationName: "google.com", + checked: false, + }); + expect(checkboxChangeSpy).toHaveBeenCalledTimes(1); + }); + + it("should emit checkboxChange with correct applicationName for different applications", () => { + // arrange + const checkboxChangeSpy = jest.fn(); + fixture.componentInstance.checkboxChange.subscribe(checkboxChangeSpy); + + const mockTarget = { checked: true } as HTMLInputElement; + + // act + fixture.componentInstance.checkboxChanged(mockTarget, "facebook.com"); + + // assert + expect(checkboxChangeSpy).toHaveBeenCalledWith({ + applicationName: "facebook.com", + checked: true, + }); + }); + }); }); diff --git a/bitwarden_license/bit-web/src/app/dirt/access-intelligence/shared/app-table-row-scrollable-m11.component.ts b/bitwarden_license/bit-web/src/app/dirt/access-intelligence/shared/app-table-row-scrollable-m11.component.ts index a23d1855ba5..65cfb8d092e 100644 --- a/bitwarden_license/bit-web/src/app/dirt/access-intelligence/shared/app-table-row-scrollable-m11.component.ts +++ b/bitwarden_license/bit-web/src/app/dirt/access-intelligence/shared/app-table-row-scrollable-m11.component.ts @@ -1,5 +1,5 @@ import { CommonModule } from "@angular/common"; -import { ChangeDetectionStrategy, Component, input } from "@angular/core"; +import { ChangeDetectionStrategy, Component, input, output } from "@angular/core"; import { JslibModule } from "@bitwarden/angular/jslib.module"; import { MenuModule, TableDataSource, TableModule, TooltipDirective } from "@bitwarden/components"; @@ -30,7 +30,8 @@ export class AppTableRowScrollableM11Component { readonly selectedUrls = input>(); readonly openApplication = input(""); readonly showAppAtRiskMembers = input<(applicationName: string) => void>(); - readonly checkboxChange = input<(applicationName: string, $event: Event) => void>(); + readonly checkboxChange = output<{ applicationName: string; checked: boolean }>(); + readonly selectAllChange = output(); allAppsSelected(): boolean { const tableData = this.dataSource()?.filteredData; @@ -43,20 +44,13 @@ export class AppTableRowScrollableM11Component { return tableData.length > 0 && tableData.every((row) => selectedUrls.has(row.applicationName)); } + checkboxChanged(target: HTMLInputElement, applicationName: string) { + const checked = target.checked; + this.checkboxChange.emit({ applicationName, checked }); + } + selectAllChanged(target: HTMLInputElement) { const checked = target.checked; - - const tableData = this.dataSource()?.filteredData; - const selectedUrls = this.selectedUrls(); - - if (!tableData || !selectedUrls) { - return false; - } - - if (checked) { - tableData.forEach((row) => selectedUrls.add(row.applicationName)); - } else { - selectedUrls.clear(); - } + this.selectAllChange.emit(checked); } }