From 4a0b6fc191d742760688ebfd0e707985667a628d Mon Sep 17 00:00:00 2001 From: Alex Morask <144709477+amorask-bitwarden@users.noreply.github.com> Date: Wed, 31 Jul 2024 09:27:16 -0400 Subject: [PATCH] Update client side error handling and remove add account credit from sub (#10214) --- .../services/web-provider.service.ts | 2 +- .../provider-billing-history.component.ts | 36 +++---- .../manage-client-name-dialog.component.ts | 2 +- ...ge-client-subscription-dialog.component.ts | 2 +- .../clients/manage-clients.component.ts | 17 ++-- .../provider-subscription.component.html | 3 - .../provider-subscription.component.ts | 15 +-- .../src/services/jslib-services.module.ts | 2 +- .../billilng-api.service.abstraction.ts | 25 +++-- .../billing/services/billing-api.service.ts | 99 ++++++++++++------- .../payment-method-warnings.service.spec.ts | 14 +-- .../payment-method-warnings.service.ts | 2 +- 12 files changed, 114 insertions(+), 105 deletions(-) diff --git a/bitwarden_license/bit-web/src/app/admin-console/providers/services/web-provider.service.ts b/bitwarden_license/bit-web/src/app/admin-console/providers/services/web-provider.service.ts index 4195ffcb05..16b2d51de4 100644 --- a/bitwarden_license/bit-web/src/app/admin-console/providers/services/web-provider.service.ts +++ b/bitwarden_license/bit-web/src/app/admin-console/providers/services/web-provider.service.ts @@ -71,7 +71,7 @@ export class WebProviderService { request.keyPair = new OrganizationKeysRequest(publicKey, encryptedPrivateKey.encryptedString); request.collectionName = encryptedCollectionName.encryptedString; - await this.billingApiService.createClientOrganization(providerId, request); + await this.billingApiService.createProviderClientOrganization(providerId, request); await this.apiService.refreshIdentityToken(); diff --git a/bitwarden_license/bit-web/src/app/billing/providers/billing-history/provider-billing-history.component.ts b/bitwarden_license/bit-web/src/app/billing/providers/billing-history/provider-billing-history.component.ts index d8385e87b3..70dd9d676b 100644 --- a/bitwarden_license/bit-web/src/app/billing/providers/billing-history/provider-billing-history.component.ts +++ b/bitwarden_license/bit-web/src/app/billing/providers/billing-history/provider-billing-history.component.ts @@ -1,7 +1,8 @@ import { DatePipe } from "@angular/common"; -import { Component, OnDestroy, OnInit } from "@angular/core"; +import { Component } from "@angular/core"; +import { takeUntilDestroyed } from "@angular/core/rxjs-interop"; import { ActivatedRoute } from "@angular/router"; -import { map, Subject, takeUntil } from "rxjs"; +import { map } from "rxjs"; import { BillingApiServiceAbstraction } from "@bitwarden/common/billing/abstractions"; import { InvoiceResponse } from "@bitwarden/common/billing/models/response/invoices.response"; @@ -9,16 +10,23 @@ import { InvoiceResponse } from "@bitwarden/common/billing/models/response/invoi @Component({ templateUrl: "./provider-billing-history.component.html", }) -export class ProviderBillingHistoryComponent implements OnInit, OnDestroy { +export class ProviderBillingHistoryComponent { private providerId: string; - private destroy$ = new Subject(); - constructor( private activatedRoute: ActivatedRoute, private billingApiService: BillingApiServiceAbstraction, private datePipe: DatePipe, - ) {} + ) { + this.activatedRoute.params + .pipe( + map(({ providerId }) => { + this.providerId = providerId; + }), + takeUntilDestroyed(), + ) + .subscribe(); + } getClientInvoiceReport = (invoiceId: string) => this.billingApiService.getProviderClientInvoiceReport(this.providerId, invoiceId); @@ -29,20 +37,4 @@ export class ProviderBillingHistoryComponent implements OnInit, OnDestroy { }; getInvoices = async () => await this.billingApiService.getProviderInvoices(this.providerId); - - ngOnInit() { - this.activatedRoute.params - .pipe( - map(({ providerId }) => { - this.providerId = providerId; - }), - takeUntil(this.destroy$), - ) - .subscribe(); - } - - ngOnDestroy() { - this.destroy$.next(); - this.destroy$.complete(); - } } diff --git a/bitwarden_license/bit-web/src/app/billing/providers/clients/manage-client-name-dialog.component.ts b/bitwarden_license/bit-web/src/app/billing/providers/clients/manage-client-name-dialog.component.ts index be46308c1c..20b7e9aaa7 100644 --- a/bitwarden_license/bit-web/src/app/billing/providers/clients/manage-client-name-dialog.component.ts +++ b/bitwarden_license/bit-web/src/app/billing/providers/clients/manage-client-name-dialog.component.ts @@ -59,7 +59,7 @@ export class ManageClientNameDialogComponent { request.assignedSeats = this.dialogParams.organization.seats; request.name = this.formGroup.value.name; - await this.billingApiService.updateClientOrganization( + await this.billingApiService.updateProviderClientOrganization( this.dialogParams.providerId, this.dialogParams.organization.id, request, diff --git a/bitwarden_license/bit-web/src/app/billing/providers/clients/manage-client-subscription-dialog.component.ts b/bitwarden_license/bit-web/src/app/billing/providers/clients/manage-client-subscription-dialog.component.ts index f92223d1b5..b87c110ee3 100644 --- a/bitwarden_license/bit-web/src/app/billing/providers/clients/manage-client-subscription-dialog.component.ts +++ b/bitwarden_license/bit-web/src/app/billing/providers/clients/manage-client-subscription-dialog.component.ts @@ -93,7 +93,7 @@ export class ManageClientSubscriptionDialogComponent implements OnInit { request.assignedSeats = this.formGroup.value.assignedSeats; request.name = this.dialogParams.organization.organizationName; - await this.billingApiService.updateClientOrganization( + await this.billingApiService.updateProviderClientOrganization( this.dialogParams.provider.id, this.dialogParams.organization.id, request, diff --git a/bitwarden_license/bit-web/src/app/billing/providers/clients/manage-clients.component.ts b/bitwarden_license/bit-web/src/app/billing/providers/clients/manage-clients.component.ts index 7d3cd13911..03ef71482f 100644 --- a/bitwarden_license/bit-web/src/app/billing/providers/clients/manage-clients.component.ts +++ b/bitwarden_license/bit-web/src/app/billing/providers/clients/manage-clients.component.ts @@ -1,9 +1,9 @@ import { Component } from "@angular/core"; +import { takeUntilDestroyed } from "@angular/core/rxjs-interop"; import { ActivatedRoute, Router } from "@angular/router"; import { firstValueFrom, from, lastValueFrom, map } from "rxjs"; -import { switchMap, takeUntil } from "rxjs/operators"; +import { switchMap } from "rxjs/operators"; -import { ApiService } from "@bitwarden/common/abstractions/api.service"; import { SearchService } from "@bitwarden/common/abstractions/search.service"; import { ProviderService } from "@bitwarden/common/admin-console/abstractions/provider.service"; import { ProviderUserType } from "@bitwarden/common/admin-console/enums"; @@ -46,7 +46,6 @@ export class ManageClientsComponent extends BaseClientsComponent { protected plans: PlanResponse[]; constructor( - private apiService: ApiService, private billingApiService: BillingApiService, private configService: ConfigService, private providerService: ProviderService, @@ -68,9 +67,7 @@ export class ManageClientsComponent extends BaseClientsComponent { validationService, webProviderService, ); - } - ngOnInit() { this.activatedRoute.parent.params .pipe( switchMap((params) => { @@ -90,15 +87,11 @@ export class ManageClientsComponent extends BaseClientsComponent { }), ); }), - takeUntil(this.destroy$), + takeUntilDestroyed(), ) .subscribe(); } - ngOnDestroy() { - super.ngOnDestroy(); - } - removeMonthly = (plan: string) => plan.replace(" (Monthly)", ""); async load() { @@ -106,7 +99,9 @@ export class ManageClientsComponent extends BaseClientsComponent { this.isProviderAdmin = this.provider.type === ProviderUserType.ProviderAdmin; - this.clients = (await this.apiService.getProviderClients(this.providerId)).data; + this.clients = ( + await this.billingApiService.getProviderClientOrganizations(this.providerId) + ).data; this.dataSource.data = this.clients; diff --git a/bitwarden_license/bit-web/src/app/billing/providers/subscription/provider-subscription.component.html b/bitwarden_license/bit-web/src/app/billing/providers/subscription/provider-subscription.component.html index f47df92efa..55675f2186 100644 --- a/bitwarden_license/bit-web/src/app/billing/providers/subscription/provider-subscription.component.html +++ b/bitwarden_license/bit-web/src/app/billing/providers/subscription/provider-subscription.component.html @@ -69,9 +69,6 @@

{{ subscription.accountCredit | currency: "$" }}

{{ "creditAppliedDesc" | i18n }}

- diff --git a/bitwarden_license/bit-web/src/app/billing/providers/subscription/provider-subscription.component.ts b/bitwarden_license/bit-web/src/app/billing/providers/subscription/provider-subscription.component.ts index d582ad071f..38f366f614 100644 --- a/bitwarden_license/bit-web/src/app/billing/providers/subscription/provider-subscription.component.ts +++ b/bitwarden_license/bit-web/src/app/billing/providers/subscription/provider-subscription.component.ts @@ -2,7 +2,6 @@ import { Component } from "@angular/core"; import { ActivatedRoute } from "@angular/router"; import { Subject, concatMap, takeUntil } from "rxjs"; -import { openAddAccountCreditDialog } from "@bitwarden/angular/billing/components"; import { BillingApiServiceAbstraction } from "@bitwarden/common/billing/abstractions/billilng-api.service.abstraction"; import { TaxInformation } from "@bitwarden/common/billing/models/domain"; import { ExpandedTaxInfoUpdateRequest } from "@bitwarden/common/billing/models/request/expanded-tax-info-update.request"; @@ -11,7 +10,7 @@ import { ProviderSubscriptionResponse, } from "@bitwarden/common/billing/models/response/provider-subscription-response"; import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; -import { DialogService, ToastService } from "@bitwarden/components"; +import { ToastService } from "@bitwarden/components"; @Component({ selector: "app-provider-subscription", @@ -27,9 +26,10 @@ export class ProviderSubscriptionComponent { totalCost: number; currentDate = new Date(); + protected readonly TaxInformation = TaxInformation; + constructor( private billingApiService: BillingApiServiceAbstraction, - private dialogService: DialogService, private i18nService: I18nService, private route: ActivatedRoute, private toastService: ToastService, @@ -63,13 +63,6 @@ export class ProviderSubscriptionComponent { this.loading = false; } - addAccountCredit = () => - openAddAccountCreditDialog(this.dialogService, { - data: { - providerId: this.providerId, - }, - }); - updateTaxInformation = async (taxInformation: TaxInformation) => { const request = ExpandedTaxInfoUpdateRequest.From(taxInformation); await this.billingApiService.updateProviderTaxInformation(this.providerId, request); @@ -108,6 +101,4 @@ export class ProviderSubscriptionComponent { this.destroy$.next(); this.destroy$.complete(); } - - protected readonly TaxInformation = TaxInformation; } diff --git a/libs/angular/src/services/jslib-services.module.ts b/libs/angular/src/services/jslib-services.module.ts index 5427c586af..db1eb1a577 100644 --- a/libs/angular/src/services/jslib-services.module.ts +++ b/libs/angular/src/services/jslib-services.module.ts @@ -1196,7 +1196,7 @@ const safeProviders: SafeProvider[] = [ safeProvider({ provide: BillingApiServiceAbstraction, useClass: BillingApiService, - deps: [ApiServiceAbstraction], + deps: [ApiServiceAbstraction, LogService, ToastService], }), safeProvider({ provide: PaymentMethodWarningsServiceAbstraction, diff --git a/libs/common/src/billing/abstractions/billilng-api.service.abstraction.ts b/libs/common/src/billing/abstractions/billilng-api.service.abstraction.ts index de3d6dd1e9..83db2fcd87 100644 --- a/libs/common/src/billing/abstractions/billilng-api.service.abstraction.ts +++ b/libs/common/src/billing/abstractions/billilng-api.service.abstraction.ts @@ -1,3 +1,4 @@ +import { ProviderOrganizationOrganizationDetailsResponse } from "@bitwarden/common/admin-console/models/response/provider/provider-organization.response"; import { PaymentMethodType } from "@bitwarden/common/billing/enums"; import { ExpandedTaxInfoUpdateRequest } from "@bitwarden/common/billing/models/request/expanded-tax-info-update.request"; import { TokenizedPaymentMethodRequest } from "@bitwarden/common/billing/models/request/tokenized-payment-method.request"; @@ -8,7 +9,6 @@ import { PaymentInformationResponse } from "@bitwarden/common/billing/models/res import { SubscriptionCancellationRequest } from "../../billing/models/request/subscription-cancellation.request"; import { OrganizationBillingMetadataResponse } from "../../billing/models/response/organization-billing-metadata.response"; import { OrganizationBillingStatusResponse } from "../../billing/models/response/organization-billing-status.response"; -import { OrganizationSubscriptionResponse } from "../../billing/models/response/organization-subscription.response"; import { PlanResponse } from "../../billing/models/response/plan.response"; import { ListResponse } from "../../models/response/list.response"; import { CreateClientOrganizationRequest } from "../models/request/create-client-organization.request"; @@ -23,39 +23,45 @@ export abstract class BillingApiServiceAbstraction { cancelPremiumUserSubscription: (request: SubscriptionCancellationRequest) => Promise; - createClientOrganization: ( + createProviderClientOrganization: ( providerId: string, request: CreateClientOrganizationRequest, ) => Promise; createSetupIntent: (paymentMethodType: PaymentMethodType) => Promise; - getBillingStatus: (id: string) => Promise; - getOrganizationBillingMetadata: ( organizationId: string, ) => Promise; - getOrganizationSubscription: ( - organizationId: string, - ) => Promise; + getOrganizationBillingStatus: (id: string) => Promise; getPlans: () => Promise>; getProviderClientInvoiceReport: (providerId: string, invoiceId: string) => Promise; + getProviderClientOrganizations: ( + providerId: string, + ) => Promise>; + getProviderInvoices: (providerId: string) => Promise; + /** + * @deprecated This endpoint is currently deactivated. + */ getProviderPaymentInformation: (providerId: string) => Promise; getProviderSubscription: (providerId: string) => Promise; - updateClientOrganization: ( + updateProviderClientOrganization: ( providerId: string, organizationId: string, request: UpdateClientOrganizationRequest, ) => Promise; + /** + * @deprecated This endpoint is currently deactivated. + */ updateProviderPaymentMethod: ( providerId: string, request: TokenizedPaymentMethodRequest, @@ -66,6 +72,9 @@ export abstract class BillingApiServiceAbstraction { request: ExpandedTaxInfoUpdateRequest, ) => Promise; + /** + * @deprecated This endpoint is currently deactivated. + */ verifyProviderBankAccount: ( providerId: string, request: VerifyBankAccountRequest, diff --git a/libs/common/src/billing/services/billing-api.service.ts b/libs/common/src/billing/services/billing-api.service.ts index 333c9ab011..a5841fc5b5 100644 --- a/libs/common/src/billing/services/billing-api.service.ts +++ b/libs/common/src/billing/services/billing-api.service.ts @@ -1,4 +1,8 @@ +import { ProviderOrganizationOrganizationDetailsResponse } from "@bitwarden/common/admin-console/models/response/provider/provider-organization.response"; import { InvoicesResponse } from "@bitwarden/common/billing/models/response/invoices.response"; +import { ErrorResponse } from "@bitwarden/common/models/response/error.response"; +import { LogService } from "@bitwarden/common/platform/abstractions/log.service"; +import { ToastService } from "@bitwarden/components"; import { ApiService } from "../../abstractions/api.service"; import { BillingApiServiceAbstraction } from "../../billing/abstractions"; @@ -9,7 +13,6 @@ import { TokenizedPaymentMethodRequest } from "../../billing/models/request/toke import { VerifyBankAccountRequest } from "../../billing/models/request/verify-bank-account.request"; import { OrganizationBillingMetadataResponse } from "../../billing/models/response/organization-billing-metadata.response"; import { OrganizationBillingStatusResponse } from "../../billing/models/response/organization-billing-status.response"; -import { OrganizationSubscriptionResponse } from "../../billing/models/response/organization-subscription.response"; import { PaymentInformationResponse } from "../../billing/models/response/payment-information.response"; import { PlanResponse } from "../../billing/models/response/plan.response"; import { ListResponse } from "../../models/response/list.response"; @@ -18,7 +21,11 @@ import { UpdateClientOrganizationRequest } from "../models/request/update-client import { ProviderSubscriptionResponse } from "../models/response/provider-subscription-response"; export class BillingApiService implements BillingApiServiceAbstraction { - constructor(private apiService: ApiService) {} + constructor( + private apiService: ApiService, + private logService: LogService, + private toastService: ToastService, + ) {} cancelOrganizationSubscription( organizationId: string, @@ -37,7 +44,7 @@ export class BillingApiService implements BillingApiServiceAbstraction { return this.apiService.send("POST", "/accounts/cancel", request, true, false); } - createClientOrganization( + createProviderClientOrganization( providerId: string, request: CreateClientOrganizationRequest, ): Promise { @@ -65,7 +72,7 @@ export class BillingApiService implements BillingApiServiceAbstraction { return response as string; } - async getBillingStatus(id: string): Promise { + async getOrganizationBillingStatus(id: string): Promise { const r = await this.apiService.send( "GET", "/organizations/" + id + "/billing-status", @@ -90,19 +97,6 @@ export class BillingApiService implements BillingApiServiceAbstraction { return new OrganizationBillingMetadataResponse(r); } - async getOrganizationSubscription( - organizationId: string, - ): Promise { - const r = await this.apiService.send( - "GET", - "/organizations/" + organizationId + "/subscription", - null, - true, - true, - ); - return new OrganizationSubscriptionResponse(r); - } - async getPlans(): Promise> { const r = await this.apiService.send("GET", "/plans", null, false, true); return new ListResponse(r, PlanResponse); @@ -119,40 +113,55 @@ export class BillingApiService implements BillingApiServiceAbstraction { return response as string; } + async getProviderClientOrganizations( + providerId: string, + ): Promise> { + const response = await this.execute(() => + this.apiService.send("GET", "/providers/" + providerId + "/organizations", null, true, true), + ); + return new ListResponse(response, ProviderOrganizationOrganizationDetailsResponse); + } + async getProviderInvoices(providerId: string): Promise { - const response = await this.apiService.send( - "GET", - "/providers/" + providerId + "/billing/invoices", - null, - true, - true, + const response = await this.execute(() => + this.apiService.send( + "GET", + "/providers/" + providerId + "/billing/invoices", + null, + true, + true, + ), ); return new InvoicesResponse(response); } async getProviderPaymentInformation(providerId: string): Promise { - const response = await this.apiService.send( - "GET", - "/providers/" + providerId + "/billing/payment-information", - null, - true, - true, + const response = await this.execute(() => + this.apiService.send( + "GET", + "/providers/" + providerId + "/billing/payment-information", + null, + true, + true, + ), ); return new PaymentInformationResponse(response); } async getProviderSubscription(providerId: string): Promise { - const r = await this.apiService.send( - "GET", - "/providers/" + providerId + "/billing/subscription", - null, - true, - true, + const response = await this.execute(() => + this.apiService.send( + "GET", + "/providers/" + providerId + "/billing/subscription", + null, + true, + true, + ), ); - return new ProviderSubscriptionResponse(r); + return new ProviderSubscriptionResponse(response); } - async updateClientOrganization( + async updateProviderClientOrganization( providerId: string, organizationId: string, request: UpdateClientOrganizationRequest, @@ -198,4 +207,20 @@ export class BillingApiService implements BillingApiServiceAbstraction { false, ); } + + private async execute(request: () => Promise): Promise { + try { + return await request(); + } catch (error) { + this.logService.error(error); + if (error instanceof ErrorResponse) { + this.toastService.showToast({ + variant: "error", + title: null, + message: error.getSingleMessage(), + }); + } + throw error; + } + } } diff --git a/libs/common/src/billing/services/payment-method-warnings.service.spec.ts b/libs/common/src/billing/services/payment-method-warnings.service.spec.ts index 55e72d3d72..6e37821ef5 100644 --- a/libs/common/src/billing/services/payment-method-warnings.service.spec.ts +++ b/libs/common/src/billing/services/payment-method-warnings.service.spec.ts @@ -113,13 +113,13 @@ describe("Payment Method Warnings Service", () => { }; activeUserState.nextState(state); await paymentMethodWarningsService.update(organizationId); - expect(billingApiService.getBillingStatus).not.toHaveBeenCalled(); + expect(billingApiService.getOrganizationBillingStatus).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( + billingApiService.getOrganizationBillingStatus.mockResolvedValue( getBillingStatusResponse(organizationId), ); await paymentMethodWarningsService.update(organizationId); @@ -131,7 +131,7 @@ describe("Payment Method Warnings Service", () => { savedAt: any(), }, }); - expect(billingApiService.getBillingStatus).toHaveBeenCalledTimes(1); + expect(billingApiService.getOrganizationBillingStatus).toHaveBeenCalledTimes(1); }); it("Retrieves the billing status from the API and uses it to update the state if the stored warning is null", async () => { @@ -139,7 +139,7 @@ describe("Payment Method Warnings Service", () => { activeUserState.nextState({ [organizationId]: null, }); - billingApiService.getBillingStatus.mockResolvedValue( + billingApiService.getOrganizationBillingStatus.mockResolvedValue( getBillingStatusResponse(organizationId), ); await paymentMethodWarningsService.update(organizationId); @@ -151,7 +151,7 @@ describe("Payment Method Warnings Service", () => { savedAt: any(), }, }); - expect(billingApiService.getBillingStatus).toHaveBeenCalledTimes(1); + expect(billingApiService.getOrganizationBillingStatus).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 () => { @@ -164,7 +164,7 @@ describe("Payment Method Warnings Service", () => { savedAt: getPastDate(10), }, }); - billingApiService.getBillingStatus.mockResolvedValue( + billingApiService.getOrganizationBillingStatus.mockResolvedValue( new OrganizationBillingStatusResponse({ OrganizationId: organizationId, OrganizationName: "Teams Organization", @@ -180,7 +180,7 @@ describe("Payment Method Warnings Service", () => { savedAt: any(), }, }); - expect(billingApiService.getBillingStatus).toHaveBeenCalledTimes(1); + expect(billingApiService.getOrganizationBillingStatus).toHaveBeenCalledTimes(1); }); }); }); diff --git a/libs/common/src/billing/services/payment-method-warnings.service.ts b/libs/common/src/billing/services/payment-method-warnings.service.ts index ad9cd02618..0dad48bb85 100644 --- a/libs/common/src/billing/services/payment-method-warnings.service.ts +++ b/libs/common/src/billing/services/payment-method-warnings.service.ts @@ -52,7 +52,7 @@ export class PaymentMethodWarningsService implements PaymentMethodWarningsServic ); if (!warning || warning.savedAt < this.getOneWeekAgo()) { const { organizationName, risksSubscriptionFailure } = - await this.billingApiService.getBillingStatus(organizationId); + await this.billingApiService.getOrganizationBillingStatus(organizationId); await this.paymentMethodWarningsState.update((state) => { state ??= {}; state[organizationId] = {