1
0
mirror of https://github.com/bitwarden/browser synced 2025-12-11 05:43:41 +00:00

[PM-27257]Fix : Remove Welcome to Bitwarden modal for users with any Organization status (#17002)

* Resolve the modal for invited members

* Resolve multiple modal display

* Fix the failing test

* Remove the await
This commit is contained in:
cyprain-okeke
2025-10-24 11:48:05 +01:00
committed by GitHub
parent 6fdeefef3d
commit 2d3712acec
3 changed files with 64 additions and 4 deletions

View File

@@ -3,10 +3,12 @@ import * as rxjs from "rxjs";
import { of } from "rxjs"; import { of } from "rxjs";
import { VaultProfileService } from "@bitwarden/angular/vault/services/vault-profile.service"; import { VaultProfileService } from "@bitwarden/angular/vault/services/vault-profile.service";
import { OrganizationService } from "@bitwarden/common/admin-console/abstractions/organization/organization.service.abstraction";
import { AccountService, Account } from "@bitwarden/common/auth/abstractions/account.service"; import { AccountService, Account } from "@bitwarden/common/auth/abstractions/account.service";
import { BillingAccountProfileStateService } from "@bitwarden/common/billing/abstractions/account/billing-account-profile-state.service"; import { BillingAccountProfileStateService } from "@bitwarden/common/billing/abstractions/account/billing-account-profile-state.service";
import { FeatureFlag } from "@bitwarden/common/enums/feature-flag.enum"; import { FeatureFlag } from "@bitwarden/common/enums/feature-flag.enum";
import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service"; import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service";
import { SyncService } from "@bitwarden/common/platform/sync/sync.service";
import { DialogRef, DialogService } from "@bitwarden/components"; import { DialogRef, DialogService } from "@bitwarden/components";
import { import {
@@ -22,7 +24,9 @@ describe("UnifiedUpgradePromptService", () => {
const mockConfigService = mock<ConfigService>(); const mockConfigService = mock<ConfigService>();
const mockBillingService = mock<BillingAccountProfileStateService>(); const mockBillingService = mock<BillingAccountProfileStateService>();
const mockVaultProfileService = mock<VaultProfileService>(); const mockVaultProfileService = mock<VaultProfileService>();
const mockSyncService = mock<SyncService>();
const mockDialogService = mock<DialogService>(); const mockDialogService = mock<DialogService>();
const mockOrganizationService = mock<OrganizationService>();
const mockDialogOpen = jest.spyOn(UnifiedUpgradeDialogComponent, "open"); const mockDialogOpen = jest.spyOn(UnifiedUpgradeDialogComponent, "open");
/** /**
@@ -50,7 +54,9 @@ describe("UnifiedUpgradePromptService", () => {
mockConfigService, mockConfigService,
mockBillingService, mockBillingService,
mockVaultProfileService, mockVaultProfileService,
mockSyncService,
mockDialogService, mockDialogService,
mockOrganizationService,
); );
} }
@@ -81,6 +87,12 @@ describe("UnifiedUpgradePromptService", () => {
mockReset(mockConfigService); mockReset(mockConfigService);
mockReset(mockBillingService); mockReset(mockBillingService);
mockReset(mockVaultProfileService); mockReset(mockVaultProfileService);
mockReset(mockSyncService);
mockReset(mockOrganizationService);
// Mock sync service methods
mockSyncService.fullSync.mockResolvedValue(true);
mockSyncService.lastSync$.mockReturnValue(of(new Date()));
}); });
it("should not show dialog when feature flag is disabled", async () => { it("should not show dialog when feature flag is disabled", async () => {
// Arrange // Arrange
@@ -97,6 +109,21 @@ describe("UnifiedUpgradePromptService", () => {
// Arrange // Arrange
mockConfigService.getFeatureFlag$.mockReturnValue(of(true)); mockConfigService.getFeatureFlag$.mockReturnValue(of(true));
mockBillingService.hasPremiumFromAnySource$.mockReturnValue(of(true)); mockBillingService.hasPremiumFromAnySource$.mockReturnValue(of(true));
mockOrganizationService.memberOrganizations$.mockReturnValue(of([]));
setupTestService();
// Act
const result = await sut.displayUpgradePromptConditionally();
// Assert
expect(result).toBeNull();
});
it("should not show dialog when user has any organization membership", async () => {
// Arrange
mockConfigService.getFeatureFlag$.mockReturnValue(of(true));
mockBillingService.hasPremiumFromAnySource$.mockReturnValue(of(false));
mockOrganizationService.memberOrganizations$.mockReturnValue(of([{ id: "org1" } as any]));
setupTestService(); setupTestService();
// Act // Act
@@ -110,6 +137,7 @@ describe("UnifiedUpgradePromptService", () => {
// Arrange // Arrange
mockConfigService.getFeatureFlag$.mockReturnValue(of(true)); mockConfigService.getFeatureFlag$.mockReturnValue(of(true));
mockBillingService.hasPremiumFromAnySource$.mockReturnValue(of(false)); mockBillingService.hasPremiumFromAnySource$.mockReturnValue(of(false));
mockOrganizationService.memberOrganizations$.mockReturnValue(of([]));
const oldDate = new Date(); const oldDate = new Date();
oldDate.setMinutes(oldDate.getMinutes() - 10); // 10 minutes old oldDate.setMinutes(oldDate.getMinutes() - 10); // 10 minutes old
mockVaultProfileService.getProfileCreationDate.mockResolvedValue(oldDate); mockVaultProfileService.getProfileCreationDate.mockResolvedValue(oldDate);
@@ -126,6 +154,7 @@ describe("UnifiedUpgradePromptService", () => {
//Arrange //Arrange
mockConfigService.getFeatureFlag$.mockReturnValue(of(true)); mockConfigService.getFeatureFlag$.mockReturnValue(of(true));
mockBillingService.hasPremiumFromAnySource$.mockReturnValue(of(false)); mockBillingService.hasPremiumFromAnySource$.mockReturnValue(of(false));
mockOrganizationService.memberOrganizations$.mockReturnValue(of([]));
const recentDate = new Date(); const recentDate = new Date();
recentDate.setMinutes(recentDate.getMinutes() - 3); // 3 minutes old recentDate.setMinutes(recentDate.getMinutes() - 3); // 3 minutes old
mockVaultProfileService.getProfileCreationDate.mockResolvedValue(recentDate); mockVaultProfileService.getProfileCreationDate.mockResolvedValue(recentDate);
@@ -159,6 +188,7 @@ describe("UnifiedUpgradePromptService", () => {
// Arrange // Arrange
mockConfigService.getFeatureFlag$.mockReturnValue(of(true)); mockConfigService.getFeatureFlag$.mockReturnValue(of(true));
mockBillingService.hasPremiumFromAnySource$.mockReturnValue(of(false)); mockBillingService.hasPremiumFromAnySource$.mockReturnValue(of(false));
mockOrganizationService.memberOrganizations$.mockReturnValue(of([]));
mockVaultProfileService.getProfileCreationDate.mockResolvedValue(null); mockVaultProfileService.getProfileCreationDate.mockResolvedValue(null);
setupTestService(); setupTestService();

View File

@@ -1,12 +1,14 @@
import { Injectable } from "@angular/core"; import { Injectable } from "@angular/core";
import { combineLatest, firstValueFrom } from "rxjs"; import { combineLatest, firstValueFrom, timeout } from "rxjs";
import { switchMap, take } from "rxjs/operators"; import { filter, switchMap, take } from "rxjs/operators";
import { VaultProfileService } from "@bitwarden/angular/vault/services/vault-profile.service"; import { VaultProfileService } from "@bitwarden/angular/vault/services/vault-profile.service";
import { OrganizationService } from "@bitwarden/common/admin-console/abstractions/organization/organization.service.abstraction";
import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; import { AccountService } from "@bitwarden/common/auth/abstractions/account.service";
import { BillingAccountProfileStateService } from "@bitwarden/common/billing/abstractions/account/billing-account-profile-state.service"; import { BillingAccountProfileStateService } from "@bitwarden/common/billing/abstractions/account/billing-account-profile-state.service";
import { FeatureFlag } from "@bitwarden/common/enums/feature-flag.enum"; import { FeatureFlag } from "@bitwarden/common/enums/feature-flag.enum";
import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service"; import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service";
import { SyncService } from "@bitwarden/common/platform/sync/sync.service";
import { DialogRef, DialogService } from "@bitwarden/components"; import { DialogRef, DialogService } from "@bitwarden/components";
import { import {
@@ -24,7 +26,9 @@ export class UnifiedUpgradePromptService {
private configService: ConfigService, private configService: ConfigService,
private billingAccountProfileStateService: BillingAccountProfileStateService, private billingAccountProfileStateService: BillingAccountProfileStateService,
private vaultProfileService: VaultProfileService, private vaultProfileService: VaultProfileService,
private syncService: SyncService,
private dialogService: DialogService, private dialogService: DialogService,
private organizationService: OrganizationService,
) {} ) {}
private shouldShowPrompt$ = combineLatest([ private shouldShowPrompt$ = combineLatest([
@@ -40,6 +44,19 @@ export class UnifiedUpgradePromptService {
return false; 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
),
);
// Check if user has premium // Check if user has premium
const hasPremium = await firstValueFrom( const hasPremium = await firstValueFrom(
this.billingAccountProfileStateService.hasPremiumFromAnySource$(account.id), this.billingAccountProfileStateService.hasPremiumFromAnySource$(account.id),
@@ -50,12 +67,25 @@ export class UnifiedUpgradePromptService {
return false; 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 // Check profile age only if needed
const isProfileLessThanFiveMinutesOld = await this.isProfileLessThanFiveMinutesOld( const isProfileLessThanFiveMinutesOld = await this.isProfileLessThanFiveMinutesOld(
account.id, account.id,
); );
return isFlagEnabled && !hasPremium && isProfileLessThanFiveMinutesOld; return isFlagEnabled && !hasPremium && !hasOrganizations && isProfileLessThanFiveMinutesOld;
}), }),
take(1), take(1),
); );

View File

@@ -620,7 +620,7 @@ export class VaultComponent<C extends CipherViewLike> implements OnInit, OnDestr
this.changeDetectorRef.markForCheck(); this.changeDetectorRef.markForCheck();
}, },
); );
await this.unifiedUpgradePromptService.displayUpgradePromptConditionally(); void this.unifiedUpgradePromptService.displayUpgradePromptConditionally();
} }
ngOnDestroy() { ngOnDestroy() {