From b964cfc8e4a1ac6ce4c6376b1d3c70382bada8dc Mon Sep 17 00:00:00 2001 From: Alex Morask <144709477+amorask-bitwarden@users.noreply.github.com> Date: Wed, 25 Feb 2026 08:25:24 -0600 Subject: [PATCH] [PM-32612] Only show subscription menu option when premium user has subscription (#19209) * fix(billing): only show Subscription menu option when premium user has subscription * fix(billing): missed state service invocation changes --- .../browser/src/background/main.background.ts | 2 - .../service-container/service-container.ts | 2 - .../premium/cloud-hosted-premium.component.ts | 26 +++++- .../src/app/layouts/user-layout.component.ts | 51 +++++++++-- .../src/services/jslib-services.module.ts | 2 +- .../billing-account-profile-state.service.ts | 7 -- ...ling-account-profile-state.service.spec.ts | 90 +------------------ .../billing-account-profile-state.service.ts | 39 ++------ 8 files changed, 75 insertions(+), 144 deletions(-) diff --git a/apps/browser/src/background/main.background.ts b/apps/browser/src/background/main.background.ts index fc83cdb136c..c55674d2d4f 100644 --- a/apps/browser/src/background/main.background.ts +++ b/apps/browser/src/background/main.background.ts @@ -948,8 +948,6 @@ export default class MainBackground { this.billingAccountProfileStateService = new DefaultBillingAccountProfileStateService( this.stateProvider, - this.platformUtilsService, - this.apiService, ); this.restrictedItemTypesService = new RestrictedItemTypesService( diff --git a/apps/cli/src/service-container/service-container.ts b/apps/cli/src/service-container/service-container.ts index ffc67fac9d9..fa6c0e99c42 100644 --- a/apps/cli/src/service-container/service-container.ts +++ b/apps/cli/src/service-container/service-container.ts @@ -737,8 +737,6 @@ export class ServiceContainer { this.billingAccountProfileStateService = new DefaultBillingAccountProfileStateService( this.stateProvider, - this.platformUtilsService, - this.apiService, ); this.taskSchedulerService = new DefaultTaskSchedulerService(this.logService); diff --git a/apps/web/src/app/billing/individual/premium/cloud-hosted-premium.component.ts b/apps/web/src/app/billing/individual/premium/cloud-hosted-premium.component.ts index 7e219c44d90..94f75c5ecd2 100644 --- a/apps/web/src/app/billing/individual/premium/cloud-hosted-premium.component.ts +++ b/apps/web/src/app/billing/individual/premium/cloud-hosted-premium.component.ts @@ -3,6 +3,7 @@ import { Component, DestroyRef, inject } from "@angular/core"; import { takeUntilDestroyed } from "@angular/core/rxjs-interop"; import { ActivatedRoute, Router } from "@angular/router"; import { + catchError, combineLatest, firstValueFrom, from, @@ -32,6 +33,7 @@ import { } from "@bitwarden/components"; import { PricingCardComponent } from "@bitwarden/pricing"; import { I18nPipe } from "@bitwarden/ui-common"; +import { AccountBillingClient } from "@bitwarden/web-vault/app/billing/clients"; import { BitwardenSubscriber, mapAccountToSubscriber } from "../../types"; import { @@ -63,10 +65,12 @@ const RouteParamValues = { I18nPipe, PricingCardComponent, ], + providers: [AccountBillingClient], }) export class CloudHostedPremiumComponent { protected hasPremiumFromAnyOrganization$: Observable; protected hasPremiumPersonally$: Observable; + protected hasSubscription$: Observable; protected shouldShowNewDesign$: Observable; protected shouldShowUpgradeDialogOnInit$: Observable; protected personalPricingTiers$: Observable; @@ -84,6 +88,7 @@ export class CloudHostedPremiumComponent { private destroyRef = inject(DestroyRef); constructor( + private accountBillingClient: AccountBillingClient, private accountService: AccountService, private apiService: ApiService, private dialogService: DialogService, @@ -109,6 +114,17 @@ export class CloudHostedPremiumComponent { ), ); + this.hasSubscription$ = this.accountService.activeAccount$.pipe( + switchMap((account) => + account + ? from(this.accountBillingClient.getSubscription()).pipe( + map((subscription) => !!subscription), + catchError(() => of(false)), + ) + : of(false), + ), + ); + this.accountService.activeAccount$ .pipe(mapAccountToSubscriber, takeUntilDestroyed(this.destroyRef)) .subscribe((subscriber) => { @@ -122,11 +138,15 @@ export class CloudHostedPremiumComponent { // redirect to user subscription page if they already have premium personally // redirect to individual vault if they already have premium from an org - combineLatest([this.hasPremiumFromAnyOrganization$, this.hasPremiumPersonally$]) + combineLatest([ + this.hasPremiumFromAnyOrganization$, + this.hasPremiumPersonally$, + this.hasSubscription$, + ]) .pipe( takeUntilDestroyed(this.destroyRef), - switchMap(([hasPremiumFromOrg, hasPremiumPersonally]) => { - if (hasPremiumPersonally) { + switchMap(([hasPremiumFromOrg, hasPremiumPersonally, hasSubscription]) => { + if (hasPremiumPersonally && hasSubscription) { return from(this.navigateToSubscriptionPage()); } if (hasPremiumFromOrg) { diff --git a/apps/web/src/app/layouts/user-layout.component.ts b/apps/web/src/app/layouts/user-layout.component.ts index 6af7b0639e5..cc02a06ab4f 100644 --- a/apps/web/src/app/layouts/user-layout.component.ts +++ b/apps/web/src/app/layouts/user-layout.component.ts @@ -4,7 +4,7 @@ import { CommonModule } from "@angular/common"; import { Component, OnInit, Signal } from "@angular/core"; import { toSignal } from "@angular/core/rxjs-interop"; import { RouterModule } from "@angular/router"; -import { map, Observable, switchMap } from "rxjs"; +import { catchError, combineLatest, from, map, Observable, of, switchMap } from "rxjs"; import { JslibModule } from "@bitwarden/angular/jslib.module"; import { PasswordManagerLogo } from "@bitwarden/assets/svg"; @@ -18,6 +18,8 @@ import { FeatureFlag } from "@bitwarden/common/enums/feature-flag.enum"; import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service"; import { SyncService } from "@bitwarden/common/platform/sync"; import { SvgModule } from "@bitwarden/components"; +import { UserId } from "@bitwarden/user-core"; +import { AccountBillingClient } from "@bitwarden/web-vault/app/billing/clients"; import { BillingFreeFamiliesNavItemComponent } from "../billing/shared/billing-free-families-nav-item.component"; @@ -36,12 +38,11 @@ import { WebLayoutModule } from "./web-layout.module"; SvgModule, BillingFreeFamiliesNavItemComponent, ], + providers: [AccountBillingClient], }) export class UserLayoutComponent implements OnInit { protected readonly logo = PasswordManagerLogo; protected readonly showEmergencyAccess: Signal; - protected hasFamilySponsorshipAvailable$: Observable; - protected showSponsoredFamilies$: Observable; protected showSubscription$: Observable; protected readonly sendEnabled$: Observable = this.accountService.activeAccount$.pipe( getUserId, @@ -49,6 +50,9 @@ export class UserLayoutComponent implements OnInit { map((isDisabled) => !isDisabled), ); protected consolidatedSessionTimeoutComponent$: Observable; + protected hasPremiumPersonally$: Observable; + protected hasPremiumFromAnyOrganization$: Observable; + protected hasSubscription$: Observable; constructor( private syncService: SyncService, @@ -56,13 +60,8 @@ export class UserLayoutComponent implements OnInit { private accountService: AccountService, private policyService: PolicyService, private configService: ConfigService, + private accountBillingClient: AccountBillingClient, ) { - this.showSubscription$ = this.accountService.activeAccount$.pipe( - switchMap((account) => - this.billingAccountProfileStateService.canViewSubscription$(account.id), - ), - ); - this.showEmergencyAccess = toSignal( this.accountService.activeAccount$.pipe( getUserId, @@ -75,10 +74,44 @@ export class UserLayoutComponent implements OnInit { this.consolidatedSessionTimeoutComponent$ = this.configService.getFeatureFlag$( FeatureFlag.ConsolidatedSessionTimeoutComponent, ); + + this.hasPremiumPersonally$ = this.ifAccountExistsCheck((userId) => + this.billingAccountProfileStateService.hasPremiumPersonally$(userId), + ); + + this.hasPremiumFromAnyOrganization$ = this.ifAccountExistsCheck((userId) => + this.billingAccountProfileStateService.hasPremiumFromAnyOrganization$(userId), + ); + + this.hasSubscription$ = this.ifAccountExistsCheck(() => + from(this.accountBillingClient.getSubscription()).pipe( + map((subscription) => !!subscription), + catchError(() => of(false)), + ), + ); + + this.showSubscription$ = combineLatest([ + this.hasPremiumPersonally$, + this.hasPremiumFromAnyOrganization$, + this.hasSubscription$, + ]).pipe( + map(([hasPremiumPersonally, hasPremiumFromAnyOrganization, hasSubscription]) => { + if (hasPremiumFromAnyOrganization && !hasPremiumPersonally) { + return false; + } + return hasSubscription; + }), + ); } async ngOnInit() { document.body.classList.remove("layout_frontend"); await this.syncService.fullSync(false); } + + private ifAccountExistsCheck(predicate$: (userId: UserId) => Observable) { + return this.accountService.activeAccount$.pipe( + switchMap((account) => (account ? predicate$(account.id) : of(false))), + ); + } } diff --git a/libs/angular/src/services/jslib-services.module.ts b/libs/angular/src/services/jslib-services.module.ts index 51e78c2627e..93facfd5be7 100644 --- a/libs/angular/src/services/jslib-services.module.ts +++ b/libs/angular/src/services/jslib-services.module.ts @@ -1533,7 +1533,7 @@ const safeProviders: SafeProvider[] = [ safeProvider({ provide: BillingAccountProfileStateService, useClass: DefaultBillingAccountProfileStateService, - deps: [StateProvider, PlatformUtilsServiceAbstraction, ApiServiceAbstraction], + deps: [StateProvider], }), safeProvider({ provide: SubscriptionPricingServiceAbstraction, diff --git a/libs/common/src/billing/abstractions/account/billing-account-profile-state.service.ts b/libs/common/src/billing/abstractions/account/billing-account-profile-state.service.ts index de9642f9194..0bb1a342cf4 100644 --- a/libs/common/src/billing/abstractions/account/billing-account-profile-state.service.ts +++ b/libs/common/src/billing/abstractions/account/billing-account-profile-state.service.ts @@ -25,13 +25,6 @@ export abstract class BillingAccountProfileStateService { */ abstract hasPremiumFromAnySource$(userId: UserId): Observable; - /** - * Emits `true` when the subscription menu item should be shown in navigation. - * This is hidden for organizations that provide premium, except if the user has premium personally - * or has a billing history. - */ - abstract canViewSubscription$(userId: UserId): Observable; - /** * Sets the user's premium status fields upon every full sync, either from their personal * subscription to premium, or an organization they're a part of that grants them premium. diff --git a/libs/common/src/billing/services/account/billing-account-profile-state.service.spec.ts b/libs/common/src/billing/services/account/billing-account-profile-state.service.spec.ts index 02dbef469d6..ab8f217991c 100644 --- a/libs/common/src/billing/services/account/billing-account-profile-state.service.spec.ts +++ b/libs/common/src/billing/services/account/billing-account-profile-state.service.spec.ts @@ -1,16 +1,14 @@ import { firstValueFrom } from "rxjs"; +import { BillingAccountProfile } from "@bitwarden/common/billing/abstractions"; + import { FakeAccountService, mockAccountServiceWith, FakeStateProvider, FakeSingleUserState, } from "../../../../spec"; -import { ApiService } from "../../../abstractions/api.service"; -import { PlatformUtilsService } from "../../../platform/abstractions/platform-utils.service"; import { UserId } from "../../../types/guid"; -import { BillingAccountProfile } from "../../abstractions/account/billing-account-profile-state.service"; -import { BillingHistoryResponse } from "../../models/response/billing-history.response"; import { BILLING_ACCOUNT_PROFILE_KEY_DEFINITION, @@ -22,26 +20,14 @@ describe("BillingAccountProfileStateService", () => { let sut: DefaultBillingAccountProfileStateService; let userBillingAccountProfileState: FakeSingleUserState; let accountService: FakeAccountService; - let platformUtilsService: jest.Mocked; - let apiService: jest.Mocked; const userId = "fakeUserId" as UserId; beforeEach(() => { accountService = mockAccountServiceWith(userId); stateProvider = new FakeStateProvider(accountService); - platformUtilsService = { - isSelfHost: jest.fn(), - } as any; - apiService = { - getUserBillingHistory: jest.fn(), - } as any; - sut = new DefaultBillingAccountProfileStateService( - stateProvider, - platformUtilsService, - apiService, - ); + sut = new DefaultBillingAccountProfileStateService(stateProvider); userBillingAccountProfileState = stateProvider.singleUser.getFake( userId, @@ -131,74 +117,4 @@ describe("BillingAccountProfileStateService", () => { expect(await firstValueFrom(sut.hasPremiumFromAnySource$(userId))).toBe(true); }); }); - - describe("canViewSubscription$", () => { - beforeEach(() => { - platformUtilsService.isSelfHost.mockReturnValue(false); - apiService.getUserBillingHistory.mockResolvedValue( - new BillingHistoryResponse({ invoices: [], transactions: [] }), - ); - }); - - it("returns true when user has premium personally", async () => { - userBillingAccountProfileState.nextState({ - hasPremiumPersonally: true, - hasPremiumFromAnyOrganization: true, - }); - - expect(await firstValueFrom(sut.canViewSubscription$(userId))).toBe(true); - }); - - it("returns true when user has no premium from any source", async () => { - userBillingAccountProfileState.nextState({ - hasPremiumPersonally: false, - hasPremiumFromAnyOrganization: false, - }); - - expect(await firstValueFrom(sut.canViewSubscription$(userId))).toBe(true); - }); - - it("returns true when user has billing history in cloud environment", async () => { - userBillingAccountProfileState.nextState({ - hasPremiumPersonally: false, - hasPremiumFromAnyOrganization: true, - }); - platformUtilsService.isSelfHost.mockReturnValue(false); - apiService.getUserBillingHistory.mockResolvedValue( - new BillingHistoryResponse({ - invoices: [{ id: "1" }], - transactions: [{ id: "2" }], - }), - ); - - expect(await firstValueFrom(sut.canViewSubscription$(userId))).toBe(true); - }); - - it("returns false when user has no premium personally, has org premium, and no billing history", async () => { - userBillingAccountProfileState.nextState({ - hasPremiumPersonally: false, - hasPremiumFromAnyOrganization: true, - }); - platformUtilsService.isSelfHost.mockReturnValue(false); - apiService.getUserBillingHistory.mockResolvedValue( - new BillingHistoryResponse({ - invoices: [], - transactions: [], - }), - ); - - expect(await firstValueFrom(sut.canViewSubscription$(userId))).toBe(false); - }); - - it("returns false when user has no premium personally, has org premium, in self-hosted environment", async () => { - userBillingAccountProfileState.nextState({ - hasPremiumPersonally: false, - hasPremiumFromAnyOrganization: true, - }); - platformUtilsService.isSelfHost.mockReturnValue(true); - - expect(await firstValueFrom(sut.canViewSubscription$(userId))).toBe(false); - expect(apiService.getUserBillingHistory).not.toHaveBeenCalled(); - }); - }); }); diff --git a/libs/common/src/billing/services/account/billing-account-profile-state.service.ts b/libs/common/src/billing/services/account/billing-account-profile-state.service.ts index 654af40ffe1..eb6dbf1451a 100644 --- a/libs/common/src/billing/services/account/billing-account-profile-state.service.ts +++ b/libs/common/src/billing/services/account/billing-account-profile-state.service.ts @@ -1,13 +1,12 @@ -import { map, Observable, combineLatest, concatMap } from "rxjs"; +import { map, Observable } from "rxjs"; -import { ApiService } from "../../../abstractions/api.service"; -import { PlatformUtilsService } from "../../../platform/abstractions/platform-utils.service"; -import { BILLING_DISK, StateProvider, UserKeyDefinition } from "../../../platform/state"; -import { UserId } from "../../../types/guid"; import { BillingAccountProfile, BillingAccountProfileStateService, -} from "../../abstractions/account/billing-account-profile-state.service"; +} from "@bitwarden/common/billing/abstractions"; +import { BILLING_DISK, StateProvider, UserKeyDefinition } from "@bitwarden/state"; + +import { UserId } from "../../../types/guid"; export const BILLING_ACCOUNT_PROFILE_KEY_DEFINITION = new UserKeyDefinition( BILLING_DISK, @@ -19,11 +18,7 @@ export const BILLING_ACCOUNT_PROFILE_KEY_DEFINITION = new UserKeyDefinition { return this.stateProvider @@ -69,26 +64,4 @@ export class DefaultBillingAccountProfileStateService implements BillingAccountP }, ); } - - canViewSubscription$(userId: UserId): Observable { - return combineLatest([ - this.hasPremiumPersonally$(userId), - this.hasPremiumFromAnyOrganization$(userId), - ]).pipe( - concatMap(async ([hasPremiumPersonally, hasPremiumFromOrg]) => { - if (hasPremiumPersonally === true || !hasPremiumFromOrg === true) { - return true; - } - - const isCloud = !this.platformUtilsService.isSelfHost(); - - if (isCloud) { - const billing = await this.apiService.getUserBillingHistory(); - return !billing?.hasNoHistory; - } - - return false; - }), - ); - } }