Clear premium interest on upgrade dialog open (#17518)

apps/web/src/app/billing/individual/upgrade/unified-upgrade-dialog/unified-upgrade-dialog.component.spec.ts

@@ -1,4 +1,4 @@
-import { Component, input, output } from "@angular/core";
+import { ChangeDetectionStrategy, Component, input, output } from "@angular/core";
 import { ComponentFixture, TestBed } from "@angular/core/testing";
 import { NoopAnimationsModule } from "@angular/platform-browser/animations";
 import { Router } from "@angular/router";
@@ -28,12 +28,11 @@ import {
   UnifiedUpgradeDialogStep,
 } from "./unified-upgrade-dialog.component";
 
-// FIXME(https://bitwarden.atlassian.net/browse/CL-764): Migrate to OnPush
-// eslint-disable-next-line @angular-eslint/prefer-on-push-component-change-detection
 @Component({
   selector: "app-upgrade-account",
   template: "",
   standalone: true,
+  changeDetection: ChangeDetectionStrategy.OnPush,
 })
 class MockUpgradeAccountComponent {
   readonly dialogTitleMessageOverride = input<string | null>(null);
@@ -42,12 +41,11 @@ class MockUpgradeAccountComponent {
   closeClicked = output<UpgradeAccountStatus>();
 }
 
-// FIXME(https://bitwarden.atlassian.net/browse/CL-764): Migrate to OnPush
-// eslint-disable-next-line @angular-eslint/prefer-on-push-component-change-detection
 @Component({
   selector: "app-upgrade-payment",
   template: "",
   standalone: true,
+  changeDetection: ChangeDetectionStrategy.OnPush,
 })
 class MockUpgradePaymentComponent {
   readonly selectedPlanId = input<PersonalSubscriptionPricingTierId | null>(null);
@@ -77,10 +75,56 @@ describe("UnifiedUpgradeDialogComponent", () => {
     planSelectionStepTitleOverride: null,
   };
 
+  /**
+   * Helper function to create and configure a fresh component instance with custom dialog data
+   */
+  async function createComponentWithDialogData(
+    dialogData: UnifiedUpgradeDialogParams,
+    waitForStable = false,
+  ): Promise<{
+    fixture: ComponentFixture<UnifiedUpgradeDialogComponent>;
+    component: UnifiedUpgradeDialogComponent;
+  }> {
+    TestBed.resetTestingModule();
+    jest.clearAllMocks();
+
+    await TestBed.configureTestingModule({
+      imports: [NoopAnimationsModule, UnifiedUpgradeDialogComponent],
+      providers: [
+        { provide: DialogRef, useValue: mockDialogRef },
+        { provide: DIALOG_DATA, useValue: dialogData },
+        { provide: Router, useValue: mockRouter },
+        { provide: PremiumInterestStateService, useValue: mockPremiumInterestStateService },
+      ],
+    })
+      .overrideComponent(UnifiedUpgradeDialogComponent, {
+        remove: {
+          imports: [UpgradeAccountComponent, UpgradePaymentComponent],
+        },
+        add: {
+          imports: [MockUpgradeAccountComponent, MockUpgradePaymentComponent],
+        },
+      })
+      .compileComponents();
+
+    const newFixture = TestBed.createComponent(UnifiedUpgradeDialogComponent);
+    const newComponent = newFixture.componentInstance;
+    newFixture.detectChanges();
+
+    if (waitForStable) {
+      await newFixture.whenStable();
+    }
+
+    return { fixture: newFixture, component: newComponent };
+  }
+
   beforeEach(async () => {
     // Reset mocks
     jest.clearAllMocks();
 
+    // Default mock: no premium interest
+    mockPremiumInterestStateService.getPremiumInterest.mockResolvedValue(false);
+
     await TestBed.configureTestingModule({
       imports: [NoopAnimationsModule, UnifiedUpgradeDialogComponent],
       providers: [
@@ -117,49 +161,63 @@ describe("UnifiedUpgradeDialogComponent", () => {
   });
 
   it("should initialize with custom initial step", async () => {
-    TestBed.resetTestingModule();
-
     const customDialogData: UnifiedUpgradeDialogParams = {
       account: mockAccount,
       initialStep: UnifiedUpgradeDialogStep.Payment,
       selectedPlan: PersonalSubscriptionPricingTierIds.Premium,
     };
 
-    await TestBed.configureTestingModule({
+    const { component: customComponent } = await createComponentWithDialogData(customDialogData);
-      imports: [NoopAnimationsModule, UnifiedUpgradeDialogComponent],
-      providers: [
-        { provide: DialogRef, useValue: mockDialogRef },
-        { provide: DIALOG_DATA, useValue: customDialogData },
-        { provide: Router, useValue: mockRouter },
-        { provide: PremiumInterestStateService, useValue: mockPremiumInterestStateService },
-      ],
-    })
-      .overrideComponent(UnifiedUpgradeDialogComponent, {
-        remove: {
-          imports: [UpgradeAccountComponent, UpgradePaymentComponent],
-        },
-        add: {
-          imports: [MockUpgradeAccountComponent, MockUpgradePaymentComponent],
-        },
-      })
-      .compileComponents();
-
-    const customFixture = TestBed.createComponent(UnifiedUpgradeDialogComponent);
-    const customComponent = customFixture.componentInstance;
-    customFixture.detectChanges();
 
     expect(customComponent["step"]()).toBe(UnifiedUpgradeDialogStep.Payment);
     expect(customComponent["selectedPlan"]()).toBe(PersonalSubscriptionPricingTierIds.Premium);
   });
 
+  describe("ngOnInit premium interest handling", () => {
+    it("should check premium interest on initialization", async () => {
+      // Component already initialized in beforeEach
+      expect(mockPremiumInterestStateService.getPremiumInterest).toHaveBeenCalledWith(
+        mockAccount.id,
+      );
+    });
+
+    it("should set hasPremiumInterest signal and clear premium interest when it exists", async () => {
+      mockPremiumInterestStateService.getPremiumInterest.mockResolvedValue(true);
+      mockPremiumInterestStateService.clearPremiumInterest.mockResolvedValue(undefined);
+
+      const { component: customComponent } = await createComponentWithDialogData(
+        defaultDialogData,
+        true,
+      );
+
+      expect(mockPremiumInterestStateService.getPremiumInterest).toHaveBeenCalledWith(
+        mockAccount.id,
+      );
+      expect(mockPremiumInterestStateService.clearPremiumInterest).toHaveBeenCalledWith(
+        mockAccount.id,
+      );
+      expect(customComponent["hasPremiumInterest"]()).toBe(true);
+    });
+
+    it("should not set hasPremiumInterest signal or clear when premium interest does not exist", async () => {
+      mockPremiumInterestStateService.getPremiumInterest.mockResolvedValue(false);
+
+      const { component: customComponent } = await createComponentWithDialogData(defaultDialogData);
+
+      expect(mockPremiumInterestStateService.getPremiumInterest).toHaveBeenCalledWith(
+        mockAccount.id,
+      );
+      expect(mockPremiumInterestStateService.clearPremiumInterest).not.toHaveBeenCalled();
+      expect(customComponent["hasPremiumInterest"]()).toBe(false);
+    });
+  });
+
   describe("custom dialog title", () => {
     it("should use null as default when no override is provided", () => {
       expect(component["planSelectionStepTitleOverride"]()).toBeNull();
     });
 
     it("should use custom title when provided in dialog config", async () => {
-      TestBed.resetTestingModule();
-
       const customDialogData: UnifiedUpgradeDialogParams = {
         account: mockAccount,
         initialStep: UnifiedUpgradeDialogStep.PlanSelection,
@@ -167,28 +225,7 @@ describe("UnifiedUpgradeDialogComponent", () => {
         planSelectionStepTitleOverride: "upgradeYourPlan",
       };
 
-      await TestBed.configureTestingModule({
+      const { component: customComponent } = await createComponentWithDialogData(customDialogData);
-        imports: [NoopAnimationsModule, UnifiedUpgradeDialogComponent],
-        providers: [
-          { provide: DialogRef, useValue: mockDialogRef },
-          { provide: DIALOG_DATA, useValue: customDialogData },
-          { provide: Router, useValue: mockRouter },
-          { provide: PremiumInterestStateService, useValue: mockPremiumInterestStateService },
-        ],
-      })
-        .overrideComponent(UnifiedUpgradeDialogComponent, {
-          remove: {
-            imports: [UpgradeAccountComponent, UpgradePaymentComponent],
-          },
-          add: {
-            imports: [MockUpgradeAccountComponent, MockUpgradePaymentComponent],
-          },
-        })
-        .compileComponents();
-
-      const customFixture = TestBed.createComponent(UnifiedUpgradeDialogComponent);
-      const customComponent = customFixture.componentInstance;
-      customFixture.detectChanges();
 
       expect(customComponent["planSelectionStepTitleOverride"]()).toBe("upgradeYourPlan");
     });
@@ -221,8 +258,6 @@ describe("UnifiedUpgradeDialogComponent", () => {
     });
 
     it("should be set to true when provided in dialog config", async () => {
-      TestBed.resetTestingModule();
-
       const customDialogData: UnifiedUpgradeDialogParams = {
         account: mockAccount,
         initialStep: null,
@@ -230,108 +265,32 @@ describe("UnifiedUpgradeDialogComponent", () => {
         hideContinueWithoutUpgradingButton: true,
       };
 
-      await TestBed.configureTestingModule({
+      const { component: customComponent } = await createComponentWithDialogData(customDialogData);
-        imports: [NoopAnimationsModule, UnifiedUpgradeDialogComponent],
-        providers: [
-          { provide: DialogRef, useValue: mockDialogRef },
-          { provide: DIALOG_DATA, useValue: customDialogData },
-          { provide: Router, useValue: mockRouter },
-          { provide: PremiumInterestStateService, useValue: mockPremiumInterestStateService },
-        ],
-      })
-        .overrideComponent(UnifiedUpgradeDialogComponent, {
-          remove: {
-            imports: [UpgradeAccountComponent, UpgradePaymentComponent],
-          },
-          add: {
-            imports: [MockUpgradeAccountComponent, MockUpgradePaymentComponent],
-          },
-        })
-        .compileComponents();
-
-      const customFixture = TestBed.createComponent(UnifiedUpgradeDialogComponent);
-      const customComponent = customFixture.componentInstance;
-      customFixture.detectChanges();
 
       expect(customComponent["hideContinueWithoutUpgradingButton"]()).toBe(true);
     });
   });
 
-  describe("onComplete with premium interest", () => {
+  describe("onComplete", () => {
-    it("should check premium interest, clear it, and route to /vault when premium interest exists", async () => {
+    it("should route to /vault when upgrading to premium with premium interest", async () => {
+      // Set up component with premium interest
       mockPremiumInterestStateService.getPremiumInterest.mockResolvedValue(true);
-      mockPremiumInterestStateService.clearPremiumInterest.mockResolvedValue();
+      mockPremiumInterestStateService.clearPremiumInterest.mockResolvedValue(undefined);
       mockRouter.navigate.mockResolvedValue(true);
 
-      const result: UpgradePaymentResult = {
+      const { component: customComponent } = await createComponentWithDialogData(
-        status: "upgradedToPremium",
+        defaultDialogData,
-        organizationId: null,
+        true,
-      };
+      );
-
-      await component["onComplete"](result);
 
+      // Premium interest should be set and cleared during ngOnInit
       expect(mockPremiumInterestStateService.getPremiumInterest).toHaveBeenCalledWith(
         mockAccount.id,
       );
       expect(mockPremiumInterestStateService.clearPremiumInterest).toHaveBeenCalledWith(
         mockAccount.id,
       );
-      expect(mockRouter.navigate).toHaveBeenCalledWith(["/vault"]);
+      expect(customComponent["hasPremiumInterest"]()).toBe(true);
-      expect(mockDialogRef.close).toHaveBeenCalledWith({
-        status: "upgradedToPremium",
-        organizationId: null,
-      });
-    });
-
-    it("should not clear premium interest when upgrading to families", async () => {
-      const result: UpgradePaymentResult = {
-        status: "upgradedToFamilies",
-        organizationId: "org-123",
-      };
-
-      await component["onComplete"](result);
-
-      expect(mockPremiumInterestStateService.getPremiumInterest).not.toHaveBeenCalled();
-      expect(mockPremiumInterestStateService.clearPremiumInterest).not.toHaveBeenCalled();
-      expect(mockDialogRef.close).toHaveBeenCalledWith({
-        status: "upgradedToFamilies",
-        organizationId: "org-123",
-      });
-    });
-
-    it("should use standard redirect when no premium interest exists", async () => {
-      TestBed.resetTestingModule();
-
-      const customDialogData: UnifiedUpgradeDialogParams = {
-        account: mockAccount,
-        redirectOnCompletion: true,
-      };
-
-      mockPremiumInterestStateService.getPremiumInterest.mockResolvedValue(false);
-      mockRouter.navigate.mockResolvedValue(true);
-
-      await TestBed.configureTestingModule({
-        imports: [NoopAnimationsModule, UnifiedUpgradeDialogComponent],
-        providers: [
-          { provide: DialogRef, useValue: mockDialogRef },
-          { provide: DIALOG_DATA, useValue: customDialogData },
-          { provide: Router, useValue: mockRouter },
-          { provide: PremiumInterestStateService, useValue: mockPremiumInterestStateService },
-        ],
-      })
-        .overrideComponent(UnifiedUpgradeDialogComponent, {
-          remove: {
-            imports: [UpgradeAccountComponent, UpgradePaymentComponent],
-          },
-          add: {
-            imports: [MockUpgradeAccountComponent, MockUpgradePaymentComponent],
-          },
-        })
-        .compileComponents();
-
-      const customFixture = TestBed.createComponent(UnifiedUpgradeDialogComponent);
-      const customComponent = customFixture.componentInstance;
-      customFixture.detectChanges();
 
       const result: UpgradePaymentResult = {
         status: "upgradedToPremium",
@@ -340,10 +299,55 @@ describe("UnifiedUpgradeDialogComponent", () => {
 
       await customComponent["onComplete"](result);
 
-      expect(mockPremiumInterestStateService.getPremiumInterest).toHaveBeenCalledWith(
+      // Should route to /vault because hasPremiumInterest signal is true
-        mockAccount.id,
+      // No additional service calls should be made in onComplete
-      );
+      expect(mockPremiumInterestStateService.getPremiumInterest).toHaveBeenCalledTimes(1); // Only from ngOnInit
-      expect(mockPremiumInterestStateService.clearPremiumInterest).not.toHaveBeenCalled();
+      expect(mockPremiumInterestStateService.clearPremiumInterest).toHaveBeenCalledTimes(1); // Only from ngOnInit
+      expect(mockRouter.navigate).toHaveBeenCalledWith(["/vault"]);
+      expect(mockDialogRef.close).toHaveBeenCalledWith({
+        status: "upgradedToPremium",
+        organizationId: null,
+      });
+    });
+
+    it("should close dialog when upgrading to families (premium interest not relevant)", async () => {
+      const result: UpgradePaymentResult = {
+        status: "upgradedToFamilies",
+        organizationId: "org-123",
+      };
+
+      await component["onComplete"](result);
+
+      // Premium interest logic only runs for premium upgrades, not families
+      expect(mockDialogRef.close).toHaveBeenCalledWith({
+        status: "upgradedToFamilies",
+        organizationId: "org-123",
+      });
+    });
+
+    it("should use standard redirect when upgrading to premium without premium interest", async () => {
+      const customDialogData: UnifiedUpgradeDialogParams = {
+        account: mockAccount,
+        redirectOnCompletion: true,
+      };
+
+      // No premium interest
+      mockPremiumInterestStateService.getPremiumInterest.mockResolvedValue(false);
+      mockRouter.navigate.mockResolvedValue(true);
+
+      const { component: customComponent } = await createComponentWithDialogData(customDialogData);
+
+      // Verify no premium interest was set during ngOnInit
+      expect(customComponent["hasPremiumInterest"]()).toBe(false);
+
+      const result: UpgradePaymentResult = {
+        status: "upgradedToPremium",
+        organizationId: null,
+      };
+
+      await customComponent["onComplete"](result);
+
+      // Should use standard redirect because hasPremiumInterest signal is false
       expect(mockRouter.navigate).toHaveBeenCalledWith([
         "/settings/subscription/user-subscription",
       ]);
@@ -354,70 +358,44 @@ describe("UnifiedUpgradeDialogComponent", () => {
     });
   });
 
-  describe("onCloseClicked with premium interest", () => {
+  describe("onCloseClicked", () => {
-    it("should clear premium interest when modal is closed", async () => {
+    it("should close dialog without clearing premium interest (cleared in ngOnInit)", async () => {
-      mockPremiumInterestStateService.clearPremiumInterest.mockResolvedValue();
-
       await component["onCloseClicked"]();
 
-      expect(mockPremiumInterestStateService.clearPremiumInterest).toHaveBeenCalledWith(
+      // Premium interest should have been cleared only once during ngOnInit, not again here
-        mockAccount.id,
+      expect(mockPremiumInterestStateService.clearPremiumInterest).toHaveBeenCalledTimes(0);
-      );
       expect(mockDialogRef.close).toHaveBeenCalledWith({ status: "closed" });
     });
   });
 
-  describe("previousStep with premium interest", () => {
+  describe("previousStep", () => {
-    it("should NOT clear premium interest when navigating between steps", async () => {
+    it("should go back to plan selection when on payment step", async () => {
       component["step"].set(UnifiedUpgradeDialogStep.Payment);
       component["selectedPlan"].set(PersonalSubscriptionPricingTierIds.Premium);
 
       await component["previousStep"]();
 
-      expect(mockPremiumInterestStateService.clearPremiumInterest).not.toHaveBeenCalled();
       expect(component["step"]()).toBe(UnifiedUpgradeDialogStep.PlanSelection);
       expect(component["selectedPlan"]()).toBeNull();
+      expect(mockPremiumInterestStateService.clearPremiumInterest).toHaveBeenCalledTimes(0);
     });
 
-    it("should clear premium interest when backing out of dialog completely", async () => {
+    it("should close dialog when backing out from plan selection step (no premium interest cleared)", async () => {
-      TestBed.resetTestingModule();
-
       const customDialogData: UnifiedUpgradeDialogParams = {
         account: mockAccount,
         initialStep: UnifiedUpgradeDialogStep.Payment,
         selectedPlan: PersonalSubscriptionPricingTierIds.Premium,
       };
 
-      mockPremiumInterestStateService.clearPremiumInterest.mockResolvedValue();
+      mockPremiumInterestStateService.getPremiumInterest.mockResolvedValue(false);
 
-      await TestBed.configureTestingModule({
+      const { component: customComponent } = await createComponentWithDialogData(customDialogData);
-        imports: [NoopAnimationsModule, UnifiedUpgradeDialogComponent],
-        providers: [
-          { provide: DialogRef, useValue: mockDialogRef },
-          { provide: DIALOG_DATA, useValue: customDialogData },
-          { provide: Router, useValue: mockRouter },
-          { provide: PremiumInterestStateService, useValue: mockPremiumInterestStateService },
-        ],
-      })
-        .overrideComponent(UnifiedUpgradeDialogComponent, {
-          remove: {
-            imports: [UpgradeAccountComponent, UpgradePaymentComponent],
-          },
-          add: {
-            imports: [MockUpgradeAccountComponent, MockUpgradePaymentComponent],
-          },
-        })
-        .compileComponents();
-
-      const customFixture = TestBed.createComponent(UnifiedUpgradeDialogComponent);
-      const customComponent = customFixture.componentInstance;
-      customFixture.detectChanges();
 
+      // Start at payment step, go back once to reach plan selection, then go back again to close
       await customComponent["previousStep"]();
 
-      expect(mockPremiumInterestStateService.clearPremiumInterest).toHaveBeenCalledWith(
+      // Premium interest cleared only in ngOnInit, not in previousStep
-        mockAccount.id,
+      expect(mockPremiumInterestStateService.clearPremiumInterest).toHaveBeenCalledTimes(0);
-      );
       expect(mockDialogRef.close).toHaveBeenCalledWith({ status: "closed" });
     });
   });

apps/web/src/app/billing/individual/upgrade/unified-upgrade-dialog/unified-upgrade-dialog.component.ts

@@ -1,6 +1,6 @@
 import { DIALOG_DATA } from "@angular/cdk/dialog";
 import { CommonModule } from "@angular/common";
-import { Component, Inject, OnInit, signal } from "@angular/core";
+import { ChangeDetectionStrategy, Component, Inject, OnInit, signal } from "@angular/core";
 import { Router } from "@angular/router";
 
 import { PremiumInterestStateService } from "@bitwarden/angular/billing/services/premium-interest/premium-interest-state.service.abstraction";
@@ -63,10 +63,9 @@ export type UnifiedUpgradeDialogParams = {
   redirectOnCompletion?: boolean;
 };
 
-// FIXME(https://bitwarden.atlassian.net/browse/CL-764): Migrate to OnPush
-// eslint-disable-next-line @angular-eslint/prefer-on-push-component-change-detection
 @Component({
   selector: "app-unified-upgrade-dialog",
+  changeDetection: ChangeDetectionStrategy.OnPush,
   imports: [
     CommonModule,
     DialogModule,
@@ -87,6 +86,7 @@ export class UnifiedUpgradeDialogComponent implements OnInit {
   protected readonly account = signal<Account | null>(null);
   protected readonly planSelectionStepTitleOverride = signal<string | null>(null);
   protected readonly hideContinueWithoutUpgradingButton = signal<boolean>(false);
+  protected readonly hasPremiumInterest = signal(false);
 
   protected readonly PaymentStep = UnifiedUpgradeDialogStep.Payment;
   protected readonly PlanSelectionStep = UnifiedUpgradeDialogStep.PlanSelection;
@@ -98,7 +98,7 @@ export class UnifiedUpgradeDialogComponent implements OnInit {
     private premiumInterestStateService: PremiumInterestStateService,
   ) {}
 
-  ngOnInit(): void {
+  async ngOnInit(): Promise<void> {
     this.account.set(this.params.account);
     this.step.set(this.params.initialStep ?? UnifiedUpgradeDialogStep.PlanSelection);
     this.selectedPlan.set(this.params.selectedPlan ?? null);
@@ -106,6 +106,19 @@ export class UnifiedUpgradeDialogComponent implements OnInit {
     this.hideContinueWithoutUpgradingButton.set(
       this.params.hideContinueWithoutUpgradingButton ?? false,
     );
+
+    /*
+     * Check if the user has premium interest at the point we open the dialog.
+     * If they do, record it on a component-level signal and clear the user's premium interest.
+     * This prevents us from having to clear it at every dialog conclusion point.
+     * */
+    const hasPremiumInterest = await this.premiumInterestStateService.getPremiumInterest(
+      this.params.account.id,
+    );
+    if (hasPremiumInterest) {
+      this.hasPremiumInterest.set(true);
+      await this.premiumInterestStateService.clearPremiumInterest(this.params.account.id);
+    }
   }
 
   protected onPlanSelected(planId: PersonalSubscriptionPricingTierId): void {
@@ -113,8 +126,6 @@ export class UnifiedUpgradeDialogComponent implements OnInit {
     this.nextStep();
   }
   protected async onCloseClicked(): Promise<void> {
-    // Clear premium interest when user closes/abandons modal
-    await this.premiumInterestStateService.clearPremiumInterest(this.params.account.id);
     this.close({ status: UnifiedUpgradeDialogStatus.Closed });
   }
 
@@ -135,8 +146,6 @@ export class UnifiedUpgradeDialogComponent implements OnInit {
       this.step.set(UnifiedUpgradeDialogStep.PlanSelection);
       this.selectedPlan.set(null);
     } else {
-      // Clear premium interest when backing out of dialog completely
-      await this.premiumInterestStateService.clearPremiumInterest(this.params.account.id);
       this.close({ status: UnifiedUpgradeDialogStatus.Closed });
     }
   }
@@ -161,11 +170,7 @@ export class UnifiedUpgradeDialogComponent implements OnInit {
 
     // Check premium interest and route to vault for marketing-initiated premium upgrades
     if (status === UnifiedUpgradeDialogStatus.UpgradedToPremium) {
-      const hasPremiumInterest = await this.premiumInterestStateService.getPremiumInterest(
+      if (this.hasPremiumInterest()) {
-        this.params.account.id,
-      );
-      if (hasPremiumInterest) {
-        await this.premiumInterestStateService.clearPremiumInterest(this.params.account.id);
         await this.router.navigate(["/vault"]);
         return; // Exit early, don't use redirectOnCompletion
       }