move state update logic from component to at risk password callout service

apps/browser/src/vault/popup/components/at-risk-callout/at-risk-password-callout.component.ts

@@ -1,5 +1,5 @@
 import { CommonModule } from "@angular/common";
-import { Component, inject, effect, signal, Signal, WritableSignal, computed } from "@angular/core";
+import { Component, inject, effect, signal, Signal, WritableSignal } from "@angular/core";
 import { toSignal } from "@angular/core/rxjs-interop";
 import { RouterModule } from "@angular/router";
 
@@ -30,15 +30,9 @@ export class AtRiskPasswordCalloutComponent {
   private atRiskPasswordCalloutService = inject(AtRiskPasswordCalloutService);
   private userIdSignal = toSignal(this.activeAccount$, { initialValue: null });
 
-  private atRiskPasswordStateSignal = toSignal(
-    this.atRiskPasswordCalloutService.atRiskPasswordState(this.userIdSignal()!).state$,
-    {
-      initialValue: {
-        hadPendingTasks: false,
-        showTasksCompleteBanner: false,
-        tasksBannerDismissed: false,
-      } as AtRiskPasswordCalloutData,
-    },
+  showTasksCompleteBanner = toSignal(
+    this.atRiskPasswordCalloutService.shouldShowCompletionBanner$(this.userIdSignal()!),
+    { initialValue: false },
   );
 
   currentPendingTasks: Signal<SecurityTask[] | null> = toSignal(
@@ -50,58 +44,24 @@ export class AtRiskPasswordCalloutComponent {
 
   dismissedClicked: WritableSignal<boolean> = signal(false);
 
-  showTasksCompleteBanner = computed(() => {
-    if (this.dismissedClicked()) {
-      return false;
-    }
-
-    const { showTasksCompleteBanner, hadPendingTasks } = this.atRiskPasswordStateSignal() ?? {};
-    const hasPendingTasks = (this.currentPendingTasks()?.length ?? 0) > 0;
-
-    return !hasPendingTasks && (showTasksCompleteBanner || hadPendingTasks);
-  });
-
   constructor() {
     effect(() => {
       const pendingTasksLength = this.currentPendingTasks()?.length ?? 0;
-      let updateObject: AtRiskPasswordCalloutData | null = null;
 
-      // If the user has resolved all tasks, we will show the banner
-      if (this.atRiskPasswordStateSignal()?.hadPendingTasks && pendingTasksLength === 0) {
-        updateObject = {
-          hadPendingTasks: false,
-          showTasksCompleteBanner: true,
-          tasksBannerDismissed: false,
-        };
-      }
-
-      // If user has pending tasks set state hadPendingTasks to true
-      if (pendingTasksLength > 0) {
-        updateObject = {
-          hadPendingTasks: true,
-          showTasksCompleteBanner: false,
-          tasksBannerDismissed: false,
-        };
-      }
-
-      if (updateObject) {
-        this.atRiskPasswordCalloutService.updateAtRiskPasswordState(
-          this.userIdSignal()!,
-          updateObject,
-        );
-      }
+      this.atRiskPasswordCalloutService.updatePendingTasksState(
+        this.userIdSignal()!,
+        pendingTasksLength,
+      );
     });
   }
 
   successBannerDismissed() {
-    // If the user dismisses the banner, we will update the state to reflect that
     const updateObject: AtRiskPasswordCalloutData = {
       hadPendingTasks: false,
       showTasksCompleteBanner: false,
       tasksBannerDismissed: true,
     };
     this.atRiskPasswordCalloutService.updateAtRiskPasswordState(this.userIdSignal()!, updateObject);
-
     this.dismissedClicked.set(true);
   }
 }

libs/vault/src/services/at-risk-password-callout.service.spec.ts

@@ -109,4 +109,144 @@ describe("AtRiskPasswordCalloutService", () => {
       expect(updater({})).toEqual(updateObj);
     });
   });
+
+  describe("shouldShowCompletionBanner$", () => {
+    beforeEach(() => {
+      jest.spyOn(mockTaskService, "pendingTasks$").mockReturnValue(of([]));
+      jest.spyOn(mockCipherService, "cipherViews$").mockReturnValue(of([]));
+    });
+
+    it("should return false when user has pending tasks", async () => {
+      const tasks = [{ id: "t1", cipherId: "c1", type: SecurityTaskType.UpdateAtRiskCredential }];
+      const ciphers = [new MockCipherView("c1", false)];
+      const state: AtRiskPasswordCalloutData = {
+        hadPendingTasks: true,
+        showTasksCompleteBanner: false,
+        tasksBannerDismissed: false,
+      };
+
+      jest.spyOn(mockTaskService, "pendingTasks$").mockReturnValue(of(tasks));
+      jest.spyOn(mockCipherService, "cipherViews$").mockReturnValue(of(ciphers));
+      mockStateProvider.getUser.mockReturnValue({ state$: of(state) });
+
+      const result = await firstValueFrom(service.shouldShowCompletionBanner$(userId));
+
+      expect(result).toBe(false);
+    });
+
+    it("should return true when no pending tasks and showTasksCompleteBanner is true", async () => {
+      const state: AtRiskPasswordCalloutData = {
+        hadPendingTasks: false,
+        showTasksCompleteBanner: true,
+        tasksBannerDismissed: false,
+      };
+
+      mockStateProvider.getUser.mockReturnValue({ state$: of(state) });
+
+      const result = await firstValueFrom(service.shouldShowCompletionBanner$(userId));
+
+      expect(result).toBe(true);
+    });
+
+    it("should return true when no pending tasks and hadPendingTasks is true", async () => {
+      const state: AtRiskPasswordCalloutData = {
+        hadPendingTasks: true,
+        showTasksCompleteBanner: false,
+        tasksBannerDismissed: false,
+      };
+
+      mockStateProvider.getUser.mockReturnValue({ state$: of(state) });
+
+      const result = await firstValueFrom(service.shouldShowCompletionBanner$(userId));
+
+      expect(result).toBe(true);
+    });
+
+    it("should return false when banner has been dismissed", async () => {
+      const state: AtRiskPasswordCalloutData = {
+        hadPendingTasks: false,
+        showTasksCompleteBanner: false,
+        tasksBannerDismissed: true,
+      };
+
+      mockStateProvider.getUser.mockReturnValue({ state$: of(state) });
+
+      const result = await firstValueFrom(service.shouldShowCompletionBanner$(userId));
+
+      expect(result).toBe(false);
+    });
+
+    it("should return false when state is null", async () => {
+      mockStateProvider.getUser.mockReturnValue({ state$: of(null) });
+
+      const result = await firstValueFrom(service.shouldShowCompletionBanner$(userId));
+
+      expect(result).toBe(false);
+    });
+  });
+
+  describe("updatePendingTasksState", () => {
+    it("should set showTasksCompleteBanner to true when user had tasks and resolved all of them", () => {
+      const currentState: AtRiskPasswordCalloutData = {
+        hadPendingTasks: true,
+        showTasksCompleteBanner: false,
+        tasksBannerDismissed: false,
+      };
+      const returnedState = {
+        state$: of(currentState),
+        update: jest.fn().mockResolvedValue(undefined),
+      };
+      mockStateProvider.getUser.mockReturnValue(returnedState);
+
+      service.updatePendingTasksState(userId, 0);
+
+      expect(returnedState.update).toHaveBeenCalledWith(expect.any(Function));
+      const updater = (returnedState.update as jest.Mock).mock.calls[0][0];
+      expect(updater()).toEqual({
+        hadPendingTasks: false,
+        showTasksCompleteBanner: true,
+        tasksBannerDismissed: false,
+      });
+    });
+
+    it("should set hadPendingTasks to true when user has tasks", () => {
+      const currentState: AtRiskPasswordCalloutData = {
+        hadPendingTasks: false,
+        showTasksCompleteBanner: false,
+        tasksBannerDismissed: false,
+      };
+      const returnedState = {
+        state$: of(currentState),
+        update: jest.fn().mockResolvedValue(undefined),
+      };
+      mockStateProvider.getUser.mockReturnValue(returnedState);
+
+      service.updatePendingTasksState(userId, 2);
+
+      expect(returnedState.update).toHaveBeenCalledWith(expect.any(Function));
+      const updater = (returnedState.update as jest.Mock).mock.calls[0][0];
+      expect(updater()).toEqual({
+        hadPendingTasks: true,
+        showTasksCompleteBanner: false,
+        tasksBannerDismissed: false,
+      });
+    });
+
+    it("should not update state when no changes made", () => {
+      const currentState: AtRiskPasswordCalloutData = {
+        hadPendingTasks: false,
+        showTasksCompleteBanner: false,
+        tasksBannerDismissed: false,
+      };
+      const returnedState = {
+        state$: of(currentState),
+        update: jest.fn().mockResolvedValue(undefined),
+      };
+      mockStateProvider.getUser.mockReturnValue(returnedState);
+
+      service.updatePendingTasksState(userId, 0);
+
+      expect(returnedState.update).not.toHaveBeenCalled();
+    });
+  });
 });

