[PM-7071] Fallback to Emitting `null` When No Active User (#8486)

* Fallback to Emitting `null` When No Active User

* Fix Tests

* Update Test Names to Follow Convention

Co-authored-by: Andreas Coroiu <acoroiu@bitwarden.com>

* Fix CLI Build

---------

Co-authored-by: Andreas Coroiu <acoroiu@bitwarden.com>
This commit is contained in:
Justin Baur 2024-03-26 11:06:33 -05:00 committed by GitHub
parent 7f14ee4994
commit f7014a973c
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
6 changed files with 108 additions and 122 deletions

View File

@ -576,7 +576,7 @@ export default class MainBackground {
); );
this.billingAccountProfileStateService = new DefaultBillingAccountProfileStateService( this.billingAccountProfileStateService = new DefaultBillingAccountProfileStateService(
this.activeUserStateProvider, this.stateProvider,
); );
this.loginStrategyService = new LoginStrategyService( this.loginStrategyService = new LoginStrategyService(

View File

@ -1,9 +1,8 @@
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 { DefaultBillingAccountProfileStateService } from "@bitwarden/common/billing/services/account/billing-account-profile-state.service"; import { DefaultBillingAccountProfileStateService } from "@bitwarden/common/billing/services/account/billing-account-profile-state.service";
import { activeUserStateProviderFactory } from "./active-user-state-provider.factory";
import { FactoryOptions, CachedServices, factory } from "./factory-options"; import { FactoryOptions, CachedServices, factory } from "./factory-options";
import { StateProviderInitOptions } from "./state-provider.factory"; import { StateProviderInitOptions, stateProviderFactory } from "./state-provider.factory";
type BillingAccountProfileStateServiceFactoryOptions = FactoryOptions; type BillingAccountProfileStateServiceFactoryOptions = FactoryOptions;
@ -21,8 +20,6 @@ export function billingAccountProfileStateServiceFactory(
"billingAccountProfileStateService", "billingAccountProfileStateService",
opts, opts,
async () => async () =>
new DefaultBillingAccountProfileStateService( new DefaultBillingAccountProfileStateService(await stateProviderFactory(cache, opts)),
await activeUserStateProviderFactory(cache, opts),
),
); );
} }

View File

