1
0
mirror of https://github.com/bitwarden/browser synced 2025-12-10 13:23:34 +00:00

[PM-27252] Upgrade Dialog Should not Show in Self Host (#17051)

* fix(billing): update and refactor observable logic

* tests(billing): add additional expects for dialog

* fix(billing): update for claude feedback

* tests(billing): update test conditions and comments
This commit is contained in:
Stephon Brown
2025-10-28 10:51:30 -04:00
committed by GitHub
parent 8162c06700
commit 6f34b6098a
2 changed files with 123 additions and 61 deletions

View File

@@ -1,6 +1,5 @@
import { mock, mockReset } from "jest-mock-extended";
import * as rxjs from "rxjs";
import { of } from "rxjs";
import { of, BehaviorSubject } from "rxjs";
import { VaultProfileService } from "@bitwarden/angular/vault/services/vault-profile.service";
import { OrganizationService } from "@bitwarden/common/admin-console/abstractions/organization/organization.service.abstraction";
@@ -8,6 +7,7 @@ import { AccountService, Account } from "@bitwarden/common/auth/abstractions/acc
import { BillingAccountProfileStateService } from "@bitwarden/common/billing/abstractions/account/billing-account-profile-state.service";
import { FeatureFlag } from "@bitwarden/common/enums/feature-flag.enum";
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/platform/sync/sync.service";
import { DialogRef, DialogService } from "@bitwarden/components";
@@ -28,6 +28,7 @@ describe("UnifiedUpgradePromptService", () => {
const mockDialogService = mock<DialogService>();
const mockOrganizationService = mock<OrganizationService>();
const mockDialogOpen = jest.spyOn(UnifiedUpgradeDialogComponent, "open");
const mockPlatformUtilsService = mock<PlatformUtilsService>();
/**
* Creates a mock DialogRef that implements the required properties for testing
@@ -57,33 +58,33 @@ describe("UnifiedUpgradePromptService", () => {
mockSyncService,
mockDialogService,
mockOrganizationService,
mockPlatformUtilsService,
);
}
const mockAccount: Account = {
id: "test-user-id",
} as Account;
const accountSubject = new rxjs.BehaviorSubject(mockAccount);
const accountSubject = new BehaviorSubject<Account | null>(mockAccount);
describe("initialization", () => {
beforeEach(() => {
mockAccountService.activeAccount$ = accountSubject.asObservable();
mockPlatformUtilsService.isSelfHost.mockReturnValue(false);
mockConfigService.getFeatureFlag$.mockReturnValue(of(true));
setupTestService();
});
it("should be created", () => {
expect(sut).toBeTruthy();
});
it("should subscribe to account and feature flag observables on construction", () => {
expect(mockConfigService.getFeatureFlag$).toHaveBeenCalledWith(
FeatureFlag.PM24996_ImplementUpgradeFromFreeDialog,
);
});
});
describe("displayUpgradePromptConditionally", () => {
beforeEach(async () => {
beforeEach(() => {
mockAccountService.activeAccount$ = accountSubject.asObservable();
mockDialogOpen.mockReset();
mockReset(mockDialogService);
mockReset(mockConfigService);
mockReset(mockBillingService);
mockReset(mockVaultProfileService);
@@ -93,20 +94,48 @@ describe("UnifiedUpgradePromptService", () => {
// Mock sync service methods
mockSyncService.fullSync.mockResolvedValue(true);
mockSyncService.lastSync$.mockReturnValue(of(new Date()));
mockReset(mockPlatformUtilsService);
});
it("should subscribe to account and feature flag observables when checking display conditions", async () => {
// Arrange
mockPlatformUtilsService.isSelfHost.mockReturnValue(false);
mockOrganizationService.memberOrganizations$.mockReturnValue(of([]));
mockConfigService.getFeatureFlag$.mockReturnValue(of(false));
mockBillingService.hasPremiumFromAnySource$.mockReturnValue(of(false));
setupTestService();
// Act
await sut.displayUpgradePromptConditionally();
// Assert
expect(mockConfigService.getFeatureFlag$).toHaveBeenCalledWith(
FeatureFlag.PM24996_ImplementUpgradeFromFreeDialog,
);
expect(mockAccountService.activeAccount$).toBeDefined();
});
it("should not show dialog when feature flag is disabled", async () => {
// Arrange
mockPlatformUtilsService.isSelfHost.mockReturnValue(false);
mockOrganizationService.memberOrganizations$.mockReturnValue(of([]));
mockConfigService.getFeatureFlag$.mockReturnValue(of(false));
mockBillingService.hasPremiumFromAnySource$.mockReturnValue(of(false));
const recentDate = new Date();
recentDate.setMinutes(recentDate.getMinutes() - 3); // 3 minutes old
mockVaultProfileService.getProfileCreationDate.mockResolvedValue(recentDate);
setupTestService();
// Act
const result = await sut.displayUpgradePromptConditionally();
// Assert
expect(result).toBeNull();
expect(mockDialogOpen).not.toHaveBeenCalled();
});
it("should not show dialog when user has premium", async () => {
// Arrange
mockPlatformUtilsService.isSelfHost.mockReturnValue(false);
mockConfigService.getFeatureFlag$.mockReturnValue(of(true));
mockBillingService.hasPremiumFromAnySource$.mockReturnValue(of(true));
mockOrganizationService.memberOrganizations$.mockReturnValue(of([]));
@@ -117,6 +146,7 @@ describe("UnifiedUpgradePromptService", () => {
// Assert
expect(result).toBeNull();
expect(mockDialogOpen).not.toHaveBeenCalled();
});
it("should not show dialog when user has any organization membership", async () => {
@@ -124,6 +154,7 @@ describe("UnifiedUpgradePromptService", () => {
mockConfigService.getFeatureFlag$.mockReturnValue(of(true));
mockBillingService.hasPremiumFromAnySource$.mockReturnValue(of(false));
mockOrganizationService.memberOrganizations$.mockReturnValue(of([{ id: "org1" } as any]));
mockPlatformUtilsService.isSelfHost.mockReturnValue(false);
setupTestService();
// Act
@@ -131,6 +162,7 @@ describe("UnifiedUpgradePromptService", () => {
// Assert
expect(result).toBeNull();
expect(mockDialogOpen).not.toHaveBeenCalled();
});
it("should not show dialog when profile is older than 5 minutes", async () => {
@@ -141,6 +173,7 @@ describe("UnifiedUpgradePromptService", () => {
const oldDate = new Date();
oldDate.setMinutes(oldDate.getMinutes() - 10); // 10 minutes old
mockVaultProfileService.getProfileCreationDate.mockResolvedValue(oldDate);
mockPlatformUtilsService.isSelfHost.mockReturnValue(false);
setupTestService();
// Act
@@ -148,6 +181,7 @@ describe("UnifiedUpgradePromptService", () => {
// Assert
expect(result).toBeNull();
expect(mockDialogOpen).not.toHaveBeenCalled();
});
it("should show dialog when all conditions are met", async () => {
@@ -158,6 +192,7 @@ describe("UnifiedUpgradePromptService", () => {
const recentDate = new Date();
recentDate.setMinutes(recentDate.getMinutes() - 3); // 3 minutes old
mockVaultProfileService.getProfileCreationDate.mockResolvedValue(recentDate);
mockPlatformUtilsService.isSelfHost.mockReturnValue(false);
const expectedResult = { status: UnifiedUpgradeDialogStatus.Closed };
mockDialogOpenMethod(createMockDialogRef(expectedResult));
@@ -182,6 +217,7 @@ describe("UnifiedUpgradePromptService", () => {
// Assert
expect(result).toBeNull();
expect(mockDialogOpen).not.toHaveBeenCalled();
});
it("should not show dialog when profile creation date is unavailable", async () => {
@@ -190,6 +226,8 @@ describe("UnifiedUpgradePromptService", () => {
mockBillingService.hasPremiumFromAnySource$.mockReturnValue(of(false));
mockOrganizationService.memberOrganizations$.mockReturnValue(of([]));
mockVaultProfileService.getProfileCreationDate.mockResolvedValue(null);
mockPlatformUtilsService.isSelfHost.mockReturnValue(false);
setupTestService();
// Act
@@ -197,6 +235,26 @@ describe("UnifiedUpgradePromptService", () => {
// Assert
expect(result).toBeNull();
expect(mockDialogOpen).not.toHaveBeenCalled();
});
it("should not show dialog when running in self-hosted environment", async () => {
// Arrange
mockConfigService.getFeatureFlag$.mockReturnValue(of(true));
mockOrganizationService.memberOrganizations$.mockReturnValue(of([]));
mockBillingService.hasPremiumFromAnySource$.mockReturnValue(of(false));
const recentDate = new Date();
recentDate.setMinutes(recentDate.getMinutes() - 3); // 3 minutes old
mockVaultProfileService.getProfileCreationDate.mockResolvedValue(recentDate);
mockPlatformUtilsService.isSelfHost.mockReturnValue(true);
setupTestService();
// Act
const result = await sut.displayUpgradePromptConditionally();
// Assert
expect(result).toBeNull();
expect(mockDialogOpen).not.toHaveBeenCalled();
});
});
});

View File

@@ -1,6 +1,6 @@
import { Injectable } from "@angular/core";
import { combineLatest, firstValueFrom, timeout } from "rxjs";
import { filter, switchMap, take } from "rxjs/operators";
import { combineLatest, firstValueFrom, timeout, from, Observable, of } from "rxjs";
import { filter, switchMap, take, map } from "rxjs/operators";
import { VaultProfileService } from "@bitwarden/angular/vault/services/vault-profile.service";
import { OrganizationService } from "@bitwarden/common/admin-console/abstractions/organization/organization.service.abstraction";
@@ -8,7 +8,9 @@ import { AccountService } from "@bitwarden/common/auth/abstractions/account.serv
import { BillingAccountProfileStateService } from "@bitwarden/common/billing/abstractions/account/billing-account-profile-state.service";
import { FeatureFlag } from "@bitwarden/common/enums/feature-flag.enum";
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/platform/sync/sync.service";
import { UserId } from "@bitwarden/common/types/guid";
import { DialogRef, DialogService } from "@bitwarden/components";
import {
@@ -29,63 +31,37 @@ export class UnifiedUpgradePromptService {
private syncService: SyncService,
private dialogService: DialogService,
private organizationService: OrganizationService,
private platformUtilsService: PlatformUtilsService,
) {}
private shouldShowPrompt$ = combineLatest([
this.accountService.activeAccount$,
private shouldShowPrompt$: Observable<boolean> = this.accountService.activeAccount$.pipe(
switchMap((account) => {
// Check self-hosted first before any other operations
if (this.platformUtilsService.isSelfHost()) {
return of(false);
}
if (!account) {
return of(false);
}
const isProfileLessThanFiveMinutesOld = from(
this.isProfileLessThanFiveMinutesOld(account.id),
);
const hasOrganizations = from(this.hasOrganizations(account.id));
return combineLatest([
isProfileLessThanFiveMinutesOld,
hasOrganizations,
this.billingAccountProfileStateService.hasPremiumFromAnySource$(account.id),
this.configService.getFeatureFlag$(FeatureFlag.PM24996_ImplementUpgradeFromFreeDialog),
]).pipe(
switchMap(async ([account, isFlagEnabled]) => {
if (!account || !account?.id) {
return false;
}
// Early return if feature flag is disabled
if (!isFlagEnabled) {
return false;
}
// Wait for sync to complete to ensure organizations are fully loaded
// Also force a sync to ensure we have the latest data
await this.syncService.fullSync(false);
// Wait for the sync to complete with timeout to prevent hanging
await firstValueFrom(
this.syncService.lastSync$(account.id).pipe(
filter((lastSync) => lastSync !== null),
take(1),
timeout(30000), // 30 second timeout
),
map(([isProfileLessThanFiveMinutesOld, hasOrganizations, hasPremium, isFlagEnabled]) => {
return (
isProfileLessThanFiveMinutesOld && !hasOrganizations && !hasPremium && isFlagEnabled
);
// Check if user has premium
const hasPremium = await firstValueFrom(
this.billingAccountProfileStateService.hasPremiumFromAnySource$(account.id),
}),
);
// Early return if user already has premium
if (hasPremium) {
return false;
}
// Check if user has any organization membership (any status including pending)
// Try using memberOrganizations$ which might have different filtering logic
const memberOrganizations = await firstValueFrom(
this.organizationService.memberOrganizations$(account.id),
);
const hasOrganizations = memberOrganizations.length > 0;
// Early return if user has any organization status
if (hasOrganizations) {
return false;
}
// Check profile age only if needed
const isProfileLessThanFiveMinutesOld = await this.isProfileLessThanFiveMinutesOld(
account.id,
);
return isFlagEnabled && !hasPremium && !hasOrganizations && isProfileLessThanFiveMinutesOld;
}),
take(1),
);
@@ -119,7 +95,7 @@ export class UnifiedUpgradePromptService {
const nowInMs = new Date().getTime();
const differenceInMs = nowInMs - createdAtInMs;
const msInAMinute = 1000 * 60; // Milliseconds in a minute for conversion 1 minute = 60 seconds * 1000 ms
const msInAMinute = 1000 * 60; // 60 seconds * 1000ms
const differenceInMinutes = Math.round(differenceInMs / msInAMinute);
return differenceInMinutes <= 5;
@@ -141,4 +117,32 @@ export class UnifiedUpgradePromptService {
// Return the result or null if the dialog was dismissed without a result
return result || null;
}
/**
* Checks if the user has any organization associated with their account
* @param userId User ID to check
* @returns Promise that resolves to true if user has any organizations, false otherwise
*/
private async hasOrganizations(userId: UserId): Promise<boolean> {
// Wait for sync to complete to ensure organizations are fully loaded
// Also force a sync to ensure we have the latest data
await this.syncService.fullSync(false);
// Wait for the sync to complete with timeout to prevent hanging
await firstValueFrom(
this.syncService.lastSync$(userId).pipe(
filter((lastSync) => lastSync !== null),
take(1),
timeout(30000), // 30 second timeout
),
);
// Check if user has any organization membership (any status including pending)
// Try using memberOrganizations$ which might have different filtering logic
const memberOrganizations = await firstValueFrom(
this.organizationService.memberOrganizations$(userId),
);
return memberOrganizations.length > 0;
}
}