bitwarden/browser
1
0
Fork 0
mirror of https://github.com/bitwarden/browser synced 2026-02-24 08:33:29 +00:00
Code Issues Releases Wiki Activity

[PM-32127] Access Intelligence: persist selected applications through filter updates (#18990)

Browse Source 
This PR includes changes to the Access Intelligence table view, which keep Applications selected in the table as the user makes changes to filters (search bar, critical applications filter). This required updating logic to ensure only visible rows in the table are considered for updates to critical status with the "Mark # as critical" button, while still maintaining the full list of selected applications in the component's selectedUrls.

The Applications table component is also refactored to use Angular output for checkbox state, emitting events on checkbox changes for individual table rows and "select all". The parent component handles these events by updating the set of selected Applications (selectedUrls) accordingly.

Test cases are updated/added to cover the updated checkbox functionality.
This commit is contained in:
Brad
2026-02-20 11:11:17 -08:00
committed by GitHub
parent 84845024fd
commit 72be42ed68
6 changed files with 339 additions and 57 deletions
Download Patch File Download Diff File Expand all files Collapse all files

9
bitwarden_license/bit-web/src/app/dirt/access-intelligence/all-applications/applications.component.html
View File

@@ -22,7 +22,7 @@
</bit-chip-select>
<div class="tw-ml-auto tw-gap-4 tw-flex">
@if (selectedUrls().size > 0) {
@if (visibleSelectedApps().size > 0) {
@if (allSelectedAppsAreCritical()) {
<button
type="button"
@@ -33,7 +33,7 @@
(click)="unmarkAppsAsCritical()"
data-testid="unmark-critical-button"
>
{{ "markAppCountAsNotCritical" | i18n: selectedUrls().size }}
{{ "markAppCountAsNotCritical" | i18n: visibleSelectedApps().size }}
</button>
} @else {
<button
@@ -45,7 +45,7 @@
(click)="markAppsAsCritical()"
data-testid="mark-critical-button"
>
{{ "markAppCountAsCritical" | i18n: selectedUrls().size }}
{{ "markAppCountAsCritical" | i18n: visibleSelectedApps().size }}
</button>
}
}
@@ -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"
></app-table-row-scrollable-m11>

202
bitwarden_license/bit-web/src/app/dirt/access-intelligence/all-applications/applications.component.spec.ts
View File

@@ -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<ApplicationTableDataSource>;
selectedUrls: WritableSignal<Set<string>>;
filteredTableData: Signal<ApplicationTableDataSource[]>;
};
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);
});
});
});
});

85
bitwarden_license/bit-web/src/app/dirt/access-intelligence/all-applications/applications.component.ts
View File

@@ -89,6 +89,9 @@ export class ApplicationsComponent implements OnInit {
// Standard properties
protected readonly dataSource = new TableDataSource<ApplicationTableDataSource>();
protected readonly searchControl = new FormControl<string>("", { nonNullable: true });
protected readonly filteredTableData = toSignal(this.dataSource.connect(), {
initialValue: [],
});
// Template driven properties
protected readonly selectedUrls = signal(new Set<string>());
@@ -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<string>();
}
const visibleSelected = new Set<string>();
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<string>();
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);
}
};
}
}

2
bitwarden_license/bit-web/src/app/dirt/access-intelligence/shared/app-table-row-scrollable-m11.component.html
View File

@@ -33,7 +33,7 @@
bitCheckbox
type="checkbox"
[checked]="selectedUrls().has(row.applicationName)"
(change)="checkboxChange()(row.applicationName, $event)"
(change)="checkboxChanged($event.target, row.applicationName)"
/>
</td>
<td

74
bitwarden_license/bit-web/src/app/dirt/access-intelligence/shared/app-table-row-scrollable-m11.component.spec.ts
View File

@@ -111,7 +111,7 @@ describe("AppTableRowScrollableM11Component", () => {
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<string>();
const dataSource = new TableDataSource<ApplicationHealthReportDetailEnriched>();
@@ -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<string>(["google.com", "facebook.com", "twitter.com"]);
const dataSource = new TableDataSource<ApplicationHealthReportDetailEnriched>();
@@ -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,
});
});
});
});

24
bitwarden_license/bit-web/src/app/dirt/access-intelligence/shared/app-table-row-scrollable-m11.component.ts
View File

@@ -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<Set<string>>();
readonly openApplication = input<string>("");
readonly showAppAtRiskMembers = input<(applicationName: string) => void>();
readonly checkboxChange = input<(applicationName: string, $event: Event) => void>();
readonly checkboxChange = output<{ applicationName: string; checked: boolean }>();
readonly selectAllChange = output<boolean>();
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);
}
}