From b646598613b6a13df665edd74ecd8eefb9e175ab Mon Sep 17 00:00:00 2001 From: Alex Date: Thu, 19 Mar 2026 12:12:45 -0400 Subject: [PATCH] fix(dirt): check response.error in Review New Applications mark critical The AllApplicationsComponent (Review New Applications widget) had the same false success toast bug as the All Applications tab. The next handler ignored the response parameter entirely, showing a success toast even when response.error was set by the orchestrator's catchError. Now checks response.error before showing toast. On error: shows "Failed to mark applications as critical" and preserves selection. Also resets markingAsCritical on all paths. Adds unit tests for mark success and error paths. [PM-33067] --- .../all-applications.component.spec.ts | 142 ++++++++++++++++++ .../all-applications.component.ts | 15 +- 2 files changed, 155 insertions(+), 2 deletions(-) create mode 100644 bitwarden_license/bit-web/src/app/dirt/access-intelligence/all-applications/all-applications.component.spec.ts diff --git a/bitwarden_license/bit-web/src/app/dirt/access-intelligence/all-applications/all-applications.component.spec.ts b/bitwarden_license/bit-web/src/app/dirt/access-intelligence/all-applications/all-applications.component.spec.ts new file mode 100644 index 00000000000..b1c0d29ed9e --- /dev/null +++ b/bitwarden_license/bit-web/src/app/dirt/access-intelligence/all-applications/all-applications.component.spec.ts @@ -0,0 +1,142 @@ +import { WritableSignal } from "@angular/core"; +import { ComponentFixture, TestBed } from "@angular/core/testing"; +import { ReactiveFormsModule } from "@angular/forms"; +import { ActivatedRoute, convertToParamMap } from "@angular/router"; +import { mock, MockProxy } from "jest-mock-extended"; +import { BehaviorSubject, of } from "rxjs"; + +import { + DrawerDetails, + DrawerType, + 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 { ReportState } from "@bitwarden/bit-common/dirt/reports/risk-insights/models/report-models"; +import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; +import { TableDataSource, ToastService } from "@bitwarden/components"; + +import { ApplicationTableDataSource } from "../shared/app-table-row-scrollable.component"; + +import { AllApplicationsComponent } from "./all-applications.component"; + +// Mock ResizeObserver +global.ResizeObserver = class ResizeObserver { + observe() {} + unobserve() {} + disconnect() {} +}; + +// Helper type to access protected members in tests +type ComponentWithProtectedMembers = AllApplicationsComponent & { + dataSource: TableDataSource; + selectedUrls: WritableSignal>; + markingAsCritical: WritableSignal; +}; + +describe("AllApplicationsComponent", () => { + let component: AllApplicationsComponent; + let fixture: ComponentFixture; + let mockI18nService: MockProxy; + let mockToastService: MockProxy; + let mockDataService: MockProxy; + + const reportStatus$ = new BehaviorSubject(ReportStatus.Complete); + const enrichedReportData$ = new BehaviorSubject(null); + const drawerDetails$ = new BehaviorSubject({ + open: false, + invokerId: "", + activeDrawerType: DrawerType.None, + atRiskMemberDetails: [], + appAtRiskMembers: null, + atRiskAppDetails: null, + }); + + beforeEach(async () => { + mockI18nService = mock(); + mockToastService = mock(); + mockDataService = mock(); + + mockI18nService.t.mockImplementation((key: string) => key); + + Object.defineProperty(mockDataService, "reportStatus$", { get: () => reportStatus$ }); + Object.defineProperty(mockDataService, "enrichedReportData$", { + get: () => enrichedReportData$, + }); + Object.defineProperty(mockDataService, "drawerDetails$", { get: () => drawerDetails$ }); + + await TestBed.configureTestingModule({ + imports: [AllApplicationsComponent, ReactiveFormsModule], + providers: [ + { provide: I18nService, useValue: mockI18nService }, + { provide: ToastService, useValue: mockToastService }, + { provide: RiskInsightsDataService, useValue: mockDataService }, + { + provide: ActivatedRoute, + useValue: { + paramMap: of(convertToParamMap({})), + snapshot: { paramMap: convertToParamMap({}) }, + }, + }, + ], + }).compileComponents(); + + fixture = TestBed.createComponent(AllApplicationsComponent); + component = fixture.componentInstance; + fixture.detectChanges(); + }); + + afterEach(() => { + jest.clearAllMocks(); + }); + + it("should create", () => { + expect(component).toBeTruthy(); + }); + + describe("markAppsAsCritical", () => { + beforeEach(() => { + (component as ComponentWithProtectedMembers).selectedUrls.set(new Set(["GitHub", "Slack"])); + }); + + it("should show success toast when response has no error", async () => { + mockDataService.saveCriticalApplications.mockReturnValue( + of({ + status: ReportStatus.Complete, + error: null, + data: { + summaryData: { ...createNewSummaryData(), totalCriticalApplicationCount: 2 }, + }, + } as ReportState), + ); + + await component.markAppsAsCritical(); + + expect(mockToastService.showToast).toHaveBeenCalledWith( + expect.objectContaining({ variant: "success" }), + ); + expect((component as ComponentWithProtectedMembers).selectedUrls().size).toBe(0); + expect((component as ComponentWithProtectedMembers).markingAsCritical()).toBe(false); + }); + + it("should show error toast when response has error field set", async () => { + mockDataService.saveCriticalApplications.mockReturnValue( + of({ + status: ReportStatus.Complete, + error: "Failed to save critical applications", + data: null, + }), + ); + + await component.markAppsAsCritical(); + + expect(mockToastService.showToast).toHaveBeenCalledWith( + expect.objectContaining({ variant: "error" }), + ); + // Selection preserved on error + expect((component as ComponentWithProtectedMembers).selectedUrls().size).toBe(2); + expect((component as ComponentWithProtectedMembers).markingAsCritical()).toBe(false); + }); + }); +}); diff --git a/bitwarden_license/bit-web/src/app/dirt/access-intelligence/all-applications/all-applications.component.ts b/bitwarden_license/bit-web/src/app/dirt/access-intelligence/all-applications/all-applications.component.ts index dc5d44d2d2b..7ef15c93fc0 100644 --- a/bitwarden_license/bit-web/src/app/dirt/access-intelligence/all-applications/all-applications.component.ts +++ b/bitwarden_license/bit-web/src/app/dirt/access-intelligence/all-applications/all-applications.component.ts @@ -118,16 +118,27 @@ export class AllApplicationsComponent implements OnInit { .saveCriticalApplications(Array.from(this.selectedUrls())) .pipe(takeUntilDestroyed(this.destroyRef)) .subscribe({ - next: () => { + next: (response) => { + this.markingAsCritical.set(false); + + if (response.error) { + this.toastService.showToast({ + variant: "error", + title: "", + message: this.i18nService.t("applicationsMarkedAsCriticalFail"), + }); + return; + } + this.toastService.showToast({ variant: "success", title: "", message: this.i18nService.t("criticalApplicationsMarkedSuccess", count.toString()), }); this.selectedUrls.set(new Set()); - this.markingAsCritical.set(false); }, error: () => { + this.markingAsCritical.set(false); this.toastService.showToast({ variant: "error", title: "",