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-12036] Removing ActiveUserState from vault-onboarding.service.ts #12898

Open
wants to merge 17 commits into
base: main
Choose a base branch
from
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
// FIXME: Update this file to be type safe and remove this and next line
// @ts-strict-ignore
import { Observable } from "rxjs";

import { UserId } from "@bitwarden/common/types/guid";

import { VaultOnboardingTasks } from "../vault-onboarding.service";

export abstract class VaultOnboardingService {
vaultOnboardingState$: Observable<VaultOnboardingTasks>;
abstract setVaultOnboardingTasks(newState: VaultOnboardingTasks): Promise<void>;
abstract setVaultOnboardingTasks(userId: UserId, newState: VaultOnboardingTasks): Promise<void>;
abstract vaultOnboardingState$(userId: UserId): Observable<VaultOnboardingTasks | null>;
}
Original file line number Diff line number Diff line change
@@ -1,14 +1,13 @@
// FIXME: Update this file to be type safe and remove this and next line
// @ts-strict-ignore
import { Injectable } from "@angular/core";
import { Observable } from "rxjs";

import {
ActiveUserState,
SingleUserState,
StateProvider,
UserKeyDefinition,
VAULT_ONBOARDING,
} from "@bitwarden/common/platform/state";
import { UserId } from "@bitwarden/common/types/guid";

import { VaultOnboardingService as VaultOnboardingServiceAbstraction } from "./abstraction/vault-onboarding.service";

Expand All @@ -26,20 +25,20 @@
clearOn: [], // do not clear tutorials
},
);

