Skip to content
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

[PM-16937] Remove Billing Circular Dependency #13085

Open
wants to merge 17 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
79dc9a6
Remove circular dependency between billing services and components
cturnbull-bitwarden Jan 24, 2025
a77906b
Merge branch 'main' into billing/PM-16937/circular-dependency
cturnbull-bitwarden Jan 27, 2025
519095b
Removed `logService` from `billing-api.service.ts`
cturnbull-bitwarden Jan 27, 2025
535e2a0
Resolved failed test
cturnbull-bitwarden Jan 27, 2025
50dbe18
Removed @bitwarden/ui-common
cturnbull-bitwarden Jan 27, 2025
c7f84f6
Merge branch 'main' into billing/PM-16937/circular-dependency
cturnbull-bitwarden Jan 27, 2025
bd3f387
Merge branch 'main' into billing/PM-16937/circular-dependency
cturnbull-bitwarden Jan 27, 2025
02ad4b9
Merge branch 'main' into billing/PM-16937/circular-dependency
cturnbull-bitwarden Jan 28, 2025
1bb58c9
Merge branch 'main' into billing/PM-16937/circular-dependency
cturnbull-bitwarden Jan 28, 2025
4c09260
Merge branch 'main' into billing/PM-16937/circular-dependency
cturnbull-bitwarden Jan 29, 2025
bc886f6
Added optional `title` parameter to `BillingNotificationService` funcโ€ฆ
cturnbull-bitwarden Jan 29, 2025
a30e3b8
Removed @bitwarden/platform from libs/common/tsconfig.json
cturnbull-bitwarden Jan 29, 2025
0033148
Merge branch 'main' into billing/PM-16937/circular-dependency
cturnbull-bitwarden Jan 29, 2025
7f5bee9
Merge branch 'main' into billing/PM-16937/circular-dependency
cturnbull-bitwarden Jan 29, 2025
9f7f2e1
Merge branch 'main' into billing/PM-16937/circular-dependency
cturnbull-bitwarden Feb 10, 2025
7a9c050
Update apps/web/src/app/billing/services/billing-notification.serviceโ€ฆ
cturnbull-bitwarden Feb 10, 2025
123a464
Update apps/web/src/app/billing/services/billing-notification.serviceโ€ฆ
cturnbull-bitwarden Feb 10, 2025
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 @@ -58,6 +58,7 @@
import { DialogService, ToastService } from "@bitwarden/components";
import { KeyService } from "@bitwarden/key-management";

import { BillingNotificationService } from "../services/billing-notification.service";
import { BillingSharedModule } from "../shared/billing-shared.module";
import { PaymentComponent } from "../shared/payment/payment.component";

Expand Down Expand Up @@ -207,6 +208,7 @@
private taxService: TaxServiceAbstraction,
private accountService: AccountService,
private organizationBillingService: OrganizationBillingService,
private billingNotificationService: BillingNotificationService,

Check warning on line 211 in apps/web/src/app/billing/organizations/change-plan-dialog.component.ts

View check run for this annotation

Codecov / codecov/patch

apps/web/src/app/billing/organizations/change-plan-dialog.component.ts#L211

Added line #L211 was not covered by tests
) {}

async ngOnInit(): Promise<void> {
Expand All @@ -227,10 +229,14 @@
.organizations$(userId)
.pipe(getOrganizationById(this.organizationId)),
);
const { accountCredit, paymentSource } =
await this.billingApiService.getOrganizationPaymentMethod(this.organizationId);
this.accountCredit = accountCredit;
this.paymentSource = paymentSource;
try {

Check warning on line 232 in apps/web/src/app/billing/organizations/change-plan-dialog.component.ts

View check run for this annotation

Codecov / codecov/patch

apps/web/src/app/billing/organizations/change-plan-dialog.component.ts#L232

Added line #L232 was not covered by tests
const { accountCredit, paymentSource } =
await this.billingApiService.getOrganizationPaymentMethod(this.organizationId);
this.accountCredit = accountCredit;
this.paymentSource = paymentSource;

Check warning on line 236 in apps/web/src/app/billing/organizations/change-plan-dialog.component.ts

View check run for this annotation

Codecov / codecov/patch

apps/web/src/app/billing/organizations/change-plan-dialog.component.ts#L234-L236

Added lines #L234 - L236 were not covered by tests
} catch (error) {
this.billingNotificationService.handleError(error);

Check warning on line 238 in apps/web/src/app/billing/organizations/change-plan-dialog.component.ts

View check run for this annotation

Codecov / codecov/patch

apps/web/src/app/billing/organizations/change-plan-dialog.component.ts#L238

Added line #L238 was not covered by tests
}
}

