Skip to content

[PM-18955] Improve performance of Billing invocations on Collections page #14159

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@
switchMap,
takeUntil,
tap,
catchError,
} from "rxjs/operators";

import {
Expand All @@ -41,8 +40,8 @@
import { Organization } from "@bitwarden/common/admin-console/models/domain/organization";
import { AccountService } from "@bitwarden/common/auth/abstractions/account.service";
import { getUserId } from "@bitwarden/common/auth/services/account.service";
import { OrganizationBillingServiceAbstraction } from "@bitwarden/common/billing/abstractions";
import { BillingApiServiceAbstraction } from "@bitwarden/common/billing/abstractions/billing-api.service.abstraction";
import { OrganizationBillingMetadataResponse } from "@bitwarden/common/billing/models/response/organization-billing-metadata.response";
import { EventType } from "@bitwarden/common/enums";
import { BroadcasterService } from "@bitwarden/common/platform/abstractions/broadcaster.service";
import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service";
Expand Down Expand Up @@ -76,7 +75,6 @@
PasswordRepromptService,
} from "@bitwarden/vault";

import { BillingNotificationService } from "../../../billing/services/billing-notification.service";
import {
ResellerWarning,
ResellerWarningService,
Expand Down Expand Up @@ -180,6 +178,8 @@
protected resellerWarning$: Observable<ResellerWarning | null>;
protected prevCipherId: string | null = null;
protected userId: UserId;
private metadata$: Observable<OrganizationBillingMetadataResponse>;

/**
* A list of collections that the user can assign items to and edit those items within.
* @protected
Expand All @@ -204,7 +204,7 @@
filter((organizations) => organizations.length === 1),
map(([organization]) => organization),
switchMap((organization) =>
from(this.billingApiService.getOrganizationBillingMetadata(organization.id)).pipe(
this.metadata$.pipe(

Check warning on line 207 in apps/web/src/app/admin-console/organizations/collections/vault.component.ts

View check run for this annotation

Codecov / codecov/patch

apps/web/src/app/admin-console/organizations/collections/vault.component.ts#L207

Added line #L207 was not covered by tests
tap((organizationMetaData) => {
this.hasSubscription$.next(organizationMetaData.hasSubscription);
}),
Expand Down Expand Up @@ -253,10 +253,8 @@
private organizationApiService: OrganizationApiServiceAbstraction,
private trialFlowService: TrialFlowService,
protected billingApiService: BillingApiServiceAbstraction,
private organizationBillingService: OrganizationBillingServiceAbstraction,
private resellerWarningService: ResellerWarningService,
private accountService: AccountService,
private billingNotificationService: BillingNotificationService,
) {}

async ngOnInit() {
Expand All @@ -279,11 +277,17 @@
map((account) => account?.id),
switchMap((id) =>
organizationId$.pipe(
switchMap((organizationId) =>
this.organizationService
switchMap((organizationId) => {
if (!this.metadata$) {
this.metadata$ = from(

Check warning on line 282 in apps/web/src/app/admin-console/organizations/collections/vault.component.ts

View check run for this annotation

Codecov / codecov/patch

apps/web/src/app/admin-console/organizations/collections/vault.component.ts#L282

Added line #L282 was not covered by tests
this.billingApiService.getOrganizationBillingMetadata(organizationId),
);
}

return this.organizationService

Check warning on line 287 in apps/web/src/app/admin-console/organizations/collections/vault.component.ts

View check run for this annotation

Codecov / codecov/patch

apps/web/src/app/admin-console/organizations/collections/vault.component.ts#L287

Added line #L287 was not covered by tests
.organizations$(id)
.pipe(map((organizations) => organizations.find((org) => org.id === organizationId))),
),
.pipe(map((organizations) => organizations.find((org) => org.id === organizationId)));

Check warning on line 289 in apps/web/src/app/admin-console/organizations/collections/vault.component.ts

View check run for this annotation

Codecov / codecov/patch

apps/web/src/app/admin-console/organizations/collections/vault.component.ts#L289

Added line #L289 was not covered by tests
}),
takeUntil(this.destroy$),
shareReplay({ refCount: false, bufferSize: 1 }),
),
Expand Down Expand Up @@ -633,27 +637,22 @@
combineLatest([
of(org),
this.organizationApiService.getSubscription(org.id),
from(this.organizationBillingService.getPaymentSource(org.id)).pipe(
catchError((error: unknown) => {
this.billingNotificationService.handleError(error);
return of(null);
}),
),
this.metadata$,
]),
),
map(([org, sub, paymentSource]) =>
this.trialFlowService.checkForOrgsWithUpcomingPaymentIssues(org, sub, paymentSource),
map(([org, sub, metadata]) =>
this.trialFlowService.checkForOrgsWithUpcomingPaymentIssues(

Check warning on line 644 in apps/web/src/app/admin-console/organizations/collections/vault.component.ts

View check run for this annotation

Codecov / codecov/patch

apps/web/src/app/admin-console/organizations/collections/vault.component.ts#L644

Added line #L644 was not covered by tests
org,
sub,
metadata.isPaymentMethodConfigured,
),
),
filter((result) => result !== null),
);

this.resellerWarning$ = organization$.pipe(
filter((org) => org.isOwner),
switchMap((org) =>
from(this.billingApiService.getOrganizationBillingMetadata(org.id)).pipe(
map((metadata) => ({ org, metadata })),
),
),
switchMap((org) => this.metadata$.pipe(map((metadata) => ({ org, metadata })))),

Check warning on line 655 in apps/web/src/app/admin-console/organizations/collections/vault.component.ts

View check run for this annotation

Codecov / codecov/patch

apps/web/src/app/admin-console/organizations/collections/vault.component.ts#L655

Added line #L655 was not covered by tests
map(({ org, metadata }) => this.resellerWarningService.getWarning(org, metadata)),
);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ export class OrganizationPaymentMethodComponent implements OnDestroy {
this.freeTrialData = this.trialFlowService.checkForOrgsWithUpcomingPaymentIssues(
this.organization,
this.organizationSubscriptionResponse,
paymentSource,
!!paymentSource,
);
}
this.isUnpaid = this.subscriptionStatus === "unpaid" ?? false;
Expand Down
6 changes: 2 additions & 4 deletions apps/web/src/app/billing/services/trial-flow.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,8 @@ import { lastValueFrom } from "rxjs";
import { OrganizationApiServiceAbstraction } from "@bitwarden/common/admin-console/abstractions/organization/organization-api.service.abstraction";
import { Organization } from "@bitwarden/common/admin-console/models/domain/organization";
import { BillingApiServiceAbstraction } from "@bitwarden/common/billing/abstractions/billing-api.service.abstraction";
import { BillingSourceResponse } from "@bitwarden/common/billing/models/response/billing.response";
import { OrganizationBillingMetadataResponse } from "@bitwarden/common/billing/models/response/organization-billing-metadata.response";
import { OrganizationSubscriptionResponse } from "@bitwarden/common/billing/models/response/organization-subscription.response";
import { PaymentSourceResponse } from "@bitwarden/common/billing/models/response/payment-source.response";
import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service";
import { DialogService } from "@bitwarden/components";

Expand All @@ -32,11 +30,11 @@ export class TrialFlowService {
checkForOrgsWithUpcomingPaymentIssues(
organization: Organization,
organizationSubscription: OrganizationSubscriptionResponse,
paymentSource: BillingSourceResponse | PaymentSourceResponse,
isPaymentConfigured: boolean,
): FreeTrial {
const trialEndDate = organizationSubscription?.subscription?.trialEndDate;
const displayBanner =
!paymentSource &&
isPaymentConfigured === false &&
organization?.isOwner &&
organizationSubscription?.subscription?.status === "trialing";
const trialRemainingDays = trialEndDate ? this.calculateTrialRemainingDays(trialEndDate) : 0;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -209,7 +209,7 @@ export class PaymentMethodComponent implements OnInit, OnDestroy {
this.freeTrialData = this.trialFlowService.checkForOrgsWithUpcomingPaymentIssues(
this.organization,
this.org,
this.billing?.paymentSource,
!!this.billing?.paymentSource,
);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -225,7 +225,7 @@ export class VaultComponent implements OnInit, OnDestroy {
this.trialFlowService.checkForOrgsWithUpcomingPaymentIssues(
org,
subscription,
paymentSource,
!!paymentSource,
),
),
),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,11 @@
]),
),
map(([org, sub, paymentSource]) => {
return this.trialFlowService.checkForOrgsWithUpcomingPaymentIssues(org, sub, paymentSource);
return this.trialFlowService.checkForOrgsWithUpcomingPaymentIssues(

Check warning on line 177 in bitwarden_license/bit-web/src/app/secrets-manager/overview/overview.component.ts

View check run for this annotation

Codecov / codecov/patch

bitwarden_license/bit-web/src/app/secrets-manager/overview/overview.component.ts#L177

Added line #L177 was not covered by tests
org,
sub,
!!paymentSource,
);
}),
filter((result) => result !== null),
takeUntil(this.destroy$),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
invoiceCreatedDate: Date | null;
subPeriodEndDate: Date | null;
isSubscriptionCanceled: boolean;
isPaymentMethodConfigured: boolean;

constructor(response: any) {
super(response);
Expand All @@ -25,6 +26,7 @@
this.invoiceCreatedDate = this.parseDate(this.getResponseProperty("InvoiceCreatedDate"));
this.subPeriodEndDate = this.parseDate(this.getResponseProperty("SubPeriodEndDate"));
this.isSubscriptionCanceled = this.getResponseProperty("IsSubscriptionCanceled");
this.isPaymentMethodConfigured = this.getResponseProperty("IsPaymentMethodConfigured");

Check warning on line 29 in libs/common/src/billing/models/response/organization-billing-metadata.response.ts

View check run for this annotation

Codecov / codecov/patch

libs/common/src/billing/models/response/organization-billing-metadata.response.ts#L29

Added line #L29 was not covered by tests
}

private parseDate(dateString: any): Date | null {
Expand Down
Loading