[PM-5971] Fix Payment Method Warning Bugs (#7923)

* Rework implementation of payment method warnings

* Move payment-method-warnings.component to module

* Moved timer/subscribe to app.component

* Remove unrelated refactoring

* Remaining feedback

* Add paymentMethodWarningsService tests

* Thomas' feedback

* fix tests

* Use barrel file imports

* Make banner work with new vault navigation

* Matt's feedback

libs/angular/src/services/jslib-services.module.ts

@@ -91,11 +91,11 @@ import {
   BadgeSettingsService,
 } from "@bitwarden/common/autofill/services/badge-settings.service";
 import { BillingApiServiceAbstraction } from "@bitwarden/common/billing/abstractions/billilng-api.service.abstraction";
-import { BillingBannerServiceAbstraction } from "@bitwarden/common/billing/abstractions/billing-banner.service.abstraction";
 import { OrganizationBillingServiceAbstraction } from "@bitwarden/common/billing/abstractions/organization-billing.service";
+import { PaymentMethodWarningsServiceAbstraction } from "@bitwarden/common/billing/abstractions/payment-method-warnings-service.abstraction";
 import { BillingApiService } from "@bitwarden/common/billing/services/billing-api.service";
-import { BillingBannerService } from "@bitwarden/common/billing/services/billing-banner.service";
 import { OrganizationBillingService } from "@bitwarden/common/billing/services/organization-billing.service";
+import { PaymentMethodWarningsService } from "@bitwarden/common/billing/services/payment-method-warnings.service";
 import { AppIdService as AppIdServiceAbstraction } from "@bitwarden/common/platform/abstractions/app-id.service";
 import { BroadcasterService as BroadcasterServiceAbstraction } from "@bitwarden/common/platform/abstractions/broadcaster.service";
 import { ConfigApiServiceAbstraction } from "@bitwarden/common/platform/abstractions/config/config-api.service.abstraction";
@@ -920,11 +920,6 @@ import { ModalService } from "./modal.service";
         DerivedStateProvider,
       ],
     },
-    {
-      provide: BillingBannerServiceAbstraction,
-      useClass: BillingBannerService,
-      deps: [StateProvider],
-    },
     {
       provide: OrganizationBillingServiceAbstraction,
       useClass: OrganizationBillingService,
@@ -933,6 +928,8 @@ import { ModalService } from "./modal.service";
         EncryptService,
         I18nServiceAbstraction,
         OrganizationApiServiceAbstraction,
+        OrganizationServiceAbstraction,
+        StateProvider,
       ],
     },
     {
@@ -969,6 +966,11 @@ import { ModalService } from "./modal.service";
       useClass: BillingApiService,
       deps: [ApiServiceAbstraction],
     },
+    {
+      provide: PaymentMethodWarningsServiceAbstraction,
+      useClass: PaymentMethodWarningsService,
+      deps: [BillingApiServiceAbstraction, StateProvider],
+    },
   ],
 })
 export class JslibServicesModule {}

libs/common/src/admin-console/abstractions/organization/organization-api.service.abstraction.ts

@@ -9,7 +9,6 @@ import { OrganizationTaxInfoUpdateRequest } from "../../../billing/models/reques
 import { PaymentRequest } from "../../../billing/models/request/payment.request";
 import { SecretsManagerSubscribeRequest } from "../../../billing/models/request/sm-subscribe.request";
 import { BillingResponse } from "../../../billing/models/response/billing.response";
-import { OrganizationRisksSubscriptionFailureResponse } from "../../../billing/models/response/organization-risks-subscription-failure.response";
 import { OrganizationSubscriptionResponse } from "../../../billing/models/response/organization-subscription.response";
 import { PaymentResponse } from "../../../billing/models/response/payment.response";
 import { TaxInfoResponse } from "../../../billing/models/response/tax-info.response";
@@ -79,6 +78,5 @@ export class OrganizationApiServiceAbstraction {
     id: string,
     request: OrganizationCollectionManagementUpdateRequest,
   ) => Promise<OrganizationResponse>;
-  risksSubscriptionFailure: (id: string) => Promise<OrganizationRisksSubscriptionFailureResponse>;
   enableCollectionEnhancements: (id: string) => Promise<void>;
 }

