From 23c89bda74e365699e9ed2b37527d3c58a5c831c Mon Sep 17 00:00:00 2001 From: Oscar Hinton Date: Wed, 3 Apr 2024 22:51:55 +0200 Subject: [PATCH] [PM-6975] Replace purchasedPremium broadcast message with observables (#8421) In https://github.com/bitwarden/clients/pull/8133 the premium state changed to be derived from observables, which means we can get rid of the `purchasePremium` messages that are sent and instead rely directly on the observable to distribute the state. --- .../billing/individual/premium.component.ts | 5 +- .../app/layouts/user-layout.component.html | 4 +- .../src/app/layouts/user-layout.component.ts | 64 ++++++----------- .../src/app/settings/settings.component.ts | 68 ------------------- .../organization.service.abstraction.ts | 2 +- .../organization/organization.service.spec.ts | 6 +- .../organization/organization.service.ts | 12 ++-- 7 files changed, 33 insertions(+), 128 deletions(-) delete mode 100644 apps/web/src/app/settings/settings.component.ts diff --git a/apps/web/src/app/billing/individual/premium.component.ts b/apps/web/src/app/billing/individual/premium.component.ts index fa17821d56..60536c17b5 100644 --- a/apps/web/src/app/billing/individual/premium.component.ts +++ b/apps/web/src/app/billing/individual/premium.component.ts @@ -124,10 +124,7 @@ export class PremiumComponent implements OnInit { await this.apiService.refreshIdentityToken(); await this.syncService.fullSync(true); this.platformUtilsService.showToast("success", null, this.i18nService.t("premiumUpdated")); - this.messagingService.send("purchasedPremium"); - // FIXME: Verify that this floating promise is intentional. If it is, add an explanatory comment and ensure there is proper error handling. - // eslint-disable-next-line @typescript-eslint/no-floating-promises - this.router.navigate(["/settings/subscription/user-subscription"]); + await this.router.navigate(["/settings/subscription/user-subscription"]); } get additionalStorageTotal(): number { diff --git a/apps/web/src/app/layouts/user-layout.component.html b/apps/web/src/app/layouts/user-layout.component.html index 397e95d485..c70b2f9ff7 100644 --- a/apps/web/src/app/layouts/user-layout.component.html +++ b/apps/web/src/app/layouts/user-layout.component.html @@ -19,7 +19,7 @@ diff --git a/apps/web/src/app/layouts/user-layout.component.ts b/apps/web/src/app/layouts/user-layout.component.ts index ee30bed0d6..1a225e49c7 100644 --- a/apps/web/src/app/layouts/user-layout.component.ts +++ b/apps/web/src/app/layouts/user-layout.component.ts @@ -1,14 +1,13 @@ import { CommonModule } from "@angular/common"; -import { Component, NgZone, OnDestroy, OnInit } from "@angular/core"; +import { Component, OnInit } from "@angular/core"; import { RouterModule } from "@angular/router"; -import { firstValueFrom } from "rxjs"; +import { Observable, combineLatest, concatMap } from "rxjs"; import { JslibModule } from "@bitwarden/angular/jslib.module"; import { ApiService } from "@bitwarden/common/abstractions/api.service"; import { OrganizationService } from "@bitwarden/common/admin-console/abstractions/organization/organization.service.abstraction"; import { BillingAccountProfileStateService } from "@bitwarden/common/billing/abstractions/account/billing-account-profile-state.service"; import { FeatureFlag } from "@bitwarden/common/enums/feature-flag.enum"; -import { BroadcasterService } from "@bitwarden/common/platform/abstractions/broadcaster.service"; import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service"; import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service"; import { SyncService } from "@bitwarden/common/vault/abstractions/sync/sync.service.abstraction"; @@ -18,8 +17,6 @@ import { PaymentMethodWarningsModule } from "../billing/shared"; import { PasswordManagerLogo } from "./password-manager-logo"; -const BroadcasterSubscriptionId = "UserLayoutComponent"; - @Component({ selector: "app-user-layout", templateUrl: "user-layout.component.html", @@ -34,10 +31,10 @@ const BroadcasterSubscriptionId = "UserLayoutComponent"; PaymentMethodWarningsModule, ], }) -export class UserLayoutComponent implements OnInit, OnDestroy { +export class UserLayoutComponent implements OnInit { protected readonly logo = PasswordManagerLogo; - hasFamilySponsorshipAvailable: boolean; - hideSubscription: boolean; + protected hasFamilySponsorshipAvailable$: Observable; + protected showSubscription$: Observable; protected showPaymentMethodWarningBanners$ = this.configService.getFeatureFlag$( FeatureFlag.ShowPaymentMethodWarningBanners, @@ -45,8 +42,6 @@ export class UserLayoutComponent implements OnInit, OnDestroy { ); constructor( - private broadcasterService: BroadcasterService, - private ngZone: NgZone, private platformUtilsService: PlatformUtilsService, private organizationService: OrganizationService, private apiService: ApiService, @@ -58,43 +53,28 @@ export class UserLayoutComponent implements OnInit, OnDestroy { async ngOnInit() { document.body.classList.remove("layout_frontend"); - this.broadcasterService.subscribe(BroadcasterSubscriptionId, async (message: any) => { - // FIXME: Verify that this floating promise is intentional. If it is, add an explanatory comment and ensure there is proper error handling. - // eslint-disable-next-line @typescript-eslint/no-floating-promises - this.ngZone.run(async () => { - switch (message.command) { - case "purchasedPremium": - await this.load(); - break; - default: - } - }); - }); - await this.syncService.fullSync(false); - await this.load(); - } - ngOnDestroy() { - this.broadcasterService.unsubscribe(BroadcasterSubscriptionId); - } + this.hasFamilySponsorshipAvailable$ = this.organizationService.canManageSponsorships$; - async load() { - const hasPremiumPersonally = await firstValueFrom( + // We want to hide the subscription menu for organizations that provide premium. + // Except if the user has premium personally or has a billing history. + this.showSubscription$ = combineLatest([ this.billingAccountProfileStateService.hasPremiumPersonally$, - ); - const hasPremiumFromOrg = await firstValueFrom( this.billingAccountProfileStateService.hasPremiumFromAnyOrganization$, - ); - const selfHosted = this.platformUtilsService.isSelfHost(); + ]).pipe( + concatMap(async ([hasPremiumPersonally, hasPremiumFromOrg]) => { + const isCloud = !this.platformUtilsService.isSelfHost(); - this.hasFamilySponsorshipAvailable = await this.organizationService.canManageSponsorships(); - let billing = null; - if (!selfHosted) { - // TODO: We should remove the need to call this! - billing = await this.apiService.getUserBillingHistory(); - } - this.hideSubscription = - !hasPremiumPersonally && hasPremiumFromOrg && (selfHosted || billing?.hasNoHistory); + let billing = null; + if (isCloud) { + // TODO: We should remove the need to call this! + billing = await this.apiService.getUserBillingHistory(); + } + + const cloudAndBillingHistory = isCloud && !billing?.hasNoHistory; + return hasPremiumPersonally || !hasPremiumFromOrg || cloudAndBillingHistory; + }), + ); } } diff --git a/apps/web/src/app/settings/settings.component.ts b/apps/web/src/app/settings/settings.component.ts deleted file mode 100644 index b5b198d0ac..0000000000 --- a/apps/web/src/app/settings/settings.component.ts +++ /dev/null @@ -1,68 +0,0 @@ -import { Component, NgZone, OnDestroy, OnInit } from "@angular/core"; -import { firstValueFrom } from "rxjs"; - -import { ApiService } from "@bitwarden/common/abstractions/api.service"; -import { OrganizationService } from "@bitwarden/common/admin-console/abstractions/organization/organization.service.abstraction"; -import { BillingAccountProfileStateService } from "@bitwarden/common/billing/abstractions/account/billing-account-profile-state.service"; -import { BroadcasterService } from "@bitwarden/common/platform/abstractions/broadcaster.service"; -import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service"; - -const BroadcasterSubscriptionId = "SettingsComponent"; - -@Component({ - selector: "app-settings", - templateUrl: "settings.component.html", -}) -export class SettingsComponent implements OnInit, OnDestroy { - premium: boolean; - selfHosted: boolean; - hasFamilySponsorshipAvailable: boolean; - hideSubscription: boolean; - - constructor( - private broadcasterService: BroadcasterService, - private ngZone: NgZone, - private platformUtilsService: PlatformUtilsService, - private organizationService: OrganizationService, - private apiService: ApiService, - private billingAccountProfileStateServiceAbstraction: BillingAccountProfileStateService, - ) {} - - async ngOnInit() { - this.broadcasterService.subscribe(BroadcasterSubscriptionId, async (message: any) => { - // FIXME: Verify that this floating promise is intentional. If it is, add an explanatory comment and ensure there is proper error handling. - // eslint-disable-next-line @typescript-eslint/no-floating-promises - this.ngZone.run(async () => { - switch (message.command) { - case "purchasedPremium": - await this.load(); - break; - default: - } - }); - }); - - this.selfHosted = await this.platformUtilsService.isSelfHost(); - await this.load(); - } - - ngOnDestroy() { - this.broadcasterService.unsubscribe(BroadcasterSubscriptionId); - } - - async load() { - this.premium = await firstValueFrom( - this.billingAccountProfileStateServiceAbstraction.hasPremiumPersonally$, - ); - this.hasFamilySponsorshipAvailable = await this.organizationService.canManageSponsorships(); - const hasPremiumFromOrg = await firstValueFrom( - this.billingAccountProfileStateServiceAbstraction.hasPremiumFromAnyOrganization$, - ); - let billing = null; - if (!this.selfHosted) { - billing = await this.apiService.getUserBillingHistory(); - } - this.hideSubscription = - !this.premium && hasPremiumFromOrg && (this.selfHosted || billing?.hasNoHistory); - } -} diff --git a/libs/common/src/admin-console/abstractions/organization/organization.service.abstraction.ts b/libs/common/src/admin-console/abstractions/organization/organization.service.abstraction.ts index 9cc4bba0eb..a1ae64a885 100644 --- a/libs/common/src/admin-console/abstractions/organization/organization.service.abstraction.ts +++ b/libs/common/src/admin-console/abstractions/organization/organization.service.abstraction.ts @@ -116,7 +116,7 @@ export abstract class OrganizationService { * https://bitwarden.atlassian.net/browse/AC-2252. */ getFromState: (id: string) => Promise; - canManageSponsorships: () => Promise; + canManageSponsorships$: Observable; hasOrganizations: () => Promise; get$: (id: string) => Observable; get: (id: string) => Promise; diff --git a/libs/common/src/admin-console/services/organization/organization.service.spec.ts b/libs/common/src/admin-console/services/organization/organization.service.spec.ts index 908f4b8e28..6d2525966b 100644 --- a/libs/common/src/admin-console/services/organization/organization.service.spec.ts +++ b/libs/common/src/admin-console/services/organization/organization.service.spec.ts @@ -121,7 +121,7 @@ describe("OrganizationService", () => { const mockData: OrganizationData[] = buildMockOrganizations(1); mockData[0].familySponsorshipAvailable = true; fakeActiveUserState.nextState(arrayToRecord(mockData)); - const result = await organizationService.canManageSponsorships(); + const result = await firstValueFrom(organizationService.canManageSponsorships$); expect(result).toBe(true); }); @@ -129,7 +129,7 @@ describe("OrganizationService", () => { const mockData: OrganizationData[] = buildMockOrganizations(1); mockData[0].familySponsorshipFriendlyName = "Something"; fakeActiveUserState.nextState(arrayToRecord(mockData)); - const result = await organizationService.canManageSponsorships(); + const result = await firstValueFrom(organizationService.canManageSponsorships$); expect(result).toBe(true); }); @@ -137,7 +137,7 @@ describe("OrganizationService", () => { const mockData: OrganizationData[] = buildMockOrganizations(1); mockData[0].familySponsorshipFriendlyName = null; fakeActiveUserState.nextState(arrayToRecord(mockData)); - const result = await organizationService.canManageSponsorships(); + const result = await firstValueFrom(organizationService.canManageSponsorships$); expect(result).toBe(false); }); }); diff --git a/libs/common/src/admin-console/services/organization/organization.service.ts b/libs/common/src/admin-console/services/organization/organization.service.ts index 3c651f4660..411850fe30 100644 --- a/libs/common/src/admin-console/services/organization/organization.service.ts +++ b/libs/common/src/admin-console/services/organization/organization.service.ts @@ -77,14 +77,10 @@ export class OrganizationService implements InternalOrganizationServiceAbstracti return await firstValueFrom(this.getOrganizationsFromState$(userId as UserId)); } - async canManageSponsorships(): Promise { - return await firstValueFrom( - this.organizations$.pipe( - mapToExcludeOrganizationsWithoutFamilySponsorshipSupport(), - mapToBooleanHasAnyOrganizations(), - ), - ); - } + canManageSponsorships$ = this.organizations$.pipe( + mapToExcludeOrganizationsWithoutFamilySponsorshipSupport(), + mapToBooleanHasAnyOrganizations(), + ); async hasOrganizations(): Promise { return await firstValueFrom(this.organizations$.pipe(mapToBooleanHasAnyOrganizations()));