@ -467,7 +467,7 @@ export class Main {
); );
this.billingAccountProfileStateService = new DefaultBillingAccountProfileStateService( this.billingAccountProfileStateService = new DefaultBillingAccountProfileStateService(
this.activeUserStateProvider, this.stateProvider,
); );
this.loginStrategyService = new LoginStrategyService( this.loginStrategyService = new LoginStrategyService(

View File

@ -1067,7 +1067,7 @@ const typesafeProviders: Array<SafeProvider> = [
safeProvider({ safeProvider({
provide: BillingAccountProfileStateService, provide: BillingAccountProfileStateService,
useClass: DefaultBillingAccountProfileStateService, useClass: DefaultBillingAccountProfileStateService,
deps: [ActiveUserStateProvider], deps: [StateProvider],
}), }),
safeProvider({ safeProvider({
provide: OrganizationManagementPreferencesService, provide: OrganizationManagementPreferencesService,

View File

@ -2,10 +2,10 @@ import { firstValueFrom } from "rxjs";
import { import {
FakeAccountService, FakeAccountService,
FakeActiveUserStateProvider,
mockAccountServiceWith, mockAccountServiceWith,
FakeActiveUserState, FakeActiveUserState,
trackEmissions, FakeStateProvider,
FakeSingleUserState,
} from "../../../../spec"; } from "../../../../spec";
import { UserId } from "../../../types/guid"; import { UserId } from "../../../types/guid";
import { BillingAccountProfile } from "../../abstractions/account/billing-account-profile-state.service"; import { BillingAccountProfile } from "../../abstractions/account/billing-account-profile-state.service";
@ -16,20 +16,26 @@ import {
} from "./billing-account-profile-state.service"; } from "./billing-account-profile-state.service";
describe("BillingAccountProfileStateService", () => { describe("BillingAccountProfileStateService", () => {
let activeUserStateProvider: FakeActiveUserStateProvider; let stateProvider: FakeStateProvider;
let sut: DefaultBillingAccountProfileStateService; let sut: DefaultBillingAccountProfileStateService;
let billingAccountProfileState: FakeActiveUserState<BillingAccountProfile>; let billingAccountProfileState: FakeActiveUserState<BillingAccountProfile>;
let userBillingAccountProfileState: FakeSingleUserState<BillingAccountProfile>;
let accountService: FakeAccountService; let accountService: FakeAccountService;
const userId = "fakeUserId" as UserId; const userId = "fakeUserId" as UserId;
beforeEach(() => { beforeEach(() => {
accountService = mockAccountServiceWith(userId); accountService = mockAccountServiceWith(userId);
activeUserStateProvider = new FakeActiveUserStateProvider(accountService); stateProvider = new FakeStateProvider(accountService);
sut = new DefaultBillingAccountProfileStateService(activeUserStateProvider); sut = new DefaultBillingAccountProfileStateService(stateProvider);
billingAccountProfileState = activeUserStateProvider.getFake( billingAccountProfileState = stateProvider.activeUser.getFake(
BILLING_ACCOUNT_PROFILE_KEY_DEFINITION,
);
userBillingAccountProfileState = stateProvider.singleUser.getFake(
userId,
BILLING_ACCOUNT_PROFILE_KEY_DEFINITION, BILLING_ACCOUNT_PROFILE_KEY_DEFINITION,
); );
}); });
@ -38,9 +44,9 @@ describe("BillingAccountProfileStateService", () => {
return jest.resetAllMocks(); return jest.resetAllMocks();
}); });
describe("accountHasPremiumFromAnyOrganization$", () => { describe("hasPremiumFromAnyOrganization$", () => {
it("should emit changes in hasPremiumFromAnyOrganization", async () => { it("returns true when they have premium from an organization", async () => {
billingAccountProfileState.nextState({ userBillingAccountProfileState.nextState({
hasPremiumPersonally: false, hasPremiumPersonally: false,
hasPremiumFromAnyOrganization: true, hasPremiumFromAnyOrganization: true,
}); });
@ -48,118 +54,91 @@ describe("BillingAccountProfileStateService", () => {
expect(await firstValueFrom(sut.hasPremiumFromAnyOrganization$)).toBe(true); expect(await firstValueFrom(sut.hasPremiumFromAnyOrganization$)).toBe(true);
}); });
it("should emit once when calling setHasPremium once", async () => { it("return false when they do not have premium from an organization", async () => {
const emissions = trackEmissions(sut.hasPremiumFromAnyOrganization$); userBillingAccountProfileState.nextState({
const startingEmissionCount = emissions.length;
await sut.setHasPremium(true, true);
const endingEmissionCount = emissions.length;
expect(endingEmissionCount - startingEmissionCount).toBe(1);
});
});
describe("hasPremiumPersonally$", () => {
it("should emit changes in hasPremiumPersonally", async () => {
billingAccountProfileState.nextState({
hasPremiumPersonally: true,
hasPremiumFromAnyOrganization: false,
});
expect(await firstValueFrom(sut.hasPremiumPersonally$)).toBe(true);
});
it("should emit once when calling setHasPremium once", async () => {
const emissions = trackEmissions(sut.hasPremiumPersonally$);
const startingEmissionCount = emissions.length;
await sut.setHasPremium(true, true);
const endingEmissionCount = emissions.length;
expect(endingEmissionCount - startingEmissionCount).toBe(1);
});
});
describe("canAccessPremium$", () => {
it("should emit changes in hasPremiumPersonally", async () => {
billingAccountProfileState.nextState({
hasPremiumPersonally: true,
hasPremiumFromAnyOrganization: false,
});
expect(await firstValueFrom(sut.hasPremiumFromAnySource$)).toBe(true);
});
it("should emit changes in hasPremiumFromAnyOrganization", async () => {
billingAccountProfileState.nextState({
hasPremiumPersonally: false, hasPremiumPersonally: false,
hasPremiumFromAnyOrganization: true, hasPremiumFromAnyOrganization: false,
}); });
expect(await firstValueFrom(sut.hasPremiumFromAnySource$)).toBe(true);
});
it("should emit changes in both hasPremiumPersonally and hasPremiumFromAnyOrganization", async () => {
billingAccountProfileState.nextState({
hasPremiumPersonally: true,
hasPremiumFromAnyOrganization: true,
});
expect(await firstValueFrom(sut.hasPremiumFromAnySource$)).toBe(true);
});
it("should emit once when calling setHasPremium once", async () => {
const emissions = trackEmissions(sut.hasPremiumFromAnySource$);
const startingEmissionCount = emissions.length;
await sut.setHasPremium(true, true);
const endingEmissionCount = emissions.length;
expect(endingEmissionCount - startingEmissionCount).toBe(1);
});
});
describe("setHasPremium", () => {
it("should have `hasPremiumPersonally$` emit `true` when passing `true` as an argument for hasPremiumPersonally", async () => {
await sut.setHasPremium(true, false);
expect(await firstValueFrom(sut.hasPremiumPersonally$)).toBe(true);
});
it("should have `hasPremiumFromAnyOrganization$` emit `true` when passing `true` as an argument for hasPremiumFromAnyOrganization", async () => {
await sut.setHasPremium(false, true);
expect(await firstValueFrom(sut.hasPremiumFromAnyOrganization$)).toBe(true);
});
it("should have `hasPremiumPersonally$` emit `false` when passing `false` as an argument for hasPremiumPersonally", async () => {
await sut.setHasPremium(false, false);
expect(await firstValueFrom(sut.hasPremiumPersonally$)).toBe(false);
});
it("should have `hasPremiumFromAnyOrganization$` emit `false` when passing `false` as an argument for hasPremiumFromAnyOrganization", async () => {
await sut.setHasPremium(false, false);
expect(await firstValueFrom(sut.hasPremiumFromAnyOrganization$)).toBe(false); expect(await firstValueFrom(sut.hasPremiumFromAnyOrganization$)).toBe(false);
}); });
it("should have `canAccessPremium$` emit `true` when passing `true` as an argument for hasPremiumPersonally", async () => { it("returns false when there is no active user", async () => {
await sut.setHasPremium(true, false); await accountService.switchAccount(null);
expect(await firstValueFrom(sut.hasPremiumFromAnyOrganization$)).toBe(false);
});
});
describe("hasPremiumPersonally$", () => {
it("returns true when the user has premium personally", async () => {
userBillingAccountProfileState.nextState({
hasPremiumPersonally: true,
hasPremiumFromAnyOrganization: false,
});
expect(await firstValueFrom(sut.hasPremiumPersonally$)).toBe(true);
});
it("returns false when the user does not have premium personally", async () => {
userBillingAccountProfileState.nextState({
hasPremiumPersonally: false,
hasPremiumFromAnyOrganization: false,
});
expect(await firstValueFrom(sut.hasPremiumPersonally$)).toBe(false);
});
it("returns false when there is no active user", async () => {
await accountService.switchAccount(null);
expect(await firstValueFrom(sut.hasPremiumPersonally$)).toBe(false);
});
});
describe("hasPremiumFromAnySource$", () => {
it("returns true when the user has premium personally", async () => {
userBillingAccountProfileState.nextState({
hasPremiumPersonally: true,
hasPremiumFromAnyOrganization: false,
});
expect(await firstValueFrom(sut.hasPremiumFromAnySource$)).toBe(true); expect(await firstValueFrom(sut.hasPremiumFromAnySource$)).toBe(true);
}); });
it("should have `canAccessPremium$` emit `true` when passing `true` as an argument for hasPremiumFromAnyOrganization", async () => { it("returns true when the user has premium from an organization", async () => {
await sut.setHasPremium(false, true); userBillingAccountProfileState.nextState({
hasPremiumPersonally: false,
hasPremiumFromAnyOrganization: true,
});
expect(await firstValueFrom(sut.hasPremiumFromAnySource$)).toBe(true); expect(await firstValueFrom(sut.hasPremiumFromAnySource$)).toBe(true);
}); });
it("should have `canAccessPremium$` emit `false` when passing `false` for all arguments", async () => { it("returns true when they have premium personally AND from an organization", async () => {
await sut.setHasPremium(false, false); userBillingAccountProfileState.nextState({
hasPremiumPersonally: true,
hasPremiumFromAnyOrganization: true,
});
expect(await firstValueFrom(sut.hasPremiumFromAnySource$)).toBe(true);
});
it("returns false when there is no active user", async () => {
await accountService.switchAccount(null);
expect(await firstValueFrom(sut.hasPremiumFromAnySource$)).toBe(false); expect(await firstValueFrom(sut.hasPremiumFromAnySource$)).toBe(false);
}); });
}); });
describe("setHasPremium", () => {
it("should update the active users state when called", async () => {
await sut.setHasPremium(true, false);
expect(billingAccountProfileState.nextMock).toHaveBeenCalledWith([
userId,
{ hasPremiumPersonally: true, hasPremiumFromAnyOrganization: false },
]);
});
});
}); });

View File

@ -1,10 +1,10 @@
import { map, Observable } from "rxjs"; import { map, Observable, of, switchMap } from "rxjs";
import { import {
ActiveUserState, ActiveUserState,
ActiveUserStateProvider,
BILLING_DISK, BILLING_DISK,
KeyDefinition, KeyDefinition,
StateProvider,
} from "../../../platform/state"; } from "../../../platform/state";
import { import {
BillingAccountProfile, BillingAccountProfile,
@ -26,24 +26,34 @@ export class DefaultBillingAccountProfileStateService implements BillingAccountP
hasPremiumPersonally$: Observable<boolean>; hasPremiumPersonally$: Observable<boolean>;
hasPremiumFromAnySource$: Observable<boolean>; hasPremiumFromAnySource$: Observable<boolean>;
constructor(activeUserStateProvider: ActiveUserStateProvider) { constructor(stateProvider: StateProvider) {
this.billingAccountProfileState = activeUserStateProvider.get( this.billingAccountProfileState = stateProvider.getActive(
BILLING_ACCOUNT_PROFILE_KEY_DEFINITION, BILLING_ACCOUNT_PROFILE_KEY_DEFINITION,
); );
this.hasPremiumFromAnyOrganization$ = this.billingAccountProfileState.state$.pipe( // Setup an observable that will always track the currently active user
// but will fallback to emitting null when there is no active user.
const billingAccountProfileOrNull = stateProvider.activeUserId$.pipe(
switchMap((userId) =>
userId != null
? stateProvider.getUser(userId, BILLING_ACCOUNT_PROFILE_KEY_DEFINITION).state$
: of(null),
),
);
this.hasPremiumFromAnyOrganization$ = billingAccountProfileOrNull.pipe(
map((billingAccountProfile) => !!billingAccountProfile?.hasPremiumFromAnyOrganization), map((billingAccountProfile) => !!billingAccountProfile?.hasPremiumFromAnyOrganization),
); );
this.hasPremiumPersonally$ = this.billingAccountProfileState.state$.pipe( this.hasPremiumPersonally$ = billingAccountProfileOrNull.pipe(
map((billingAccountProfile) => !!billingAccountProfile?.hasPremiumPersonally), map((billingAccountProfile) => !!billingAccountProfile?.hasPremiumPersonally),
); );
this.hasPremiumFromAnySource$ = this.billingAccountProfileState.state$.pipe( this.hasPremiumFromAnySource$ = billingAccountProfileOrNull.pipe(
map( map(
(billingAccountProfile) => (billingAccountProfile) =>
billingAccountProfile?.hasPremiumFromAnyOrganization || billingAccountProfile?.hasPremiumFromAnyOrganization === true ||
billingAccountProfile?.hasPremiumPersonally, billingAccountProfile?.hasPremiumPersonally === true,
), ),
); );
} }