if (!this.selfHosted) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
import { DialogService, ToastService } from "@bitwarden/components";

import { FreeTrial } from "../../../core/types/free-trial";
import { BillingNotificationService } from "../../services/billing-notification.service";

Check warning on line 27 in apps/web/src/app/billing/organizations/payment-method/organization-payment-method.component.ts

View check run for this annotation

Codecov / codecov/patch

apps/web/src/app/billing/organizations/payment-method/organization-payment-method.component.ts#L27

Added line #L27 was not covered by tests
import { TrialFlowService } from "../../services/trial-flow.service";
import {
AddCreditDialogResult,
Expand Down Expand Up @@ -66,6 +67,7 @@
private organizationService: OrganizationService,
private accountService: AccountService,
protected syncService: SyncService,
private billingNotificationService: BillingNotificationService,

Check warning on line 70 in apps/web/src/app/billing/organizations/payment-method/organization-payment-method.component.ts

View check run for this annotation

Codecov / codecov/patch

apps/web/src/app/billing/organizations/payment-method/organization-payment-method.component.ts#L70

Added line #L70 was not covered by tests
) {
this.activatedRoute.params
.pipe(
Expand Down Expand Up @@ -115,47 +117,52 @@

protected load = async (): Promise<void> => {
this.loading = true;
const { accountCredit, paymentSource, subscriptionStatus } =
await this.billingApiService.getOrganizationPaymentMethod(this.organizationId);
this.accountCredit = accountCredit;
this.paymentSource = paymentSource;
this.subscriptionStatus = subscriptionStatus;

if (this.organizationId) {
const organizationSubscriptionPromise = this.organizationApiService.getSubscription(
this.organizationId,
);
const userId = await firstValueFrom(
this.accountService.activeAccount$.pipe(map((a) => a?.id)),
);
const organizationPromise = await firstValueFrom(
this.organizationService
.organizations$(userId)
.pipe(getOrganizationById(this.organizationId)),
);

[this.organizationSubscriptionResponse, this.organization] = await Promise.all([
organizationSubscriptionPromise,
organizationPromise,
]);
this.freeTrialData = this.trialFlowService.checkForOrgsWithUpcomingPaymentIssues(
this.organization,
this.organizationSubscriptionResponse,
paymentSource,
);
}
this.isUnpaid = this.subscriptionStatus === "unpaid" ?? false;
// If the flag `launchPaymentModalAutomatically` is set to true,
// we schedule a timeout (delay of 800ms) to automatically launch the payment modal.
// This delay ensures that any prior UI/rendering operations complete before triggering the modal.
if (this.launchPaymentModalAutomatically) {
window.setTimeout(async () => {
await this.changePayment();
this.launchPaymentModalAutomatically = false;
this.location.replaceState(this.location.path(), "", {});
}, 800);
try {

Check warning on line 120 in apps/web/src/app/billing/organizations/payment-method/organization-payment-method.component.ts

View check run for this annotation

Codecov / codecov/patch

apps/web/src/app/billing/organizations/payment-method/organization-payment-method.component.ts#L120

Added line #L120 was not covered by tests
const { accountCredit, paymentSource, subscriptionStatus } =
await this.billingApiService.getOrganizationPaymentMethod(this.organizationId);
this.accountCredit = accountCredit;
this.paymentSource = paymentSource;
this.subscriptionStatus = subscriptionStatus;

Check warning on line 125 in apps/web/src/app/billing/organizations/payment-method/organization-payment-method.component.ts

View check run for this annotation

Codecov / codecov/patch

apps/web/src/app/billing/organizations/payment-method/organization-payment-method.component.ts#L122-L125

Added lines #L122 - L125 were not covered by tests

if (this.organizationId) {
const organizationSubscriptionPromise = this.organizationApiService.getSubscription(

Check warning on line 128 in apps/web/src/app/billing/organizations/payment-method/organization-payment-method.component.ts

View check run for this annotation

Codecov / codecov/patch

apps/web/src/app/billing/organizations/payment-method/organization-payment-method.component.ts#L128

Added line #L128 was not covered by tests
this.organizationId,
);
const userId = await firstValueFrom(

Check warning on line 131 in apps/web/src/app/billing/organizations/payment-method/organization-payment-method.component.ts

View check run for this annotation

Codecov / codecov/patch

apps/web/src/app/billing/organizations/payment-method/organization-payment-method.component.ts#L131

Added line #L131 was not covered by tests
this.accountService.activeAccount$.pipe(map((a) => a?.id)),
);
const organizationPromise = await firstValueFrom(

Check warning on line 134 in apps/web/src/app/billing/organizations/payment-method/organization-payment-method.component.ts

View check run for this annotation

Codecov / codecov/patch

apps/web/src/app/billing/organizations/payment-method/organization-payment-method.component.ts#L134

Added line #L134 was not covered by tests
this.organizationService
.organizations$(userId)
.pipe(getOrganizationById(this.organizationId)),
);

[this.organizationSubscriptionResponse, this.organization] = await Promise.all([

Check warning on line 140 in apps/web/src/app/billing/organizations/payment-method/organization-payment-method.component.ts

View check run for this annotation

Codecov / codecov/patch

apps/web/src/app/billing/organizations/payment-method/organization-payment-method.component.ts#L140

Added line #L140 was not covered by tests
organizationSubscriptionPromise,
organizationPromise,
]);
this.freeTrialData = this.trialFlowService.checkForOrgsWithUpcomingPaymentIssues(

Check warning on line 144 in apps/web/src/app/billing/organizations/payment-method/organization-payment-method.component.ts

View check run for this annotation

Codecov / codecov/patch

apps/web/src/app/billing/organizations/payment-method/organization-payment-method.component.ts#L144

Added line #L144 was not covered by tests
this.organization,
this.organizationSubscriptionResponse,
paymentSource,
);
}
this.isUnpaid = this.subscriptionStatus === "unpaid" ?? false;
// If the flag `launchPaymentModalAutomatically` is set to true,
// we schedule a timeout (delay of 800ms) to automatically launch the payment modal.
// This delay ensures that any prior UI/rendering operations complete before triggering the modal.
if (this.launchPaymentModalAutomatically) {
window.setTimeout(async () => {
await this.changePayment();
this.launchPaymentModalAutomatically = false;
this.location.replaceState(this.location.path(), "", {});

Check warning on line 158 in apps/web/src/app/billing/organizations/payment-method/organization-payment-method.component.ts

View check run for this annotation

Codecov / codecov/patch

apps/web/src/app/billing/organizations/payment-method/organization-payment-method.component.ts#L155-L158

Added lines #L155 - L158 were not covered by tests
}, 800);
}
} catch (error) {
this.billingNotificationService.handleError(error);

Check warning on line 162 in apps/web/src/app/billing/organizations/payment-method/organization-payment-method.component.ts

View check run for this annotation

Codecov / codecov/patch

apps/web/src/app/billing/organizations/payment-method/organization-payment-method.component.ts#L162

Added line #L162 was not covered by tests
} finally {
this.loading = false;

Check warning on line 164 in apps/web/src/app/billing/organizations/payment-method/organization-payment-method.component.ts

View check run for this annotation

Codecov / codecov/patch

apps/web/src/app/billing/organizations/payment-method/organization-payment-method.component.ts#L164

Added line #L164 was not covered by tests
}
this.loading = false;
};

protected updatePaymentMethod = async (): Promise<void> => {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
import { mock, MockProxy } from "jest-mock-extended";

import { ErrorResponse } from "@bitwarden/common/models/response/error.response";
import { LogService } from "@bitwarden/common/platform/abstractions/log.service";
import { ToastService } from "@bitwarden/components";

import { BillingNotificationService } from "./billing-notification.service";

describe("BillingNotificationService", () => {
let service: BillingNotificationService;
let logService: MockProxy<LogService>;
let toastService: MockProxy<ToastService>;

beforeEach(() => {
logService = mock<LogService>();
toastService = mock<ToastService>();
service = new BillingNotificationService(logService, toastService);
});

describe("handleError", () => {
it("should log error and show toast for ErrorResponse", () => {
const error = new ErrorResponse(["test error"], 400);

expect(() => service.handleError(error)).toThrow();
expect(logService.error).toHaveBeenCalledWith(error);
expect(toastService.showToast).toHaveBeenCalledWith({
variant: "error",
title: "",
message: error.getSingleMessage(),
});
});

it("should only log error for non-ErrorResponse", () => {
const error = new Error("test error");

expect(() => service.handleError(error)).toThrow();
expect(logService.error).toHaveBeenCalledWith(error);
expect(toastService.showToast).not.toHaveBeenCalled();
});
});

describe("showSuccess", () => {
it("should show success toast", () => {
const message = "test message";
service.showSuccess(message);

expect(toastService.showToast).toHaveBeenCalledWith({
variant: "success",
title: "",
message,
});
});
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
import { Injectable } from "@angular/core";

import { ErrorResponse } from "@bitwarden/common/models/response/error.response";
import { LogService } from "@bitwarden/common/platform/abstractions/log.service";
import { ToastService } from "@bitwarden/components";

@Injectable({
providedIn: "root",
})
export class BillingNotificationService {
cturnbull-bitwarden marked this conversation as resolved.
Show resolved Hide resolved
constructor(
private logService: LogService,
private toastService: ToastService,
) {}

handleError(error: unknown) {
this.logService.error(error);
if (error instanceof ErrorResponse) {
this.toastService.showToast({
variant: "error",
title: "",
message: error.getSingleMessage(),
});
}
throw error;
}

showSuccess(message: string) {
this.toastService.showToast({
variant: "success",
title: "",
cturnbull-bitwarden marked this conversation as resolved.
Show resolved Hide resolved
message: message,
});
}
}
Original file line number Diff line number Diff line change
@@ -1,4 +1,8 @@
import { NgModule } from "@angular/core";

@NgModule({})
import { BillingNotificationService } from "./billing-notification.service";

@NgModule({
providers: [BillingNotificationService],
})
export class BillingServicesModule {}
15 changes: 13 additions & 2 deletions apps/web/src/app/vault/individual-vault/vault.component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
take,
takeUntil,
tap,
catchError,
} from "rxjs/operators";

import {
Expand Down Expand Up @@ -84,6 +85,7 @@
PasswordRepromptService,
} from "@bitwarden/vault";

import { BillingNotificationService } from "../../billing/services/billing-notification.service";

Check warning on line 88 in apps/web/src/app/vault/individual-vault/vault.component.ts

View check run for this annotation

Codecov / codecov/patch

apps/web/src/app/vault/individual-vault/vault.component.ts#L88

Added line #L88 was not covered by tests
import { TrialFlowService } from "../../billing/services/trial-flow.service";
import { FreeTrial } from "../../core/types/free-trial";
import { SharedModule } from "../../shared/shared.module";
Expand Down Expand Up @@ -236,9 +238,17 @@
ownerOrgs.map((org) =>
combineLatest([
this.organizationApiService.getSubscription(org.id),
this.organizationBillingService.getPaymentSource(org.id),
from(this.organizationBillingService.getPaymentSource(org.id)).pipe(
catchError((error: unknown) => {
this.billingNotificationService.handleError(error);
return of(null);

Check warning on line 244 in apps/web/src/app/vault/individual-vault/vault.component.ts

View check run for this annotation

Codecov / codecov/patch

apps/web/src/app/vault/individual-vault/vault.component.ts#L243-L244

Added lines #L243 - L244 were not covered by tests
}),
),
]).pipe(
map(([subscription, paymentSource]) => {
if (!paymentSource) {
return null;

Check warning on line 250 in apps/web/src/app/vault/individual-vault/vault.component.ts

View check run for this annotation

Codecov / codecov/patch

apps/web/src/app/vault/individual-vault/vault.component.ts#L250

Added line #L250 was not covered by tests
}
return this.trialFlowService.checkForOrgsWithUpcomingPaymentIssues(
org,
subscription,
Expand All @@ -249,7 +259,7 @@
),
);
}),
map((results) => results.filter((result) => result.shownBanner)),
map((results) => results.filter((result) => result !== null && result.shownBanner)),
shareReplay({ refCount: false, bufferSize: 1 }),
);

Expand Down Expand Up @@ -287,6 +297,7 @@
protected billingApiService: BillingApiServiceAbstraction,
private trialFlowService: TrialFlowService,
private organizationBillingService: OrganizationBillingServiceAbstraction,
private billingNotificationService: BillingNotificationService,

Check warning on line 300 in apps/web/src/app/vault/individual-vault/vault.component.ts

View check run for this annotation

Codecov / codecov/patch

apps/web/src/app/vault/individual-vault/vault.component.ts#L300

Added line #L300 was not covered by tests
) {}

async ngOnInit() {
Expand Down
14 changes: 13 additions & 1 deletion apps/web/src/app/vault/org-vault/vault.component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
switchMap,
takeUntil,
tap,
catchError,
} from "rxjs/operators";

import {
Expand Down Expand Up @@ -87,6 +88,7 @@

import { GroupApiService, GroupView } from "../../admin-console/organizations/core";
import { openEntityEventsDialog } from "../../admin-console/organizations/manage/entity-events.component";
import { BillingNotificationService } from "../../billing/services/billing-notification.service";

Check warning on line 91 in apps/web/src/app/vault/org-vault/vault.component.ts

View check run for this annotation

Codecov / codecov/patch

apps/web/src/app/vault/org-vault/vault.component.ts#L91

Added line #L91 was not covered by tests
import {
ResellerWarning,
ResellerWarningService,
Expand Down Expand Up @@ -275,6 +277,7 @@
private organizationBillingService: OrganizationBillingServiceAbstraction,
private resellerWarningService: ResellerWarningService,
private accountService: AccountService,
private billingNotificationService: BillingNotificationService,

Check warning on line 280 in apps/web/src/app/vault/org-vault/vault.component.ts

View check run for this annotation

Codecov / codecov/patch

apps/web/src/app/vault/org-vault/vault.component.ts#L280

Added line #L280 was not covered by tests
) {}

async ngOnInit() {
Expand Down Expand Up @@ -652,12 +655,21 @@
combineLatest([
of(org),
this.organizationApiService.getSubscription(org.id),
this.organizationBillingService.getPaymentSource(org.id),
from(this.organizationBillingService.getPaymentSource(org.id)).pipe(
catchError((error: unknown) => {
cturnbull-bitwarden marked this conversation as resolved.
Show resolved Hide resolved
this.billingNotificationService.handleError(error);
return of(null);

Check warning on line 661 in apps/web/src/app/vault/org-vault/vault.component.ts

View check run for this annotation

Codecov / codecov/patch

apps/web/src/app/vault/org-vault/vault.component.ts#L660-L661

Added lines #L660 - L661 were not covered by tests
}),
),
]),
),
map(([org, sub, paymentSource]) => {
if (!paymentSource) {
return null;

Check warning on line 668 in apps/web/src/app/vault/org-vault/vault.component.ts

View check run for this annotation

Codecov / codecov/patch

apps/web/src/app/vault/org-vault/vault.component.ts#L668

Added line #L668 was not covered by tests
}
return this.trialFlowService.checkForOrgsWithUpcomingPaymentIssues(org, sub, paymentSource);
}),
filter((result) => result !== null),

Check warning on line 672 in apps/web/src/app/vault/org-vault/vault.component.ts

View check run for this annotation

Codecov / codecov/patch

apps/web/src/app/vault/org-vault/vault.component.ts#L672

Added line #L672 was not covered by tests
);

this.resellerWarning$ = organization$.pipe(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@

import { BillingApiServiceAbstraction } from "@bitwarden/common/billing/abstractions";
import { InvoiceResponse } from "@bitwarden/common/billing/models/response/invoices.response";
import { BillingNotificationService } from "@bitwarden/web-vault/app/billing/services/billing-notification.service";

Check warning on line 11 in bitwarden_license/bit-web/src/app/billing/providers/billing-history/provider-billing-history.component.ts

View check run for this annotation

Codecov / codecov/patch

bitwarden_license/bit-web/src/app/billing/providers/billing-history/provider-billing-history.component.ts#L11

Added line #L11 was not covered by tests

@Component({
templateUrl: "./provider-billing-history.component.html",
Expand All @@ -19,6 +20,7 @@
private activatedRoute: ActivatedRoute,
private billingApiService: BillingApiServiceAbstraction,
private datePipe: DatePipe,
private billingNotificationService: BillingNotificationService,

Check warning on line 23 in bitwarden_license/bit-web/src/app/billing/providers/billing-history/provider-billing-history.component.ts

View check run for this annotation

Codecov / codecov/patch

bitwarden_license/bit-web/src/app/billing/providers/billing-history/provider-billing-history.component.ts#L23

Added line #L23 was not covered by tests
) {
this.activatedRoute.params
.pipe(
Expand All @@ -30,13 +32,27 @@
.subscribe();
}

getClientInvoiceReport = (invoiceId: string) =>
this.billingApiService.getProviderClientInvoiceReport(this.providerId, invoiceId);
getClientInvoiceReport = async (invoiceId: string) => {
try {
return await this.billingApiService.getProviderClientInvoiceReport(

Check warning on line 37 in bitwarden_license/bit-web/src/app/billing/providers/billing-history/provider-billing-history.component.ts

View check run for this annotation

Codecov / codecov/patch

bitwarden_license/bit-web/src/app/billing/providers/billing-history/provider-billing-history.component.ts#L35-L37

Added lines #L35 - L37 were not covered by tests
this.providerId,
invoiceId,
);
} catch (error) {
this.billingNotificationService.handleError(error);

Check warning on line 42 in bitwarden_license/bit-web/src/app/billing/providers/billing-history/provider-billing-history.component.ts

View check run for this annotation

Codecov / codecov/patch

bitwarden_license/bit-web/src/app/billing/providers/billing-history/provider-billing-history.component.ts#L42

Added line #L42 was not covered by tests
}
};

getClientInvoiceReportName = (invoice: InvoiceResponse) => {
const date = this.datePipe.transform(invoice.date, "yyyyMMdd");
return `bitwarden_provider-billing-history_${date}_${invoice.number}`;
};

getInvoices = async () => await this.billingApiService.getProviderInvoices(this.providerId);
getInvoices = async () => {
try {
return await this.billingApiService.getProviderInvoices(this.providerId);

Check warning on line 53 in bitwarden_license/bit-web/src/app/billing/providers/billing-history/provider-billing-history.component.ts

View check run for this annotation

Codecov / codecov/patch

bitwarden_license/bit-web/src/app/billing/providers/billing-history/provider-billing-history.component.ts#L51-L53

Added lines #L51 - L53 were not covered by tests
} catch (error) {
this.billingNotificationService.handleError(error);

Check warning on line 55 in bitwarden_license/bit-web/src/app/billing/providers/billing-history/provider-billing-history.component.ts

View check run for this annotation

Codecov / codecov/patch

bitwarden_license/bit-web/src/app/billing/providers/billing-history/provider-billing-history.component.ts#L55

Added line #L55 was not covered by tests
}
};
}
Loading
Loading