libs/common/src/admin-console/services/organization/organization-api.service.ts

@@ -10,7 +10,6 @@ import { OrganizationTaxInfoUpdateRequest } from "../../../billing/models/reques
 import { PaymentRequest } from "../../../billing/models/request/payment.request";
 import { SecretsManagerSubscribeRequest } from "../../../billing/models/request/sm-subscribe.request";
 import { BillingResponse } from "../../../billing/models/response/billing.response";
-import { OrganizationRisksSubscriptionFailureResponse } from "../../../billing/models/response/organization-risks-subscription-failure.response";
 import { OrganizationSubscriptionResponse } from "../../../billing/models/response/organization-subscription.response";
 import { PaymentResponse } from "../../../billing/models/response/payment.response";
 import { TaxInfoResponse } from "../../../billing/models/response/tax-info.response";
@@ -344,20 +343,6 @@ export class OrganizationApiService implements OrganizationApiServiceAbstraction
     return data;
   }
 
-  async risksSubscriptionFailure(
-    id: string,
-  ): Promise<OrganizationRisksSubscriptionFailureResponse> {
-    const r = await this.apiService.send(
-      "GET",
-      "/organizations/" + id + "/risks-subscription-failure",
-      null,
-      true,
-      true,
-    );
-
-    return new OrganizationRisksSubscriptionFailureResponse(r);
-  }
-
   async enableCollectionEnhancements(id: string): Promise<void> {
     await this.apiService.send(
       "POST",

libs/common/src/billing/abstractions/billilng-api.service.abstraction.ts

@@ -1,4 +1,5 @@
 import { SubscriptionCancellationRequest } from "../../billing/models/request/subscription-cancellation.request";
+import { OrganizationBillingStatusResponse } from "../../billing/models/response/organization-billing-status.response";
 
 export abstract class BillingApiServiceAbstraction {
   cancelOrganizationSubscription: (
@@ -6,4 +7,5 @@ export abstract class BillingApiServiceAbstraction {
     request: SubscriptionCancellationRequest,
   ) => Promise<void>;
   cancelPremiumUserSubscription: (request: SubscriptionCancellationRequest) => Promise<void>;
+  getBillingStatus: (id: string) => Promise<OrganizationBillingStatusResponse>;
 }

libs/common/src/billing/abstractions/billing-banner.service.abstraction.ts

@@ -1,6 +0,0 @@
-import { Observable } from "rxjs";
-
-export class BillingBannerServiceAbstraction {
-  paymentMethodBannerStates$: Observable<{ organizationId: string; visible: boolean }[]>;
-  setPaymentMethodBannerState: (organizationId: string, visible: boolean) => Promise<void>;
-}

libs/common/src/billing/abstractions/payment-method-warnings-service.abstraction.ts

@@ -0,0 +1,31 @@
+import { Observable } from "rxjs";
+
+import { PaymentMethodWarning } from "../models/domain/payment-method-warning";
+
+export abstract class PaymentMethodWarningsServiceAbstraction {
+  /**
+   * An {@link Observable} record in the {@link ActiveUserState} of the user's organization IDs each mapped to their respective {@link PaymentMethodWarning}.
+   */
+  paymentMethodWarnings$: Observable<Record<string, PaymentMethodWarning>>;
+  /**
+   * Updates the {@link ActiveUserState} by setting `acknowledged` to `true` for the {@link PaymentMethodWarning} represented by the provided organization ID.
+   * @param organizationId - The ID of the organization whose warning you'd like to acknowledge.
+   */
+  acknowledge: (organizationId: string) => Promise<void>;
+  /**
+   * Updates the {@link ActiveUserState} by setting `risksSubscriptionFailure` to `false` for the {@link PaymentMethodWarning} represented by the provided organization ID.
+   * @param organizationId - The ID of the organization whose subscription risk you'd like to remove.
+   */
+  removeSubscriptionRisk: (organizationId: string) => Promise<void>;
+  /**
+   * Clears the {@link PaymentMethodWarning} record from the {@link ActiveUserState}.
+   */
+  clear: () => Promise<void>;
+  /**
+   * Tries to retrieve the {@link PaymentMethodWarning} for the provided organization ID from the {@link ActiveUserState}.
+   * If the warning does not exist, or if the warning has been in state for longer than a week, fetches the current {@link OrganizationBillingStatusResponse} for the organization
+   * from the API and uses it to update the warning in state.
+   * @param organizationId - The ID of the organization whose {@link PaymentMethodWarning} you'd like to update.
+   */
+  update: (organizationId: string) => Promise<void>;
+}

libs/common/src/billing/models/billing-keys.state.ts

@@ -0,0 +1,13 @@
+import { BILLING_DISK, KeyDefinition } from "../../platform/state";
+import { PaymentMethodWarning } from "../models/domain/payment-method-warning";
+
+export const PAYMENT_METHOD_WARNINGS_KEY = KeyDefinition.record<PaymentMethodWarning>(
+  BILLING_DISK,
+  "paymentMethodWarnings",
+  {
+    deserializer: (warnings) => ({
+      ...warnings,
+      savedAt: new Date(warnings.savedAt),
+    }),
+  },
+);

libs/common/src/billing/models/domain/payment-method-warning.ts

@@ -0,0 +1,6 @@
+export type PaymentMethodWarning = {
+  organizationName: string;
+  risksSubscriptionFailure: boolean;
+  acknowledged: boolean;
+  savedAt: Date;
+};

libs/common/src/billing/models/response/organization-billing-status.response.ts

@@ -0,0 +1,15 @@
+import { BaseResponse } from "../../../models/response/base.response";
+
+export class OrganizationBillingStatusResponse extends BaseResponse {
+  organizationId: string;
+  organizationName: string;
+  risksSubscriptionFailure: boolean;
+
+  constructor(response: any) {
+    super(response);
+
+    this.organizationId = this.getResponseProperty("OrganizationId");
+    this.organizationName = this.getResponseProperty("OrganizationName");
+    this.risksSubscriptionFailure = this.getResponseProperty("RisksSubscriptionFailure");
+  }
+}

libs/common/src/billing/services/billing-api.service.ts

@@ -1,6 +1,7 @@
 import { ApiService } from "../../abstractions/api.service";
 import { BillingApiServiceAbstraction } from "../../billing/abstractions/billilng-api.service.abstraction";
 import { SubscriptionCancellationRequest } from "../../billing/models/request/subscription-cancellation.request";
+import { OrganizationBillingStatusResponse } from "../../billing/models/response/organization-billing-status.response";
 
 export class BillingApiService implements BillingApiServiceAbstraction {
   constructor(private apiService: ApiService) {}
@@ -21,4 +22,16 @@ export class BillingApiService implements BillingApiServiceAbstraction {
   cancelPremiumUserSubscription(request: SubscriptionCancellationRequest): Promise<void> {
     return this.apiService.send("POST", "/accounts/churn-premium", request, true, false);
   }
+
+  async getBillingStatus(id: string): Promise<OrganizationBillingStatusResponse> {
+    const r = await this.apiService.send(
+      "GET",
+      "/organizations/" + id + "/billing-status",
+      null,
+      true,
+      true,
+    );
+
+    return new OrganizationBillingStatusResponse(r);
+  }
 }

libs/common/src/billing/services/billing-banner.service.ts

@@ -1,44 +0,0 @@
-import { map, Observable } from "rxjs";
-
-import {
-  ActiveUserState,
-  BILLING_BANNERS_DISK,
-  KeyDefinition,
-  StateProvider,
-} from "../../platform/state";
-import { BillingBannerServiceAbstraction } from "../abstractions/billing-banner.service.abstraction";
-
-const PAYMENT_METHOD_BANNERS_KEY = KeyDefinition.record<boolean>(
-  BILLING_BANNERS_DISK,
-  "paymentMethodBanners",
-  {
-    deserializer: (b) => b,
-  },
-);
-
-export class BillingBannerService implements BillingBannerServiceAbstraction {
-  private paymentMethodBannerStates: ActiveUserState<Record<string, boolean>>;
-  paymentMethodBannerStates$: Observable<{ organizationId: string; visible: boolean }[]>;
-
-  constructor(private stateProvider: StateProvider) {
-    this.paymentMethodBannerStates = this.stateProvider.getActive(PAYMENT_METHOD_BANNERS_KEY);
-    this.paymentMethodBannerStates$ = this.paymentMethodBannerStates.state$.pipe(
-      map((billingBannerStates) =>
-        !billingBannerStates
-          ? []
-          : Object.entries(billingBannerStates).map(([organizationId, visible]) => ({
-              organizationId,
-              visible,
-            })),
-      ),
-    );
-  }
-
-  async setPaymentMethodBannerState(organizationId: string, visibility: boolean): Promise<void> {
-    await this.paymentMethodBannerStates.update((states) => {
-      states ??= {};
-      states[organizationId] = visibility;
-      return states;
-    });
-  }
-}

libs/common/src/billing/services/payment-method-warnings.service.spec.ts

@@ -0,0 +1,186 @@
+import { any, mock, MockProxy } from "jest-mock-extended";
+import { firstValueFrom } from "rxjs";
+
+import { FakeAccountService, FakeStateProvider, mockAccountServiceWith } from "../../../spec";
+import { FakeActiveUserState } from "../../../spec/fake-state";
+import { Utils } from "../../platform/misc/utils";
+import { UserId } from "../../types/guid";
+import { BillingApiServiceAbstraction as BillingApiService } from "../abstractions/billilng-api.service.abstraction";
+import { PAYMENT_METHOD_WARNINGS_KEY } from "../models/billing-keys.state";
+import { PaymentMethodWarning } from "../models/domain/payment-method-warning";
+import { OrganizationBillingStatusResponse } from "../models/response/organization-billing-status.response";
+
+import { PaymentMethodWarningsService } from "./payment-method-warnings.service";
+
+describe("Payment Method Warnings Service", () => {
+  let paymentMethodWarningsService: PaymentMethodWarningsService;
+  let billingApiService: MockProxy<BillingApiService>;
+
+  const mockUserId = Utils.newGuid() as UserId;
+  let accountService: FakeAccountService;
+  let stateProvider: FakeStateProvider;
+  let activeUserState: FakeActiveUserState<Record<string, PaymentMethodWarning>>;
+
+  function getPastDate(daysAgo: number) {
+    const date = new Date();
+    date.setDate(date.getDate() - daysAgo);
+    return date;
+  }
+
+  const getBillingStatusResponse = (organizationId: string) =>
+    new OrganizationBillingStatusResponse({
+      OrganizationId: organizationId,
+      OrganizationName: "Teams Organization",
+      RisksSubscriptionFailure: true,
+    });
+
+  beforeEach(() => {
+    accountService = mockAccountServiceWith(mockUserId);
+    stateProvider = new FakeStateProvider(accountService);
+    activeUserState = stateProvider.activeUser.getFake(PAYMENT_METHOD_WARNINGS_KEY);
+
+    billingApiService = mock<BillingApiService>();
+    paymentMethodWarningsService = new PaymentMethodWarningsService(
+      billingApiService,
+      stateProvider,
+    );
+  });
+
+  it("acknowledge", async () => {
+    const organizationId = "1";
+    const state: Record<string, PaymentMethodWarning> = {
+      [organizationId]: {
+        organizationName: "Teams Organization",
+        risksSubscriptionFailure: true,
+        acknowledged: false,
+        savedAt: getPastDate(3),
+      },
+    };
+    activeUserState.nextState(state);
+    await paymentMethodWarningsService.acknowledge(organizationId);
+    expect(await firstValueFrom(paymentMethodWarningsService.paymentMethodWarnings$)).toEqual({
+      [organizationId]: {
+        ...state[organizationId],
+        acknowledged: true,
+      },
+    });
+  });
+
+  it("clear", async () => {
+    const state: Record<string, PaymentMethodWarning> = {
+      "1": {
+        organizationName: "Teams Organization",
+        risksSubscriptionFailure: true,
+        acknowledged: false,
+        savedAt: getPastDate(3),
+      },
+    };
+    activeUserState.nextState(state);
+    await paymentMethodWarningsService.clear();
+    expect(await firstValueFrom(paymentMethodWarningsService.paymentMethodWarnings$)).toEqual({});
+  });
+
+  it("removeSubscriptionRisk", async () => {
+    const organizationId = "1";
+    const state: Record<string, PaymentMethodWarning> = {
+      [organizationId]: {
+        organizationName: "Teams Organization",
+        risksSubscriptionFailure: true,
+        acknowledged: false,
+        savedAt: getPastDate(3),
+      },
+    };
+    activeUserState.nextState(state);
+    await paymentMethodWarningsService.removeSubscriptionRisk(organizationId);
+    expect(await firstValueFrom(paymentMethodWarningsService.paymentMethodWarnings$)).toEqual({
+      [organizationId]: {
+        ...state[organizationId],
+        risksSubscriptionFailure: false,
+      },
+    });
+  });
+
+  describe("update", () => {
+    it("Does nothing if the stored payment method warning is less than a week old", async () => {
+      const organizationId = "1";
+      const state: Record<string, PaymentMethodWarning> = {
+        [organizationId]: {
+          organizationName: "Teams Organization",
+          risksSubscriptionFailure: true,
+          acknowledged: false,
+          savedAt: getPastDate(3),
+        },
+      };
+      activeUserState.nextState(state);
+      await paymentMethodWarningsService.update(organizationId);
+      expect(billingApiService.getBillingStatus).not.toHaveBeenCalled();
+    });
+
+    it("Retrieves the billing status from the API and uses it to update the state if the state is null", async () => {
+      const organizationId = "1";
+      activeUserState.nextState(null);
+      billingApiService.getBillingStatus.mockResolvedValue(
+        getBillingStatusResponse(organizationId),
+      );
+      await paymentMethodWarningsService.update(organizationId);
+      expect(await firstValueFrom(paymentMethodWarningsService.paymentMethodWarnings$)).toEqual({
+        [organizationId]: {
+          organizationName: "Teams Organization",
+          risksSubscriptionFailure: true,
+          acknowledged: false,
+          savedAt: any(),
+        },
+      });
+      expect(billingApiService.getBillingStatus).toHaveBeenCalledTimes(1);
+    });
+
+    it("Retrieves the billing status from the API and uses it to update the state if the stored warning is null", async () => {
+      const organizationId = "1";
+      activeUserState.nextState({
+        [organizationId]: null,
+      });
+      billingApiService.getBillingStatus.mockResolvedValue(
+        getBillingStatusResponse(organizationId),
+      );
+      await paymentMethodWarningsService.update(organizationId);
+      expect(await firstValueFrom(paymentMethodWarningsService.paymentMethodWarnings$)).toEqual({
+        [organizationId]: {
+          organizationName: "Teams Organization",
+          risksSubscriptionFailure: true,
+          acknowledged: false,
+          savedAt: any(),
+        },
+      });
+      expect(billingApiService.getBillingStatus).toHaveBeenCalledTimes(1);
+    });
+
+    it("Retrieves the billing status from the API and uses it to update the state if the stored warning is older than a week", async () => {
+      const organizationId = "1";
+      activeUserState.nextState({
+        [organizationId]: {
+          organizationName: "Teams Organization",
+          risksSubscriptionFailure: false,
+          acknowledged: false,
+          savedAt: getPastDate(10),
+        },
+      });
+      billingApiService.getBillingStatus.mockResolvedValue(
+        new OrganizationBillingStatusResponse({
+          OrganizationId: organizationId,
+          OrganizationName: "Teams Organization",
+          RisksSubscriptionFailure: true,
+        }),
+      );
+      await paymentMethodWarningsService.update(organizationId);
+      expect(await firstValueFrom(paymentMethodWarningsService.paymentMethodWarnings$)).toEqual({
+        [organizationId]: {
+          organizationName: "Teams Organization",
+          risksSubscriptionFailure: true,
+          acknowledged: false,
+          savedAt: any(),
+        },
+      });
+      expect(billingApiService.getBillingStatus).toHaveBeenCalledTimes(1);
+    });
+  });
+});

libs/common/src/billing/services/payment-method-warnings.service.ts

@@ -0,0 +1,74 @@
+import { firstValueFrom, map, Observable } from "rxjs";
+
+import { ActiveUserState, StateProvider } from "../../platform/state";
+import { BillingApiServiceAbstraction as BillingApiService } from "../abstractions/billilng-api.service.abstraction";
+import { PaymentMethodWarningsServiceAbstraction } from "../abstractions/payment-method-warnings-service.abstraction";
+import { PAYMENT_METHOD_WARNINGS_KEY } from "../models/billing-keys.state";
+import { PaymentMethodWarning } from "../models/domain/payment-method-warning";
+
+export class PaymentMethodWarningsService implements PaymentMethodWarningsServiceAbstraction {
+  private paymentMethodWarningsState: ActiveUserState<Record<string, PaymentMethodWarning>>;
+  paymentMethodWarnings$: Observable<Record<string, PaymentMethodWarning>>;
+
+  constructor(
+    private billingApiService: BillingApiService,
+    private stateProvider: StateProvider,
+  ) {
+    this.paymentMethodWarningsState = this.stateProvider.getActive(PAYMENT_METHOD_WARNINGS_KEY);
+    this.paymentMethodWarnings$ = this.paymentMethodWarningsState.state$;
+  }
+
+  async acknowledge(organizationId: string): Promise<void> {
+    await this.paymentMethodWarningsState.update((state) => {
+      const current = state[organizationId];
+      state[organizationId] = {
+        ...current,
+        acknowledged: true,
+      };
+      return state;
+    });
+  }
+
+  async removeSubscriptionRisk(organizationId: string): Promise<void> {
+    await this.paymentMethodWarningsState.update((state) => {
+      const current = state[organizationId];
+      state[organizationId] = {
+        ...current,
+        risksSubscriptionFailure: false,
+      };
+      return state;
+    });
+  }
+
+  async clear(): Promise<void> {
+    await this.paymentMethodWarningsState.update(() => ({}));
+  }
+
+  async update(organizationId: string): Promise<void> {
+    const warning = await firstValueFrom(
+      this.paymentMethodWarningsState.state$.pipe(
+        map((state) => (!state ? null : state[organizationId])),
+      ),
+    );
+    if (!warning || warning.savedAt < this.getOneWeekAgo()) {
+      const { organizationName, risksSubscriptionFailure } =
+        await this.billingApiService.getBillingStatus(organizationId);
+      await this.paymentMethodWarningsState.update((state) => {
+        state ??= {};
+        state[organizationId] = {
+          organizationName,
+          risksSubscriptionFailure,
+          acknowledged: false,
+          savedAt: new Date(),
+        };
+        return state;
+      });
+    }
+  }
+
+  private getOneWeekAgo = (): Date => {
+    const date = new Date();
+    date.setDate(date.getDate() - 7);
+    return date;
+  };
+}

libs/common/src/enums/feature-flag.enum.ts

@@ -8,6 +8,7 @@ export enum FeatureFlag {
   KeyRotationImprovements = "key-rotation-improvements",
   FlexibleCollectionsMigration = "flexible-collections-migration",
   AC1607_PresentUserOffboardingSurvey = "AC-1607_present-user-offboarding-survey",
+  ShowPaymentMethodWarningBanners = "show-payment-method-warning-banners",
 }
 
 // Replace this with a type safe lookup of the feature flag values in PM-2282

libs/common/src/platform/state/state-definitions.ts

@@ -19,8 +19,6 @@ import { StateDefinition } from "./state-definition";
 
 export const ACCOUNT_MEMORY = new StateDefinition("account", "memory");
 
-export const BILLING_BANNERS_DISK = new StateDefinition("billingBanners", "disk");
-
 export const CRYPTO_DISK = new StateDefinition("crypto", "disk");
 export const CRYPTO_MEMORY = new StateDefinition("crypto", "memory");
 
@@ -42,6 +40,8 @@ export const ORGANIZATIONS_DISK = new StateDefinition("organizations", "disk");
 export const POLICIES_DISK = new StateDefinition("policies", "disk");
 export const PROVIDERS_DISK = new StateDefinition("providers", "disk");
 
+export const BILLING_DISK = new StateDefinition("billing", "disk");
+
 export const FOLDER_DISK = new StateDefinition("folder", "disk", { web: "memory" });
 
 export const SYNC_STATE = new StateDefinition("sync", "disk", { web: "memory" });