From b1da2cb3d2676dda21239969ac9ba24b77495e8b Mon Sep 17 00:00:00 2001
From: Alex <adewitt@bitwarden.com>
Date: Wed, 29 Oct 2025 13:02:46 -0400
Subject: [PATCH] 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
---
 .../services/view/all-activities.service.ts   | 22 +++++--------------
 1 file changed, 5 insertions(+), 17 deletions(-)

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 4fa8a6779b9..695a3fc18f0 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,5 +1,5 @@
-import { BehaviorSubject, Subject } from "rxjs";
-import { takeUntil } from "rxjs/operators";
+import { takeUntilDestroyed } from "@angular/core/rxjs-interop";
+import { BehaviorSubject } from "rxjs";
 
 import { ApplicationHealthReportDetailEnriched } from "../../models";
 import { OrganizationReportSummary } from "../../models/report-models";
@@ -12,8 +12,6 @@ export class AllActivitiesService {
   /// Going forward, this class can be simplified by using the RiskInsightsDataService
   /// as it contains the application summary data.
 
-  private _destroy$ = new Subject<void>();
-
   private reportSummarySubject$ = new BehaviorSubject<OrganizationReportSummary>({
     totalMemberCount: 0,
     totalCriticalMemberCount: 0,
@@ -43,7 +41,7 @@ export class AllActivitiesService {
 
   constructor(private dataService: RiskInsightsDataService) {
     // All application summary changes
-    this.dataService.enrichedReportData$.pipe(takeUntil(this._destroy$)).subscribe((report) => {
+    this.dataService.enrichedReportData$.pipe(takeUntilDestroyed()).subscribe((report) => {
       if (report) {
         this.setAllAppsReportSummary(report.summaryData);
         this.setAllAppsReportDetails(report.reportData);
@@ -51,14 +49,14 @@ export class AllActivitiesService {
     });
 
     // Critical application summary changes
-    this.dataService.criticalReportResults$.pipe(takeUntil(this._destroy$)).subscribe((report) => {
+    this.dataService.criticalReportResults$.pipe(takeUntilDestroyed()).subscribe((report) => {
       if (report) {
         this.setCriticalAppsReportSummary(report.summaryData);
       }
     });
 
     // New applications changes (from orchestrator's reactive pipeline)
-    this.dataService.newApplications$.pipe(takeUntil(this._destroy$)).subscribe((newApps) => {
+    this.dataService.newApplications$.pipe(takeUntilDestroyed()).subscribe((newApps) => {
       this.setNewApplications(newApps);
     });
   }
@@ -111,14 +109,4 @@ export class AllActivitiesService {
       newApplications: newApps,
     });
   }
-
-  /**
-   * Cleanup method to prevent memory leaks.
-   * Should be called when the service is no longer needed.
-   * Complies with ADR-0003 (Observable Data Services) requirements.
-   */
-  destroy(): void {
-    this._destroy$.next();
-    this._destroy$.complete();
-  }
 }