From 2b009778e8a371cd609f6558c1f06799e32f5155 Mon Sep 17 00:00:00 2001 From: Alex <55413326+AlexRubik@users.noreply.github.com> Date: Thu, 30 Oct 2025 14:13:01 -0400 Subject: [PATCH] [PM-27284] new applications card real data (#17088) * feat(dirt): add newApplications$ observable to orchestrator Add reactive observable that filters applicationData for unreviewed apps (reviewedDate === null). Observable automatically updates when report state changes through the pipeline. - Add newApplications$ observable with distinctUntilChanged - Filters rawReportData$.data.applicationData - Uses shareReplay for multi-subscriber efficiency Related to PM-27284 * feat(dirt): add saveApplicationReviewStatus$ to orchestrator Implement method to save application review status and critical flags. Updates all applications where reviewedDate === null to set current date, and marks selected applications as critical. - Add saveApplicationReviewStatus$() method - Add _updateReviewStatusAndCriticalFlags() helper - Uses existing encryption and API update patterns - Single API call for both review status and critical flags - Follows same pattern as saveCriticalApplications$() Related to PM-27284 * feat(dirt): expose newApplications$ in data service Expose orchestrator's newApplications$ observable and save method through RiskInsightsDataService facade. Maintains clean separation between orchestrator (business logic) and components (UI). - Expose newApplications$ observable - Expose saveApplicationReviewStatus() delegation method - Maintains facade pattern consistency Related to PM-27284 * feat(dirt): make AllActivitiesService reactive to new applications Update AllActivitiesService to subscribe to orchestrator's newApplications$ observable instead of receiving data through summary updates. - Subscribe to dataService.newApplications$ in constructor - Add setNewApplications() helper method - Remove newApplications update from setAllAppsReportSummary() - New applications now update reactively when review status changes Related to PM-27284 * feat(dirt): connect dialog to review status save method Update NewApplicationsDialogComponent to call the data service's saveApplicationReviewStatus method when marking applications as critical. - Inject RiskInsightsDataService - Replace placeholder onMarkAsCritical() with real implementation - Handle success/error cases with appropriate toast notifications - Close dialog on successful save - Show different messages based on whether apps were marked critical Related to PM-27284 * feat(dirt): add i18n strings for application review Add internationalization strings for the new applications review dialog success and error messages. - applicationReviewSaved: Success toast title - applicationsMarkedAsCritical: Success message when apps marked critical - newApplicationsReviewed: Success message when apps reviewed only - errorSavingReviewStatus: Error toast title - pleaseTryAgain: Error toast message Related to PM-27284 * fix(dirt): add subscription cleanup to AllActivitiesService Critical fix for production code quality and memory leak prevention. Adds takeUntil pattern to all subscriptions to comply with ADR-0003 (Observable Data Services) requirements. **Subscription Cleanup (ADR-0003 Compliance):** - Add takeUntil pattern to AllActivitiesService subscriptions - Add _destroy$ Subject and destroy() method - Prevents memory leaks by properly unsubscribing from observables - Follows Observable Data Services ADR requirements Changes: - Import Subject and takeUntil from rxjs - Add private _destroy$ Subject for cleanup coordination - Apply takeUntil(this._destroy$) to all 3 subscriptions: - enrichedReportData$ subscription - criticalReportResults$ subscription - newApplications$ subscription - Add destroy() method for proper resource cleanup This ensures proper resource cleanup and follows Bitwarden's architectural decision records for observable management. Related to PM-27284 * fix(dirt): replace manual takeUntil with takeUntilDestroyed in AllActivitiesService Fixes critical memory leak by replacing manual subscription cleanup with Angular's automatic DestroyRef-based cleanup pattern. **Changes:** - Replace `takeUntil(this._destroy$)` with `takeUntilDestroyed()` for all 3 subscriptions - Remove unused `_destroy$` Subject and manual `destroy()` method - Update imports to use `@angular/core/rxjs-interop` **Why:** - Manual `destroy()` method was never called anywhere in codebase - Subscriptions accumulated without cleanup, causing memory leaks - `takeUntilDestroyed()` uses Angular's DestroyRef for automatic cleanup - Aligns with ADR-0003 and .claude/CLAUDE.md requirements **Impact:** - Automatic subscription cleanup when service context is destroyed - Prevents memory leaks during hot module reloads and route changes - Reduces code complexity (no manual lifecycle management needed) Related to PM-27284 * refactor(dirt): remove newApplications from OrganizationReportSummary Removes redundant newApplications field from summary type and uses derived newApplications$ observable from orchestrator instead. **Changes:** - Remove newApplications from OrganizationReportSummary type definition - Remove dummy data array from RiskInsightsReportService.getApplicationsSummary() - Remove newApplications subscription from AllActivitiesService - Update AllActivityComponent to subscribe directly to dataService.newApplications$ **Why:** - Eliminates data redundancy (stored vs derived) - newApplications$ already computes from applicationData.reviewedDate === null - Single source of truth: applicationData is the source - Simplifies encrypted payload (less data in summary) - Better separation: stored data (counts) vs computed data (lists) **Impact:** - No functional changes - UI continues to display new applications correctly - Cleaner architecture with computed observable pattern * cleanup * fix(dirt): improve dialog type safety and error logging Addresses critical PR review issues in NewApplicationsDialogComponent: **Type Safety:** - Replace unsafe type casting `(this as any).dialogRef` with proper DialogRef injection - Inject DialogRef using Angular's inject() function - Ensures type safety and prevents runtime errors from missing dialogRef **Error Handling:** - Add LogService to dialog component - Log errors with "[NewApplicationsDialog]" for debugging - Maintain user-facing error toast while adding server-side logging **Impact:** - Eliminates TypeScript safety bypasses - Improves production debugging capabilities - Follows Angular dependency injection best practices * fixing mock data and test cases for new apps * refactor(dirt): remove newApplications validation from OrganizationReportSummary type guard Removes redundant newApplications field validation from the OrganizationReportSummary type guard and related test cases. **Changes:** - Remove "newApplications" from allowed keys in isOrganizationReportSummary() - Remove newApplications array validation logic - Remove newApplications validation from validateOrganizationReportSummary() - Remove 2 test cases for newApplications validation - Remove newApplications field from 8 test data objects **Rationale:** The newApplications field was removed from OrganizationReportSummary type definition because it's derived data that can be calculated from applicationData (filtering where reviewedDate === null). The data is now accessed via the reactive newApplications$ observable instead of being stored redundantly in the summary object. **Impact:** - No functional changes - UI continues to display new applications via observable - Type guard now correctly validates the actual OrganizationReportSummary structure - Eliminates data redundancy and maintains single source of truth - All 43 tests passing --------- Co-authored-by: Tom --- apps/web/src/locales/en/messages.json | 21 +++ .../helpers/risk-insights-data-mappers.ts | 1 - .../risk-insights/models/mocks/mock-data.ts | 1 - .../risk-insights/models/report-models.ts | 1 - .../risk-insights-orchestrator.service.ts | 155 ++++++++++++++++++ .../risk-insights-report.service.spec.ts | 1 - .../domain/risk-insights-report.service.ts | 18 -- .../domain/risk-insights-type-guards.spec.ts | 45 ----- .../domain/risk-insights-type-guards.ts | 11 +- .../services/view/all-activities.service.ts | 8 +- .../view/risk-insights-data.service.ts | 8 + .../activity/all-activity.component.ts | 9 +- .../new-applications-dialog.component.ts | 47 ++++-- 13 files changed, 232 insertions(+), 94 deletions(-) diff --git a/apps/web/src/locales/en/messages.json b/apps/web/src/locales/en/messages.json index df67973ee5a..554d0eb31c9 100644 --- a/apps/web/src/locales/en/messages.json +++ b/apps/web/src/locales/en/messages.json @@ -328,6 +328,27 @@ "markAsCriticalPlaceholder": { "message": "Mark as critical functionality will be implemented in a future update" }, + "applicationReviewSaved": { + "message": "Application review saved" + }, + "applicationsMarkedAsCritical": { + "message": "$COUNT$ applications marked as critical", + "placeholders": { + "count": { + "content": "$1", + "example": "3" + } + } + }, + "newApplicationsReviewed": { + "message": "New applications reviewed" + }, + "errorSavingReviewStatus": { + "message": "Error saving review status" + }, + "pleaseTryAgain": { + "message": "Please try again" + }, "unmarkAsCritical": { "message": "Unmark as critical" }, diff --git a/bitwarden_license/bit-common/src/dirt/reports/risk-insights/helpers/risk-insights-data-mappers.ts b/bitwarden_license/bit-common/src/dirt/reports/risk-insights/helpers/risk-insights-data-mappers.ts index 624b695e6be..f7d635bb5fe 100644 --- a/bitwarden_license/bit-common/src/dirt/reports/risk-insights/helpers/risk-insights-data-mappers.ts +++ b/bitwarden_license/bit-common/src/dirt/reports/risk-insights/helpers/risk-insights-data-mappers.ts @@ -73,7 +73,6 @@ export function createNewSummaryData(): OrganizationReportSummary { totalCriticalAtRiskMemberCount: 0, totalCriticalApplicationCount: 0, totalCriticalAtRiskApplicationCount: 0, - newApplications: [], }; } export function getAtRiskApplicationList( diff --git a/bitwarden_license/bit-common/src/dirt/reports/risk-insights/models/mocks/mock-data.ts b/bitwarden_license/bit-common/src/dirt/reports/risk-insights/models/mocks/mock-data.ts index 1a30ad754c3..022b0aa42fb 100644 --- a/bitwarden_license/bit-common/src/dirt/reports/risk-insights/models/mocks/mock-data.ts +++ b/bitwarden_license/bit-common/src/dirt/reports/risk-insights/models/mocks/mock-data.ts @@ -77,7 +77,6 @@ export const mockSummaryData: OrganizationReportSummary = { totalCriticalAtRiskMemberCount: 1, totalCriticalApplicationCount: 1, totalCriticalAtRiskApplicationCount: 1, - newApplications: [], }; export const mockApplicationData: OrganizationReportApplication[] = [ { diff --git a/bitwarden_license/bit-common/src/dirt/reports/risk-insights/models/report-models.ts b/bitwarden_license/bit-common/src/dirt/reports/risk-insights/models/report-models.ts index 76892004d3e..497c10dbcad 100644 --- a/bitwarden_license/bit-common/src/dirt/reports/risk-insights/models/report-models.ts +++ b/bitwarden_license/bit-common/src/dirt/reports/risk-insights/models/report-models.ts @@ -55,7 +55,6 @@ export type OrganizationReportSummary = { totalCriticalMemberCount: number; totalCriticalAtRiskMemberCount: number; totalCriticalAtRiskApplicationCount: number; - newApplications: string[]; }; /** diff --git a/bitwarden_license/bit-common/src/dirt/reports/risk-insights/services/domain/risk-insights-orchestrator.service.ts b/bitwarden_license/bit-common/src/dirt/reports/risk-insights/services/domain/risk-insights-orchestrator.service.ts index 61b7c659977..00f8da6f797 100644 --- a/bitwarden_license/bit-common/src/dirt/reports/risk-insights/services/domain/risk-insights-orchestrator.service.ts +++ b/bitwarden_license/bit-common/src/dirt/reports/risk-insights/services/domain/risk-insights-orchestrator.service.ts @@ -91,6 +91,22 @@ export class RiskInsightsOrchestratorService { private _enrichedReportDataSubject = new BehaviorSubject(null); enrichedReportData$ = this._enrichedReportDataSubject.asObservable(); + // New applications that haven't been reviewed (reviewedDate === null) + newApplications$: Observable = this.rawReportData$.pipe( + map((reportState) => { + if (!reportState.data?.applicationData) { + return []; + } + return reportState.data.applicationData + .filter((app) => app.reviewedDate === null) + .map((app) => app.applicationName); + }), + distinctUntilChanged( + (prev, curr) => prev.length === curr.length && prev.every((app, i) => app === curr[i]), + ), + shareReplay({ bufferSize: 1, refCount: true }), + ); + // Generate report trigger and state private _generateReportTriggerSubject = new BehaviorSubject(false); generatingReport$ = this._generateReportTriggerSubject.asObservable(); @@ -341,6 +357,112 @@ export class RiskInsightsOrchestratorService { ); } + /** + * Saves review status for new applications and optionally marks selected ones as critical. + * This method: + * 1. Sets reviewedDate to current date for all applications where reviewedDate === null + * 2. Sets isCritical = true for applications in the selectedCriticalApps array + * + * @param selectedCriticalApps Array of application names to mark as critical (can be empty) + * @returns Observable of updated ReportState + */ + saveApplicationReviewStatus$(selectedCriticalApps: string[]): Observable { + this.logService.info("[RiskInsightsOrchestratorService] Saving application review status", { + criticalAppsCount: selectedCriticalApps.length, + }); + + return this.rawReportData$.pipe( + take(1), + filter((data) => !data.loading && data.data != null), + withLatestFrom( + this.organizationDetails$.pipe(filter((org) => !!org && !!org.organizationId)), + this._userId$.pipe(filter((userId) => !!userId)), + ), + map(([reportState, organizationDetails, userId]) => { + const existingApplicationData = reportState?.data?.applicationData || []; + const updatedApplicationData = this._updateReviewStatusAndCriticalFlags( + existingApplicationData, + selectedCriticalApps, + ); + + const updatedState = { + ...reportState, + data: { + ...reportState.data, + applicationData: updatedApplicationData, + }, + } as ReportState; + + this.logService.debug("[RiskInsightsOrchestratorService] Updated review status", { + totalApps: updatedApplicationData.length, + reviewedApps: updatedApplicationData.filter((app) => app.reviewedDate !== null).length, + criticalApps: updatedApplicationData.filter((app) => app.isCritical).length, + }); + + return { reportState, organizationDetails, updatedState, userId }; + }), + switchMap(({ reportState, organizationDetails, updatedState, userId }) => { + return from( + this.riskInsightsEncryptionService.encryptRiskInsightsReport( + { + organizationId: organizationDetails!.organizationId, + userId: userId!, + }, + { + reportData: reportState?.data?.reportData ?? [], + summaryData: reportState?.data?.summaryData ?? createNewSummaryData(), + applicationData: updatedState?.data?.applicationData ?? [], + }, + reportState?.data?.contentEncryptionKey, + ), + ).pipe( + map((encryptedData) => ({ + reportState, + organizationDetails, + updatedState, + encryptedData, + })), + ); + }), + switchMap(({ reportState, organizationDetails, updatedState, encryptedData }) => { + this.logService.debug( + `[RiskInsightsOrchestratorService] Persisting review status - report id: ${reportState?.data?.id}`, + ); + + if (!reportState?.data?.id || !organizationDetails?.organizationId) { + this.logService.warning( + "[RiskInsightsOrchestratorService] Cannot save review status - missing report id or org id", + ); + return of({ ...reportState }); + } + + return this.reportApiService + .updateRiskInsightsApplicationData$( + reportState.data.id, + organizationDetails.organizationId, + { + data: { + applicationData: encryptedData.encryptedApplicationData.toSdk(), + }, + }, + ) + .pipe( + map(() => updatedState), + tap((finalState) => { + this._markUnmarkUpdatesSubject.next(finalState); + }), + catchError((error: unknown) => { + this.logService.error( + "[RiskInsightsOrchestratorService] Failed to save review status", + error, + ); + return of({ ...reportState, error: "Failed to save application review status" }); + }), + ); + }), + ); + } + private _fetchReport$(organizationId: OrganizationId, userId: UserId): Observable { return this.reportService.getRiskInsightsReport$(organizationId, userId).pipe( tap(() => this.logService.debug("[RiskInsightsOrchestratorService] Fetching report")), @@ -501,6 +623,39 @@ export class RiskInsightsOrchestratorService { return updatedApps; } + /** + * Updates review status and critical flags for applications. + * Sets reviewedDate for all apps with null reviewedDate. + * Sets isCritical flag for apps in the criticalApplications array. + * + * @param existingApplications Current application data + * @param criticalApplications Array of application names to mark as critical + * @returns Updated application data with review dates and critical flags + */ + private _updateReviewStatusAndCriticalFlags( + existingApplications: OrganizationReportApplication[], + criticalApplications: string[], + ): OrganizationReportApplication[] { + const criticalSet = new Set(criticalApplications); + const currentDate = new Date(); + + return existingApplications.map((app) => { + const shouldMarkCritical = criticalSet.has(app.applicationName); + const needsReviewDate = app.reviewedDate === null; + + // Only create new object if changes are needed + if (needsReviewDate || shouldMarkCritical) { + return { + ...app, + reviewedDate: needsReviewDate ? currentDate : app.reviewedDate, + isCritical: shouldMarkCritical || app.isCritical, + }; + } + + return app; + }); + } + // Toggles the isCritical flag on applications via criticalApplicationName private _removeCriticalApplication( applicationData: OrganizationReportApplication[], diff --git a/bitwarden_license/bit-common/src/dirt/reports/risk-insights/services/domain/risk-insights-report.service.spec.ts b/bitwarden_license/bit-common/src/dirt/reports/risk-insights/services/domain/risk-insights-report.service.spec.ts index 3211b44322a..3061ea8cded 100644 --- a/bitwarden_license/bit-common/src/dirt/reports/risk-insights/services/domain/risk-insights-report.service.spec.ts +++ b/bitwarden_license/bit-common/src/dirt/reports/risk-insights/services/domain/risk-insights-report.service.spec.ts @@ -175,7 +175,6 @@ describe("RiskInsightsReportService", () => { totalCriticalAtRiskMemberCount: 1, totalCriticalApplicationCount: 1, totalCriticalAtRiskApplicationCount: 1, - newApplications: [], }, applicationData: [], }; diff --git a/bitwarden_license/bit-common/src/dirt/reports/risk-insights/services/domain/risk-insights-report.service.ts b/bitwarden_license/bit-common/src/dirt/reports/risk-insights/services/domain/risk-insights-report.service.ts index 470442a811b..47745b7e577 100644 --- a/bitwarden_license/bit-common/src/dirt/reports/risk-insights/services/domain/risk-insights-report.service.ts +++ b/bitwarden_license/bit-common/src/dirt/reports/risk-insights/services/domain/risk-insights-report.service.ts @@ -56,23 +56,6 @@ export class RiskInsightsReportService { const atRiskMembers = reports.flatMap((x) => x.atRiskMemberDetails); const uniqueAtRiskMembers = getUniqueMembers(atRiskMembers); - // TODO: Replace with actual new applications detection logic (PM-26185) - const dummyNewApplications = [ - "github.com", - "google.com", - "stackoverflow.com", - "gitlab.com", - "bitbucket.org", - "npmjs.com", - "docker.com", - "aws.amazon.com", - "azure.microsoft.com", - "jenkins.io", - "terraform.io", - "kubernetes.io", - "atlassian.net", - ]; - return { totalMemberCount: uniqueMembers.length, totalAtRiskMemberCount: uniqueAtRiskMembers.length, @@ -82,7 +65,6 @@ export class RiskInsightsReportService { totalCriticalAtRiskMemberCount: 0, totalCriticalApplicationCount: 0, totalCriticalAtRiskApplicationCount: 0, - newApplications: dummyNewApplications, }; } diff --git a/bitwarden_license/bit-common/src/dirt/reports/risk-insights/services/domain/risk-insights-type-guards.spec.ts b/bitwarden_license/bit-common/src/dirt/reports/risk-insights/services/domain/risk-insights-type-guards.spec.ts index 6c130dc77fa..3e32937d9e2 100644 --- a/bitwarden_license/bit-common/src/dirt/reports/risk-insights/services/domain/risk-insights-type-guards.spec.ts +++ b/bitwarden_license/bit-common/src/dirt/reports/risk-insights/services/domain/risk-insights-type-guards.spec.ts @@ -158,7 +158,6 @@ describe("Risk Insights Type Guards", () => { totalCriticalMemberCount: 4, totalCriticalAtRiskMemberCount: 1, totalCriticalAtRiskApplicationCount: 1, - newApplications: ["app-1", "app-2"], }; expect(() => validateOrganizationReportSummary(validData)).not.toThrow(); @@ -174,7 +173,6 @@ describe("Risk Insights Type Guards", () => { totalCriticalMemberCount: 4, totalCriticalAtRiskMemberCount: 1, totalCriticalAtRiskApplicationCount: 1, - newApplications: ["app-1"], }; expect(() => validateOrganizationReportSummary(invalidData)).toThrow( @@ -186,7 +184,6 @@ describe("Risk Insights Type Guards", () => { const invalidData = { totalMemberCount: 10, // missing multiple fields - newApplications: ["app-1"], }; expect(() => validateOrganizationReportSummary(invalidData)).toThrow( @@ -204,43 +201,6 @@ describe("Risk Insights Type Guards", () => { totalCriticalMemberCount: 4, totalCriticalAtRiskMemberCount: 1, totalCriticalAtRiskApplicationCount: 1, - newApplications: ["app-1"], - }; - - expect(() => validateOrganizationReportSummary(invalidData)).toThrow( - /Invalid OrganizationReportSummary/, - ); - }); - - it("should throw error for non-array newApplications", () => { - const invalidData = { - totalMemberCount: 10, - totalApplicationCount: 5, - totalAtRiskMemberCount: 2, - totalAtRiskApplicationCount: 1, - totalCriticalApplicationCount: 3, - totalCriticalMemberCount: 4, - totalCriticalAtRiskMemberCount: 1, - totalCriticalAtRiskApplicationCount: 1, - newApplications: "not-an-array", - }; - - expect(() => validateOrganizationReportSummary(invalidData)).toThrow( - /Invalid OrganizationReportSummary.*newApplications/, - ); - }); - - it("should throw error for empty string in newApplications", () => { - const invalidData = { - totalMemberCount: 10, - totalApplicationCount: 5, - totalAtRiskMemberCount: 2, - totalAtRiskApplicationCount: 1, - totalCriticalApplicationCount: 3, - totalCriticalMemberCount: 4, - totalCriticalAtRiskMemberCount: 1, - totalCriticalAtRiskApplicationCount: 1, - newApplications: ["app-1", "", "app-3"], // empty string }; expect(() => validateOrganizationReportSummary(invalidData)).toThrow( @@ -551,7 +511,6 @@ describe("Risk Insights Type Guards", () => { totalCriticalMemberCount: 4, totalCriticalAtRiskMemberCount: 1, totalCriticalAtRiskApplicationCount: 1, - newApplications: ["app-1"], }; expect(isOrganizationReportSummary(validData)).toBe(true); }); @@ -566,7 +525,6 @@ describe("Risk Insights Type Guards", () => { totalCriticalMemberCount: 4, totalCriticalAtRiskMemberCount: 1, totalCriticalAtRiskApplicationCount: 1, - newApplications: ["app-1"], }; expect(isOrganizationReportSummary(invalidData)).toBe(false); }); @@ -581,7 +539,6 @@ describe("Risk Insights Type Guards", () => { totalCriticalMemberCount: 4, totalCriticalAtRiskMemberCount: 1, totalCriticalAtRiskApplicationCount: 1, - newApplications: ["app-1"], }; expect(isOrganizationReportSummary(invalidData)).toBe(false); }); @@ -596,7 +553,6 @@ describe("Risk Insights Type Guards", () => { totalCriticalMemberCount: 4, totalCriticalAtRiskMemberCount: 1, totalCriticalAtRiskApplicationCount: 1, - newApplications: ["app-1"], }; expect(isOrganizationReportSummary(invalidData)).toBe(false); }); @@ -611,7 +567,6 @@ describe("Risk Insights Type Guards", () => { totalCriticalMemberCount: 4, totalCriticalAtRiskMemberCount: 1, totalCriticalAtRiskApplicationCount: 1, - newApplications: ["app-1"], extraField: "should be rejected", }; expect(isOrganizationReportSummary(invalidData)).toBe(false); diff --git a/bitwarden_license/bit-common/src/dirt/reports/risk-insights/services/domain/risk-insights-type-guards.ts b/bitwarden_license/bit-common/src/dirt/reports/risk-insights/services/domain/risk-insights-type-guards.ts index e3bcb3e18a2..c225586d6f7 100644 --- a/bitwarden_license/bit-common/src/dirt/reports/risk-insights/services/domain/risk-insights-type-guards.ts +++ b/bitwarden_license/bit-common/src/dirt/reports/risk-insights/services/domain/risk-insights-type-guards.ts @@ -181,7 +181,6 @@ export function isOrganizationReportSummary(obj: any): obj is OrganizationReport "totalCriticalMemberCount", "totalCriticalAtRiskMemberCount", "totalCriticalAtRiskApplicationCount", - "newApplications", ]; const actualKeys = Object.keys(obj); const hasUnexpectedProps = actualKeys.some((key) => !allowedKeys.includes(key)); @@ -229,12 +228,7 @@ export function isOrganizationReportSummary(obj: any): obj is OrganizationReport Number.isFinite(obj.totalCriticalAtRiskApplicationCount) && Number.isSafeInteger(obj.totalCriticalAtRiskApplicationCount) && obj.totalCriticalAtRiskApplicationCount >= 0 && - obj.totalCriticalAtRiskApplicationCount <= MAX_COUNT && - Array.isArray(obj.newApplications) && - obj.newApplications.length <= MAX_ARRAY_LENGTH && - obj.newApplications.every( - (app: any) => typeof app === "string" && app.length > 0 && app.length <= MAX_STRING_LENGTH, - ) + obj.totalCriticalAtRiskApplicationCount <= MAX_COUNT ); } @@ -346,9 +340,6 @@ export function validateOrganizationReportSummary(data: any): OrganizationReport if (typeof data?.totalCriticalAtRiskApplicationCount !== "number") { missingFields.push("totalCriticalAtRiskApplicationCount (number)"); } - if (!Array.isArray(data?.newApplications)) { - missingFields.push("newApplications (string[])"); - } throw new Error( `Invalid OrganizationReportSummary: ${missingFields.length > 0 ? `missing or invalid fields: ${missingFields.join(", ")}` : "structure validation failed"}`, diff --git a/bitwarden_license/bit-common/src/dirt/reports/risk-insights/services/view/all-activities.service.ts b/bitwarden_license/bit-common/src/dirt/reports/risk-insights/services/view/all-activities.service.ts index a25f2ba7fe8..22d8e24562d 100644 --- a/bitwarden_license/bit-common/src/dirt/reports/risk-insights/services/view/all-activities.service.ts +++ b/bitwarden_license/bit-common/src/dirt/reports/risk-insights/services/view/all-activities.service.ts @@ -1,3 +1,4 @@ +import { takeUntilDestroyed } from "@angular/core/rxjs-interop"; import { BehaviorSubject } from "rxjs"; import { ApplicationHealthReportDetailEnriched } from "../../models"; @@ -20,7 +21,6 @@ export class AllActivitiesService { totalCriticalApplicationCount: 0, totalAtRiskApplicationCount: 0, totalCriticalAtRiskApplicationCount: 0, - newApplications: [], }); reportSummary$ = this.reportSummarySubject$.asObservable(); @@ -40,14 +40,15 @@ export class AllActivitiesService { constructor(private dataService: RiskInsightsDataService) { // All application summary changes - this.dataService.enrichedReportData$.subscribe((report) => { + this.dataService.enrichedReportData$.pipe(takeUntilDestroyed()).subscribe((report) => { if (report) { this.setAllAppsReportSummary(report.summaryData); this.setAllAppsReportDetails(report.reportData); } }); + // Critical application summary changes - this.dataService.criticalReportResults$.subscribe((report) => { + this.dataService.criticalReportResults$.pipe(takeUntilDestroyed()).subscribe((report) => { if (report) { this.setCriticalAppsReportSummary(report.summaryData); } @@ -78,7 +79,6 @@ export class AllActivitiesService { totalAtRiskMemberCount: summary.totalAtRiskMemberCount, totalApplicationCount: summary.totalApplicationCount, totalAtRiskApplicationCount: summary.totalAtRiskApplicationCount, - newApplications: summary.newApplications, }); } diff --git a/bitwarden_license/bit-common/src/dirt/reports/risk-insights/services/view/risk-insights-data.service.ts b/bitwarden_license/bit-common/src/dirt/reports/risk-insights/services/view/risk-insights-data.service.ts index 6855274498a..4dee99ae044 100644 --- a/bitwarden_license/bit-common/src/dirt/reports/risk-insights/services/view/risk-insights-data.service.ts +++ b/bitwarden_license/bit-common/src/dirt/reports/risk-insights/services/view/risk-insights-data.service.ts @@ -29,6 +29,9 @@ export class RiskInsightsDataService { readonly isGeneratingReport$: Observable = of(false); readonly criticalReportResults$: Observable = of(null); + // New applications that need review (reviewedDate === null) + readonly newApplications$: Observable = of([]); + // ------------------------- Drawer Variables --------------------- // Drawer variables unified into a single BehaviorSubject private drawerDetailsSubject = new BehaviorSubject({ @@ -48,6 +51,7 @@ export class RiskInsightsDataService { this.organizationDetails$ = this.orchestrator.organizationDetails$; this.enrichedReportData$ = this.orchestrator.enrichedReportData$; this.criticalReportResults$ = this.orchestrator.criticalReportResults$; + this.newApplications$ = this.orchestrator.newApplications$; // Expose the loading state this.isLoading$ = this.reportState$.pipe( @@ -242,4 +246,8 @@ export class RiskInsightsDataService { removeCriticalApplication(hostname: string) { return this.orchestrator.removeCriticalApplication$(hostname); } + + saveApplicationReviewStatus(selectedCriticalApps: string[]) { + return this.orchestrator.saveApplicationReviewStatus$(selectedCriticalApps); + } } diff --git a/bitwarden_license/bit-web/src/app/dirt/access-intelligence/activity/all-activity.component.ts b/bitwarden_license/bit-web/src/app/dirt/access-intelligence/activity/all-activity.component.ts index 9689110866a..cca4b1c7a06 100644 --- a/bitwarden_license/bit-web/src/app/dirt/access-intelligence/activity/all-activity.component.ts +++ b/bitwarden_license/bit-web/src/app/dirt/access-intelligence/activity/all-activity.component.ts @@ -69,8 +69,13 @@ export class AllActivityComponent implements OnInit { this.totalCriticalAppsAtRiskMemberCount = summary.totalCriticalAtRiskMemberCount; this.totalCriticalAppsCount = summary.totalCriticalApplicationCount; this.totalCriticalAppsAtRiskCount = summary.totalCriticalAtRiskApplicationCount; - this.newApplications = summary.newApplications; - this.newApplicationsCount = summary.newApplications.length; + }); + + this.dataService.newApplications$ + .pipe(takeUntilDestroyed(this.destroyRef)) + .subscribe((newApps) => { + this.newApplications = newApps; + this.newApplicationsCount = newApps.length; }); this.allActivitiesService.passwordChangeProgressMetricHasProgressBar$ diff --git a/bitwarden_license/bit-web/src/app/dirt/access-intelligence/activity/new-applications-dialog.component.ts b/bitwarden_license/bit-web/src/app/dirt/access-intelligence/activity/new-applications-dialog.component.ts index 05b47da40ed..c9df3283fae 100644 --- a/bitwarden_license/bit-web/src/app/dirt/access-intelligence/activity/new-applications-dialog.component.ts +++ b/bitwarden_license/bit-web/src/app/dirt/access-intelligence/activity/new-applications-dialog.component.ts @@ -1,10 +1,14 @@ import { CommonModule } from "@angular/common"; import { Component, inject } from "@angular/core"; +import { firstValueFrom } from "rxjs"; +import { RiskInsightsDataService } from "@bitwarden/bit-common/dirt/reports/risk-insights"; import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; +import { LogService } from "@bitwarden/common/platform/abstractions/log.service"; import { ButtonModule, DialogModule, + DialogRef, DialogService, ToastService, TypographyModule, @@ -25,8 +29,11 @@ export class NewApplicationsDialogComponent { protected newApplications: string[] = []; protected selectedApplications: Set = new Set(); + private dialogRef = inject(DialogRef); + private dataService = inject(RiskInsightsDataService); private toastService = inject(ToastService); private i18nService = inject(I18nService); + private logService = inject(LogService); /** * Opens the new applications dialog @@ -35,7 +42,7 @@ export class NewApplicationsDialogComponent { * @returns Dialog reference */ static open(dialogService: DialogService, data: NewApplicationsDialogData) { - const ref = dialogService.open( + const ref = dialogService.open( NewApplicationsDialogComponent, { data, @@ -73,16 +80,34 @@ export class NewApplicationsDialogComponent { }; /** - * Placeholder handler for mark as critical functionality. - * Shows a toast notification with count of selected applications. - * TODO: Implement actual mark as critical functionality (PM-26203 follow-up) + * Handles the "Mark as Critical" button click. + * Saves review status for all new applications and marks selected ones as critical. + * Closes the dialog on success. */ - onMarkAsCritical = () => { - const selectedCount = this.selectedApplications.size; - this.toastService.showToast({ - variant: "info", - title: this.i18nService.t("markAsCritical"), - message: `${selectedCount} ${this.i18nService.t("applicationsSelected")}`, - }); + onMarkAsCritical = async () => { + const selectedCriticalApps = Array.from(this.selectedApplications); + + try { + await firstValueFrom(this.dataService.saveApplicationReviewStatus(selectedCriticalApps)); + + this.toastService.showToast({ + variant: "success", + title: this.i18nService.t("applicationReviewSaved"), + message: + selectedCriticalApps.length > 0 + ? this.i18nService.t("applicationsMarkedAsCritical", selectedCriticalApps.length) + : this.i18nService.t("newApplicationsReviewed"), + }); + + // Close dialog with success indicator + this.dialogRef.close(true); + } catch { + this.logService.error("[NewApplicationsDialog] Failed to save review status"); + this.toastService.showToast({ + variant: "error", + title: this.i18nService.t("errorSavingReviewStatus"), + message: this.i18nService.t("pleaseTryAgain"), + }); + } }; }