aherman
/
bitwarden-clients
mirror of https://github.com/bitwarden/clients
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

[PM-29055] Remove pm-25379-use-new-organization-metadata-structure feature flag (#18848)

Browse Source 
Remove the fully-enabled feature flag and simplify the billing metadata
API to always use the vNext endpoints. The legacy API path is removed
since the server will no longer serve it.

- Remove FeatureFlag.PM25379_UseNewOrganizationMetadataStructure enum and default
- Delete legacy getOrganizationBillingMetadata() API method (old /billing/metadata path)
- Rename vNext methods to remove VNext suffix
- Simplify OrganizationMetadataService to always use cached vNext path
- Remove ConfigService dependency from OrganizationMetadataService
- Update tests to remove feature flag branching
pull/19062/head
Alex Morask 3 months ago committed by GitHub
parent 51731c1526
commit 5161a232f5
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
6 changed files with 93 additions and 244 deletions
Show Stats Download Patch File Download Diff File

2
libs/angular/src/services/jslib-services.module.ts
Unescape View File

@ -1528,7 +1528,7 @@ const safeProviders: SafeProvider[] = [
safeProvider({
provide: OrganizationMetadataServiceAbstraction,
useClass: DefaultOrganizationMetadataService,
deps: [BillingApiServiceAbstraction, ConfigService, PlatformUtilsServiceAbstraction],
deps: [BillingApiServiceAbstraction, PlatformUtilsServiceAbstraction],
}),
safeProvider({
provide: BillingAccountProfileStateService,

6
libs/common/src/billing/abstractions/billing-api.service.abstraction.ts
Unescape View File

@ -21,11 +21,7 @@ export abstract class BillingApiServiceAbstraction {
organizationId: OrganizationId,
): Promise<OrganizationBillingMetadataResponse>;
abstract getOrganizationBillingMetadataVNext(
organizationId: OrganizationId,
): Promise<OrganizationBillingMetadataResponse>;
abstract getOrganizationBillingMetadataVNextSelfHost(
abstract getOrganizationBillingMetadataSelfHost(
organizationId: OrganizationId,
): Promise<OrganizationBillingMetadataResponse>;

16
libs/common/src/billing/services/billing-api.service.ts
Unescape View File

@ -36,20 +36,6 @@ export class BillingApiService implements BillingApiServiceAbstraction {
async getOrganizationBillingMetadata(
organizationId: OrganizationId,
): Promise<OrganizationBillingMetadataResponse> {
const r = await this.apiService.send(
"GET",
"/organizations/" + organizationId + "/billing/metadata",
null,
true,
true,
);
return new OrganizationBillingMetadataResponse(r);
}
async getOrganizationBillingMetadataVNext(
organizationId: OrganizationId,
): Promise<OrganizationBillingMetadataResponse> {
const r = await this.apiService.send(
"GET",
@ -62,7 +48,7 @@ export class BillingApiService implements BillingApiServiceAbstraction {
return new OrganizationBillingMetadataResponse(r);
}
async getOrganizationBillingMetadataVNextSelfHost(
async getOrganizationBillingMetadataSelfHost(
organizationId: OrganizationId,
): Promise<OrganizationBillingMetadataResponse> {
const r = await this.apiService.send(

254
libs/common/src/billing/services/organization/organization-metadata.service.spec.ts
Unescape View File

@ -1,13 +1,11 @@
import { mock } from "jest-mock-extended";
import { BehaviorSubject, firstValueFrom } from "rxjs";
import { firstValueFrom } from "rxjs";
import { BillingApiServiceAbstraction } from "@bitwarden/common/billing/abstractions";
import { OrganizationBillingMetadataResponse } from "@bitwarden/common/billing/models/response/organization-billing-metadata.response";
import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service";
import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service";
import { newGuid } from "@bitwarden/guid";
import { FeatureFlag } from "../../../enums/feature-flag.enum";
import { OrganizationId } from "../../../types/guid";
import { DefaultOrganizationMetadataService } from "./organization-metadata.service";
@ -15,9 +13,7 @@ import { DefaultOrganizationMetadataService } from "./organization-metadata.serv
describe("DefaultOrganizationMetadataService", () => {
let service: DefaultOrganizationMetadataService;
let billingApiService: jest.Mocked<BillingApiServiceAbstraction>;
let configService: jest.Mocked<ConfigService>;
let platformUtilsService: jest.Mocked<PlatformUtilsService>;
let featureFlagSubject: BehaviorSubject<boolean>;
const mockOrganizationId = newGuid() as OrganizationId;
const mockOrganizationId2 = newGuid() as OrganizationId;
@ -34,182 +30,114 @@ describe("DefaultOrganizationMetadataService", () => {
beforeEach(() => {
billingApiService = mock<BillingApiServiceAbstraction>();
configService = mock<ConfigService>();
platformUtilsService = mock<PlatformUtilsService>();
featureFlagSubject = new BehaviorSubject<boolean>(false);
configService.getFeatureFlag$.mockReturnValue(featureFlagSubject.asObservable());
platformUtilsService.isSelfHost.mockReturnValue(false);
service = new DefaultOrganizationMetadataService(
billingApiService,
configService,
platformUtilsService,
);
service = new DefaultOrganizationMetadataService(billingApiService, platformUtilsService);
});
afterEach(() => {
jest.resetAllMocks();
featureFlagSubject.complete();
});
describe("getOrganizationMetadata$", () => {
describe("feature flag OFF", () => {
beforeEach(() => {
featureFlagSubject.next(false);
});
it("calls getOrganizationBillingMetadata when feature flag is off", async () => {
const mockResponse = createMockMetadataResponse(false, 10);
billingApiService.getOrganizationBillingMetadata.mockResolvedValue(mockResponse);
const result = await firstValueFrom(service.getOrganizationMetadata$(mockOrganizationId));
expect(configService.getFeatureFlag$).toHaveBeenCalledWith(
FeatureFlag.PM25379_UseNewOrganizationMetadataStructure,
);
expect(billingApiService.getOrganizationBillingMetadata).toHaveBeenCalledWith(
mockOrganizationId,
);
expect(billingApiService.getOrganizationBillingMetadataVNext).not.toHaveBeenCalled();
expect(result).toEqual(mockResponse);
});
it("does not cache metadata when feature flag is off", async () => {
const mockResponse1 = createMockMetadataResponse(false, 10);
const mockResponse2 = createMockMetadataResponse(false, 15);
billingApiService.getOrganizationBillingMetadata
.mockResolvedValueOnce(mockResponse1)
.mockResolvedValueOnce(mockResponse2);
it("calls getOrganizationBillingMetadata for cloud-hosted", async () => {
const mockResponse = createMockMetadataResponse(false, 10);
billingApiService.getOrganizationBillingMetadata.mockResolvedValue(mockResponse);
const result1 = await firstValueFrom(service.getOrganizationMetadata$(mockOrganizationId));
const result2 = await firstValueFrom(service.getOrganizationMetadata$(mockOrganizationId));
const result = await firstValueFrom(service.getOrganizationMetadata$(mockOrganizationId));
expect(billingApiService.getOrganizationBillingMetadata).toHaveBeenCalledTimes(2);
expect(result1).toEqual(mockResponse1);
expect(result2).toEqual(mockResponse2);
});
expect(billingApiService.getOrganizationBillingMetadata).toHaveBeenCalledWith(
mockOrganizationId,
);
expect(result).toEqual(mockResponse);
});
describe("feature flag ON", () => {
beforeEach(() => {
featureFlagSubject.next(true);
});
it("calls getOrganizationBillingMetadataSelfHost when isSelfHost is true", async () => {
platformUtilsService.isSelfHost.mockReturnValue(true);
const mockResponse = createMockMetadataResponse(true, 25);
billingApiService.getOrganizationBillingMetadataSelfHost.mockResolvedValue(mockResponse);
it("calls getOrganizationBillingMetadataVNext when feature flag is on", async () => {
const mockResponse = createMockMetadataResponse(true, 15);
billingApiService.getOrganizationBillingMetadataVNext.mockResolvedValue(mockResponse);
const result = await firstValueFrom(service.getOrganizationMetadata$(mockOrganizationId));
const result = await firstValueFrom(service.getOrganizationMetadata$(mockOrganizationId));
expect(platformUtilsService.isSelfHost).toHaveBeenCalled();
expect(billingApiService.getOrganizationBillingMetadataSelfHost).toHaveBeenCalledWith(
mockOrganizationId,
);
expect(billingApiService.getOrganizationBillingMetadata).not.toHaveBeenCalled();
expect(result).toEqual(mockResponse);
});
expect(configService.getFeatureFlag$).toHaveBeenCalledWith(
FeatureFlag.PM25379_UseNewOrganizationMetadataStructure,
);
expect(billingApiService.getOrganizationBillingMetadataVNext).toHaveBeenCalledWith(
mockOrganizationId,
);
expect(billingApiService.getOrganizationBillingMetadata).not.toHaveBeenCalled();
expect(result).toEqual(mockResponse);
});
it("caches metadata by organization ID", async () => {
const mockResponse = createMockMetadataResponse(true, 10);
billingApiService.getOrganizationBillingMetadata.mockResolvedValue(mockResponse);
it("caches metadata by organization ID when feature flag is on", async () => {
const mockResponse = createMockMetadataResponse(true, 10);
billingApiService.getOrganizationBillingMetadataVNext.mockResolvedValue(mockResponse);
const result1 = await firstValueFrom(service.getOrganizationMetadata$(mockOrganizationId));
const result2 = await firstValueFrom(service.getOrganizationMetadata$(mockOrganizationId));
const result1 = await firstValueFrom(service.getOrganizationMetadata$(mockOrganizationId));
const result2 = await firstValueFrom(service.getOrganizationMetadata$(mockOrganizationId));
expect(billingApiService.getOrganizationBillingMetadata).toHaveBeenCalledTimes(1);
expect(result1).toEqual(mockResponse);
expect(result2).toEqual(mockResponse);
});
expect(billingApiService.getOrganizationBillingMetadataVNext).toHaveBeenCalledTimes(1);
expect(result1).toEqual(mockResponse);
expect(result2).toEqual(mockResponse);
});
it("maintains separate cache entries for different organization IDs", async () => {
const mockResponse1 = createMockMetadataResponse(true, 10);
const mockResponse2 = createMockMetadataResponse(false, 20);
billingApiService.getOrganizationBillingMetadata
.mockResolvedValueOnce(mockResponse1)
.mockResolvedValueOnce(mockResponse2);
it("maintains separate cache entries for different organization IDs", async () => {
const mockResponse1 = createMockMetadataResponse(true, 10);
const mockResponse2 = createMockMetadataResponse(false, 20);
billingApiService.getOrganizationBillingMetadataVNext
.mockResolvedValueOnce(mockResponse1)
.mockResolvedValueOnce(mockResponse2);
const result1 = await firstValueFrom(service.getOrganizationMetadata$(mockOrganizationId));
const result2 = await firstValueFrom(service.getOrganizationMetadata$(mockOrganizationId2));
const result3 = await firstValueFrom(service.getOrganizationMetadata$(mockOrganizationId));
const result4 = await firstValueFrom(service.getOrganizationMetadata$(mockOrganizationId2));
expect(billingApiService.getOrganizationBillingMetadataVNext).toHaveBeenCalledTimes(2);
expect(billingApiService.getOrganizationBillingMetadataVNext).toHaveBeenNthCalledWith(
1,
mockOrganizationId,
);
expect(billingApiService.getOrganizationBillingMetadataVNext).toHaveBeenNthCalledWith(
2,
mockOrganizationId2,
);
expect(result1).toEqual(mockResponse1);
expect(result2).toEqual(mockResponse2);
expect(result3).toEqual(mockResponse1);
expect(result4).toEqual(mockResponse2);
});
const result1 = await firstValueFrom(service.getOrganizationMetadata$(mockOrganizationId));
const result2 = await firstValueFrom(service.getOrganizationMetadata$(mockOrganizationId2));
const result3 = await firstValueFrom(service.getOrganizationMetadata$(mockOrganizationId));
const result4 = await firstValueFrom(service.getOrganizationMetadata$(mockOrganizationId2));
it("calls getOrganizationBillingMetadataVNextSelfHost when feature flag is on and isSelfHost is true", async () => {
platformUtilsService.isSelfHost.mockReturnValue(true);
const mockResponse = createMockMetadataResponse(true, 25);
billingApiService.getOrganizationBillingMetadataVNextSelfHost.mockResolvedValue(
mockResponse,
);
const result = await firstValueFrom(service.getOrganizationMetadata$(mockOrganizationId));
expect(platformUtilsService.isSelfHost).toHaveBeenCalled();
expect(billingApiService.getOrganizationBillingMetadataVNextSelfHost).toHaveBeenCalledWith(
mockOrganizationId,
);
expect(billingApiService.getOrganizationBillingMetadataVNext).not.toHaveBeenCalled();
expect(billingApiService.getOrganizationBillingMetadata).not.toHaveBeenCalled();
expect(result).toEqual(mockResponse);
});
expect(billingApiService.getOrganizationBillingMetadata).toHaveBeenCalledTimes(2);
expect(billingApiService.getOrganizationBillingMetadata).toHaveBeenNthCalledWith(
1,
mockOrganizationId,
);
expect(billingApiService.getOrganizationBillingMetadata).toHaveBeenNthCalledWith(
2,
mockOrganizationId2,
);
expect(result1).toEqual(mockResponse1);
expect(result2).toEqual(mockResponse2);
expect(result3).toEqual(mockResponse1);
expect(result4).toEqual(mockResponse2);
});
describe("shareReplay behavior", () => {
beforeEach(() => {
featureFlagSubject.next(true);
});
it("does not call API multiple times when the same cached observable is subscribed to multiple times", async () => {
const mockResponse = createMockMetadataResponse(true, 10);
billingApiService.getOrganizationBillingMetadataVNext.mockResolvedValue(mockResponse);
it("does not call API multiple times when the same cached observable is subscribed to multiple times", async () => {
const mockResponse = createMockMetadataResponse(true, 10);
billingApiService.getOrganizationBillingMetadata.mockResolvedValue(mockResponse);
const metadata$ = service.getOrganizationMetadata$(mockOrganizationId);
const metadata$ = service.getOrganizationMetadata$(mockOrganizationId);
const subscription1Promise = firstValueFrom(metadata$);
const subscription2Promise = firstValueFrom(metadata$);
const subscription3Promise = firstValueFrom(metadata$);
const subscription1Promise = firstValueFrom(metadata$);
const subscription2Promise = firstValueFrom(metadata$);
const subscription3Promise = firstValueFrom(metadata$);
const [result1, result2, result3] = await Promise.all([
subscription1Promise,
subscription2Promise,
subscription3Promise,
]);
const [result1, result2, result3] = await Promise.all([
subscription1Promise,
subscription2Promise,
subscription3Promise,
]);
expect(billingApiService.getOrganizationBillingMetadataVNext).toHaveBeenCalledTimes(1);
expect(result1).toEqual(mockResponse);
expect(result2).toEqual(mockResponse);
expect(result3).toEqual(mockResponse);
});
expect(billingApiService.getOrganizationBillingMetadata).toHaveBeenCalledTimes(1);
expect(result1).toEqual(mockResponse);
expect(result2).toEqual(mockResponse);
expect(result3).toEqual(mockResponse);
});
});
describe("refreshMetadataCache", () => {
beforeEach(() => {
featureFlagSubject.next(true);
});
it("refreshes cached metadata when called with feature flag on", (done) => {
it("refreshes cached metadata when called", (done) => {
const mockResponse1 = createMockMetadataResponse(true, 10);
const mockResponse2 = createMockMetadataResponse(true, 20);
let invocationCount = 0;
billingApiService.getOrganizationBillingMetadataVNext
billingApiService.getOrganizationBillingMetadata
.mockResolvedValueOnce(mockResponse1)
.mockResolvedValueOnce(mockResponse2);
@ -221,7 +149,7 @@ describe("DefaultOrganizationMetadataService", () => {
expect(result).toEqual(mockResponse1);
} else if (invocationCount === 2) {
expect(result).toEqual(mockResponse2);
expect(billingApiService.getOrganizationBillingMetadataVNext).toHaveBeenCalledTimes(2);
expect(billingApiService.getOrganizationBillingMetadata).toHaveBeenCalledTimes(2);
subscription.unsubscribe();
done();
}
@ -234,45 +162,13 @@ describe("DefaultOrganizationMetadataService", () => {
}, 10);
});
it("does trigger refresh when feature flag is disabled", async () => {
featureFlagSubject.next(false);
const mockResponse1 = createMockMetadataResponse(false, 10);
const mockResponse2 = createMockMetadataResponse(false, 20);
let invocationCount = 0;
billingApiService.getOrganizationBillingMetadata
.mockResolvedValueOnce(mockResponse1)
.mockResolvedValueOnce(mockResponse2);
const subscription = service.getOrganizationMetadata$(mockOrganizationId).subscribe({
next: () => {
invocationCount++;
},
});
// wait for initial invocation
await new Promise((resolve) => setTimeout(resolve, 10));
expect(invocationCount).toBe(1);
service.refreshMetadataCache();
await new Promise((resolve) => setTimeout(resolve, 10));
expect(invocationCount).toBe(2);
expect(billingApiService.getOrganizationBillingMetadata).toHaveBeenCalledTimes(2);
subscription.unsubscribe();
});
it("bypasses cache when refreshing metadata", (done) => {
const mockResponse1 = createMockMetadataResponse(true, 10);
const mockResponse2 = createMockMetadataResponse(true, 20);
const mockResponse3 = createMockMetadataResponse(true, 30);
let invocationCount = 0;
billingApiService.getOrganizationBillingMetadataVNext
billingApiService.getOrganizationBillingMetadata
.mockResolvedValueOnce(mockResponse1)
.mockResolvedValueOnce(mockResponse2)
.mockResolvedValueOnce(mockResponse3);
@ -289,7 +185,7 @@ describe("DefaultOrganizationMetadataService", () => {
service.refreshMetadataCache();
} else if (invocationCount === 3) {
expect(result).toEqual(mockResponse3);
expect(billingApiService.getOrganizationBillingMetadataVNext).toHaveBeenCalledTimes(3);
expect(billingApiService.getOrganizationBillingMetadata).toHaveBeenCalledTimes(3);
subscription.unsubscribe();
done();
}

57
libs/common/src/billing/services/organization/organization-metadata.service.ts
Unescape View File

@ -1,10 +1,8 @@
import { BehaviorSubject, combineLatest, from, Observable, shareReplay, switchMap } from "rxjs";
import { BehaviorSubject, from, Observable, shareReplay, switchMap } from "rxjs";
import { BillingApiServiceAbstraction } from "@bitwarden/common/billing/abstractions";
import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service";
import { FeatureFlag } from "../../../enums/feature-flag.enum";
import { ConfigService } from "../../../platform/abstractions/config/config.service";
import { OrganizationId } from "../../../types/guid";
import { OrganizationMetadataServiceAbstraction } from "../../abstractions/organization-metadata.service.abstraction";
import { OrganizationBillingMetadataResponse } from "../../models/response/organization-billing-metadata.response";
@ -17,7 +15,6 @@ export class DefaultOrganizationMetadataService implements OrganizationMetadataS
constructor(
private billingApiService: BillingApiServiceAbstraction,
private configService: ConfigService,
private platformUtilsService: PlatformUtilsService,
) {}
private refreshMetadataTrigger = new BehaviorSubject<void>(undefined);
@ -28,50 +25,26 @@ export class DefaultOrganizationMetadataService implements OrganizationMetadataS
};
getOrganizationMetadata$(orgId: OrganizationId): Observable<OrganizationBillingMetadataResponse> {
return combineLatest([
this.refreshMetadataTrigger,
this.configService.getFeatureFlag$(FeatureFlag.PM25379_UseNewOrganizationMetadataStructure),
]).pipe(
switchMap(([_, featureFlagEnabled]) =>
featureFlagEnabled
? this.vNextGetOrganizationMetadataInternal$(orgId)
: this.getOrganizationMetadataInternal$(orgId),
),
);
}
private vNextGetOrganizationMetadataInternal$(
orgId: OrganizationId,
): Observable<OrganizationBillingMetadataResponse> {
const cacheHit = this.metadataCache.get(orgId);
if (cacheHit) {
return cacheHit;
}
const result = from(this.fetchMetadata(orgId, true)).pipe(
shareReplay({ bufferSize: 1, refCount: false }),
);
this.metadataCache.set(orgId, result);
return result;
}
private getOrganizationMetadataInternal$(
organizationId: OrganizationId,
): Observable<OrganizationBillingMetadataResponse> {
return from(this.fetchMetadata(organizationId, false)).pipe(
shareReplay({ bufferSize: 1, refCount: false }),
return this.refreshMetadataTrigger.pipe(
switchMap(() => {
const cacheHit = this.metadataCache.get(orgId);
if (cacheHit) {
return cacheHit;
}
const result = from(this.fetchMetadata(orgId)).pipe(
shareReplay({ bufferSize: 1, refCount: false }),
);
this.metadataCache.set(orgId, result);
return result;
}),
);
}
private async fetchMetadata(
organizationId: OrganizationId,
featureFlagEnabled: boolean,
): Promise<OrganizationBillingMetadataResponse> {
return featureFlagEnabled
? this.platformUtilsService.isSelfHost()
? await this.billingApiService.getOrganizationBillingMetadataVNextSelfHost(organizationId)
: await this.billingApiService.getOrganizationBillingMetadataVNext(organizationId)
return this.platformUtilsService.isSelfHost()
? await this.billingApiService.getOrganizationBillingMetadataSelfHost(organizationId)
: await this.billingApiService.getOrganizationBillingMetadata(organizationId);
}
}

2
libs/common/src/enums/feature-flag.enum.ts
Unescape View File

@ -31,7 +31,6 @@ export enum FeatureFlag {
/* Billing */
TrialPaymentOptional = "PM-8163-trial-payment",
PM24032_NewNavigationPremiumUpgradeButton = "pm-24032-new-navigation-premium-upgrade-button",
PM25379_UseNewOrganizationMetadataStructure = "pm-25379-use-new-organization-metadata-structure",
PM26793_FetchPremiumPriceFromPricingService = "pm-26793-fetch-premium-price-from-pricing-service",
PM23713_PremiumBadgeOpensNewPremiumUpgradeDialog = "pm-23713-premium-badge-opens-new-premium-upgrade-dialog",
PM26462_Milestone_3 = "pm-26462-milestone-3",
@ -149,7 +148,6 @@ export const DefaultFeatureFlagValue = {
/* Billing */
[FeatureFlag.TrialPaymentOptional]: FALSE,
[FeatureFlag.PM24032_NewNavigationPremiumUpgradeButton]: FALSE,
[FeatureFlag.PM25379_UseNewOrganizationMetadataStructure]: FALSE,
[FeatureFlag.PM26793_FetchPremiumPriceFromPricingService]: FALSE,
[FeatureFlag.PM23713_PremiumBadgeOpensNewPremiumUpgradeDialog]: FALSE,
[FeatureFlag.PM26462_Milestone_3]: FALSE,

Write Preview
Loading…
Cancel
Save