libs/vault/src/services/at-risk-password-callout.service.ts

@@ -1,5 +1,5 @@
 import { Injectable } from "@angular/core";
-import { combineLatest, map, Observable } from "rxjs";
+import { combineLatest, map, Observable, take, tap } from "rxjs";
 
 import {
   SingleUserState,
@@ -53,6 +53,30 @@ export class AtRiskPasswordCalloutService {
     );
   }
 
+  shouldShowCompletionBanner$(userId: UserId): Observable<boolean> {
+    return combineLatest([
+      this.pendingTasks$(userId),
+      this.atRiskPasswordState(userId).state$,
+    ]).pipe(
+      map(([pendingTasks, state]) => {
+        const hasPendingTasks = pendingTasks.length > 0;
+        const {
+          showTasksCompleteBanner = false,
+          hadPendingTasks = false,
+          tasksBannerDismissed = false,
+        } = state ?? {};
+
+        // Show banner if
+        // user has resolved all pending tasks
+        // user had showTasksCompleteBanner set to true and hasn't received new tasks
+        // user had showTasksCompleteBanner set to true and hasn't dismissed
+        return (
+          !hasPendingTasks && (showTasksCompleteBanner || hadPendingTasks) && !tasksBannerDismissed
+        );
+      }),
+    );
+  }
+
   atRiskPasswordState(userId: UserId): SingleUserState<AtRiskPasswordCalloutData> {
     return this.stateProvider.getUser(userId, AT_RISK_PASSWORD_CALLOUT_KEY);
   }
@@ -60,4 +84,35 @@ export class AtRiskPasswordCalloutService {
   updateAtRiskPasswordState(userId: UserId, updatedState: AtRiskPasswordCalloutData): void {
     void this.atRiskPasswordState(userId).update(() => updatedState);
   }
+
+  updatePendingTasksState(userId: UserId, currentTaskCount: number): void {
+    this.atRiskPasswordState(userId)
+      .state$.pipe(
+        take(1),
+        tap((currentState) => {
+          let updateObject: AtRiskPasswordCalloutData | null = null;
+
+          // If user had pending tasks and resolved all, show banner
+          if (currentState?.hadPendingTasks && currentTaskCount === 0) {
+            updateObject = {
+              hadPendingTasks: false,
+              showTasksCompleteBanner: true,
+              tasksBannerDismissed: false,
+            };
+            // If user has pending tasks, set hadPendingTasks to true
+          } else if (currentTaskCount > 0) {
+            updateObject = {
+              hadPendingTasks: true,
+              showTasksCompleteBanner: false,
+              tasksBannerDismissed: false,
+            };
+          }
+
+          if (updateObject) {
+            this.updateAtRiskPasswordState(userId, updateObject);
+          }
+        }),
+      )
+      .subscribe();
+  }
 }