@Injectable()
export class VaultOnboardingService implements VaultOnboardingServiceAbstraction {
private vaultOnboardingState: ActiveUserState<VaultOnboardingTasks>;
vaultOnboardingState$: Observable<VaultOnboardingTasks>;
constructor(private stateProvider: StateProvider) {}

private vaultOnboardingState(userId: UserId): SingleUserState<VaultOnboardingTasks> {
return this.stateProvider.getUser(userId, VAULT_ONBOARDING_KEY);
}

constructor(private stateProvider: StateProvider) {
this.vaultOnboardingState = this.stateProvider.getActive(VAULT_ONBOARDING_KEY);
this.vaultOnboardingState$ = this.vaultOnboardingState.state$;
vaultOnboardingState$(userId: UserId): Observable<VaultOnboardingTasks | null> {
return this.vaultOnboardingState(userId).state$;

Check warning on line 37 in apps/web/src/app/vault/individual-vault/vault-onboarding/services/vault-onboarding.service.ts

View check run for this annotation

Codecov / codecov/patch

apps/web/src/app/vault/individual-vault/vault-onboarding/services/vault-onboarding.service.ts#L37

Added line #L37 was not covered by tests
}

async setVaultOnboardingTasks(newState: VaultOnboardingTasks): Promise<void> {
await this.vaultOnboardingState.update(() => {
return { ...newState };
});
async setVaultOnboardingTasks(userId: UserId, newState: VaultOnboardingTasks): Promise<void> {
const state = this.vaultOnboardingState(userId);
await state.update(() => ({ ...newState }));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,18 @@ import { Subject, of } from "rxjs";

import { ApiService } from "@bitwarden/common/abstractions/api.service";
import { PolicyService } from "@bitwarden/common/admin-console/abstractions/policy/policy.service.abstraction";
import { AccountService } from "@bitwarden/common/auth/abstractions/account.service";
import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service";
import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service";
import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service";
import { Utils } from "@bitwarden/common/platform/misc/utils";
import { StateProvider } from "@bitwarden/common/platform/state";
import {
FakeAccountService,
FakeStateProvider,
mockAccountServiceWith,
} from "@bitwarden/common/spec";
import { UserId } from "@bitwarden/common/types/guid";
import { CipherType } from "@bitwarden/common/vault/enums/cipher-type";
import { VaultOnboardingMessages } from "@bitwarden/common/vault/enums/vault-onboarding.enum";

Expand All @@ -25,10 +33,11 @@ describe("VaultOnboardingComponent", () => {
let mockPolicyService: MockProxy<PolicyService>;
let mockI18nService: MockProxy<I18nService>;
let mockVaultOnboardingService: MockProxy<VaultOnboardingServiceAbstraction>;
let mockStateProvider: Partial<StateProvider>;
let setInstallExtLinkSpy: any;
let individualVaultPolicyCheckSpy: any;
let mockConfigService: MockProxy<ConfigService>;
const mockAccountService: FakeAccountService = mockAccountServiceWith(Utils.newGuid() as UserId);
const fakeStateProvider = new FakeStateProvider(mockAccountService);

beforeEach(() => {
mockPolicyService = mock<PolicyService>();
Expand All @@ -38,15 +47,6 @@ describe("VaultOnboardingComponent", () => {
getProfile: jest.fn(),
};
mockVaultOnboardingService = mock<VaultOnboardingServiceAbstraction>();
mockStateProvider = {
getActive: jest.fn().mockReturnValue(
of({
createAccount: true,
importData: false,
installExtension: false,
}),
),
};
mockConfigService = mock<ConfigService>();

// eslint-disable-next-line @typescript-eslint/no-floating-promises
Expand All @@ -59,8 +59,9 @@ describe("VaultOnboardingComponent", () => {
{ provide: VaultOnboardingServiceAbstraction, useValue: mockVaultOnboardingService },
{ provide: I18nService, useValue: mockI18nService },
{ provide: ApiService, useValue: mockApiService },
{ provide: StateProvider, useValue: mockStateProvider },
{ provide: StateProvider, useValue: fakeStateProvider },
{ provide: ConfigService, useValue: mockConfigService },
{ provide: AccountService, useValue: mockAccountService },
],
}).compileComponents();
fixture = TestBed.createComponent(VaultOnboardingComponent);
Expand All @@ -71,11 +72,15 @@ describe("VaultOnboardingComponent", () => {
.mockReturnValue(undefined);
jest.spyOn(component, "checkCreationDate").mockReturnValue(null);
jest.spyOn(window, "postMessage").mockImplementation(jest.fn());
(component as any).vaultOnboardingService.vaultOnboardingState$ = of({
createAccount: true,
importData: false,
installExtension: false,
});
(component as any).vaultOnboardingService.vaultOnboardingState$ = jest
.fn()
.mockImplementation(() => {
return of({
createAccount: true,
importData: false,
installExtension: false,
});
});
});

it("should create", () => {
Expand Down Expand Up @@ -169,12 +174,15 @@ describe("VaultOnboardingComponent", () => {
.spyOn((component as any).vaultOnboardingService, "setVaultOnboardingTasks")
.mockReturnValue(Promise.resolve());

(component as any).vaultOnboardingService.vaultOnboardingState$ = of({
createAccount: true,
importData: false,
installExtension: false,
});

(component as any).vaultOnboardingService.vaultOnboardingState$ = jest
.fn()
.mockImplementation(() => {
return of({
createAccount: true,
importData: false,
installExtension: false,
});
});
const eventData = { data: { command: VaultOnboardingMessages.HasBwInstalled } };

(component as any).showOnboarding = true;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@
import { PolicyService } from "@bitwarden/common/admin-console/abstractions/policy/policy.service.abstraction";
import { PolicyType } from "@bitwarden/common/admin-console/enums";
import { Organization } from "@bitwarden/common/admin-console/models/domain/organization";
import { AccountService } from "@bitwarden/common/auth/abstractions/account.service";
import { getUserId } from "@bitwarden/common/auth/services/account.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";
Expand Down Expand Up @@ -66,10 +68,13 @@
private apiService: ApiService,
private vaultOnboardingService: VaultOnboardingServiceAbstraction,
private configService: ConfigService,
private accountService: AccountService,
) {}

async ngOnInit() {
this.onboardingTasks$ = this.vaultOnboardingService.vaultOnboardingState$;
const activeId = await firstValueFrom(getUserId(this.accountService.activeAccount$));
this.onboardingTasks$ = this.vaultOnboardingService.vaultOnboardingState$(activeId);

await this.setOnboardingTasks();
this.setInstallExtLink();
this.individualVaultPolicyCheck();
Expand All @@ -81,13 +86,14 @@

async ngOnChanges(changes: SimpleChanges) {
if (this.showOnboarding && changes?.ciphers) {
const activeId = await firstValueFrom(getUserId(this.accountService.activeAccount$));

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

View check run for this annotation

Codecov / codecov/patch

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

Added line #L89 was not covered by tests
const currentTasks = await firstValueFrom(this.onboardingTasks$);
const updatedTasks = {
createAccount: true,
importData: this.ciphers.length > 0,
installExtension: currentTasks.installExtension,
};
await this.vaultOnboardingService.setVaultOnboardingTasks(updatedTasks);
await this.vaultOnboardingService.setVaultOnboardingTasks(activeId, updatedTasks);

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

View check run for this annotation

Codecov / codecov/patch

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

Added line #L96 was not covered by tests
}
}

Expand All @@ -110,13 +116,14 @@

async getMessages(event: any) {
if (event.data.command === VaultOnboardingMessages.HasBwInstalled && this.showOnboarding) {
const activeId = await firstValueFrom(getUserId(this.accountService.activeAccount$));
const currentTasks = await firstValueFrom(this.onboardingTasks$);
const updatedTasks = {
createAccount: currentTasks.createAccount,
importData: currentTasks.importData,
installExtension: true,
};
await this.vaultOnboardingService.setVaultOnboardingTasks(updatedTasks);
await this.vaultOnboardingService.setVaultOnboardingTasks(activeId, updatedTasks);
}
}

Expand Down Expand Up @@ -159,7 +166,8 @@

private async saveCompletedTasks(vaultTasks: VaultOnboardingTasks) {
this.showOnboarding = Object.values(vaultTasks).includes(false);
await this.vaultOnboardingService.setVaultOnboardingTasks(vaultTasks);
const activeId = await firstValueFrom(getUserId(this.accountService.activeAccount$));
await this.vaultOnboardingService.setVaultOnboardingTasks(activeId, vaultTasks);
}

individualVaultPolicyCheck() {
Expand Down
Loading