[PM-5272] Migrate CollapsedGroupings to State Provider (#7954)

2025-12-14 15:23:33 +00:00 · 2024-02-16 12:53:24 -05:00
parent b0dd64bab4
commit 5b652092cd
16 changed files with 245 additions and 81 deletions
--- a/apps/browser/src/background/main.background.ts
+++ b/apps/browser/src/background/main.background.ts
@@ -607,12 +607,12 @@ export default class MainBackground {
    );

    this.vaultFilterService = new VaultFilterService(
-      this.stateService,
      this.organizationService,
      this.folderService,
      this.cipherService,
      this.collectionService,
      this.policyService,
+      this.stateProvider,
      this.accountService,
    );

--- a/apps/browser/src/popup/services/services.module.ts
+++ b/apps/browser/src/popup/services/services.module.ts
@@ -427,28 +427,14 @@ function getBgService<T>(service: keyof MainBackground) {
    },
    {
      provide: VaultFilterService,
-      useFactory: (
-        stateService: StateServiceAbstraction,
-        organizationService: OrganizationService,
-        folderService: FolderServiceAbstraction,
-        policyService: PolicyService,
-        accountService: AccountServiceAbstraction,
-      ) => {
-        return new VaultFilterService(
-          stateService,
-          organizationService,
-          folderService,
-          getBgService<CipherService>("cipherService")(),
-          getBgService<CollectionService>("collectionService")(),
-          policyService,
-          accountService,
-        );
-      },
+      useClass: VaultFilterService,
      deps: [
-        StateServiceAbstraction,
        OrganizationService,
        FolderServiceAbstraction,
+        CipherService,
+        CollectionService,
        PolicyService,
+        StateProvider,
        AccountServiceAbstraction,
      ],
    },
--- a/apps/browser/src/vault/services/vault-filter.service.ts
+++ b/apps/browser/src/vault/services/vault-filter.service.ts
@@ -3,7 +3,7 @@ import { VaultFilterService as BaseVaultFilterService } from "@bitwarden/angular
 import { OrganizationService } from "@bitwarden/common/admin-console/abstractions/organization/organization.service.abstraction";
 import { PolicyService } from "@bitwarden/common/admin-console/abstractions/policy/policy.service.abstraction";
 import { AccountService } from "@bitwarden/common/auth/abstractions/account.service";
-import { StateService } from "@bitwarden/common/platform/abstractions/state.service";
+import { StateProvider } from "@bitwarden/common/platform/state";
 import { CipherService } from "@bitwarden/common/vault/abstractions/cipher.service";
 import { CollectionService } from "@bitwarden/common/vault/abstractions/collection.service";
 import { FolderService } from "@bitwarden/common/vault/abstractions/folder/folder.service.abstraction";
@@ -16,21 +16,21 @@ export class VaultFilterService extends BaseVaultFilterService {
  myVault = "myVault";

  constructor(
-    stateService: StateService,
    organizationService: OrganizationService,
    folderService: FolderService,
    cipherService: CipherService,
    collectionService: CollectionService,
    policyService: PolicyService,
+    stateProvider: StateProvider,
    private accountService: AccountService,
  ) {
    super(
-      stateService,
      organizationService,
      folderService,
      cipherService,
      collectionService,
      policyService,
+      stateProvider,
    );
    this.vaultFilter.myVaultOnly = false;
    this.vaultFilter.selectedOrganizationId = null;
--- a/apps/web/src/app/vault/individual-vault/vault-filter/services/vault-filter.service.spec.ts
+++ b/apps/web/src/app/vault/individual-vault/vault-filter/services/vault-filter.service.spec.ts
@@ -1,24 +1,31 @@
+import {
+  FakeAccountService,
+  mockAccountServiceWith,
+} from "@bitwarden/common/../spec/fake-account-service";
+import { FakeActiveUserState } from "@bitwarden/common/../spec/fake-state";
+import { FakeStateProvider } from "@bitwarden/common/../spec/fake-state-provider";
 import { mock, MockProxy } from "jest-mock-extended";
-import { firstValueFrom, ReplaySubject, take } from "rxjs";
+import { firstValueFrom, ReplaySubject } from "rxjs";

 import { OrganizationService } from "@bitwarden/common/admin-console/abstractions/organization/organization.service.abstraction";
 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 { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service";
-import { StateService } from "@bitwarden/common/platform/abstractions/state.service";
+import { Utils } from "@bitwarden/common/platform/misc/utils";
+import { UserId } from "@bitwarden/common/types/guid";
 import { CipherService } from "@bitwarden/common/vault/abstractions/cipher.service";
 import { FolderService } from "@bitwarden/common/vault/abstractions/folder/folder.service.abstraction";
 import { CipherView } from "@bitwarden/common/vault/models/view/cipher.view";
 import { CollectionView } from "@bitwarden/common/vault/models/view/collection.view";
 import { FolderView } from "@bitwarden/common/vault/models/view/folder.view";
+import { COLLAPSED_GROUPINGS } from "@bitwarden/common/vault/services/key-state/collapsed-groupings.state";

 import { VaultFilterService } from "./vault-filter.service";

 describe("vault filter service", () => {
  let vaultFilterService: VaultFilterService;

-  let stateService: MockProxy<StateService>;
  let organizationService: MockProxy<OrganizationService>;
  let folderService: MockProxy<FolderService>;
  let cipherService: MockProxy<CipherService>;
@@ -26,14 +33,20 @@ describe("vault filter service", () => {
  let i18nService: MockProxy<I18nService>;
  let organizations: ReplaySubject<Organization[]>;
  let folderViews: ReplaySubject<FolderView[]>;
+  let stateProvider: FakeStateProvider;
+
+  const mockUserId = Utils.newGuid() as UserId;
+  let accountService: FakeAccountService;
+  let collapsedGroupingsState: FakeActiveUserState<string[]>;

  beforeEach(() => {
-    stateService = mock<StateService>();
    organizationService = mock<OrganizationService>();
    folderService = mock<FolderService>();
    cipherService = mock<CipherService>();
    policyService = mock<PolicyService>();
    i18nService = mock<I18nService>();
+    accountService = mockAccountServiceWith(mockUserId);
+    stateProvider = new FakeStateProvider(accountService);
    i18nService.collator = new Intl.Collator("en-US");

    organizations = new ReplaySubject<Organization[]>(1);
@@ -43,31 +56,32 @@ describe("vault filter service", () => {
    folderService.folderViews$ = folderViews;

    vaultFilterService = new VaultFilterService(
-      stateService,
      organizationService,
      folderService,
      cipherService,
      policyService,
      i18nService,
+      stateProvider,
    );
+    collapsedGroupingsState = stateProvider.activeUser.getFake(COLLAPSED_GROUPINGS);
  });

  describe("collapsed filter nodes", () => {
    const nodes = new Set(["1", "2"]);
-    it("updates observable when saving", (complete) => {
-      vaultFilterService.collapsedFilterNodes$.pipe(take(1)).subscribe((value) => {
-        if (value === nodes) {
-          complete();
-        }
-      });

-      // FIXME: Verify that this floating promise is intentional. If it is, add an explanatory comment and ensure there is proper error handling.
-      // eslint-disable-next-line @typescript-eslint/no-floating-promises
-      vaultFilterService.setCollapsedFilterNodes(nodes);
+    it("should update the collapsedFilterNodes$", async () => {
+      await vaultFilterService.setCollapsedFilterNodes(nodes);
+
+      const collapsedGroupingsState = stateProvider.activeUser.getFake(COLLAPSED_GROUPINGS);
+      expect(await firstValueFrom(collapsedGroupingsState.state$)).toEqual(Array.from(nodes));
+      expect(collapsedGroupingsState.nextMock).toHaveBeenCalledWith([
+        mockUserId,
+        Array.from(nodes),
+      ]);
    });

    it("loads from state on initialization", async () => {
-      stateService.getCollapsedGroupings.mockResolvedValue(["1", "2"]);
+      collapsedGroupingsState.nextState(["1", "2"]);

      await expect(firstValueFrom(vaultFilterService.collapsedFilterNodes$)).resolves.toEqual(
        nodes,
--- a/apps/web/src/app/vault/individual-vault/vault-filter/services/vault-filter.service.ts
+++ b/apps/web/src/app/vault/individual-vault/vault-filter/services/vault-filter.service.ts
@@ -16,7 +16,7 @@ import { PolicyService } from "@bitwarden/common/admin-console/abstractions/poli
 import { PolicyType } from "@bitwarden/common/admin-console/enums";
 import { Organization } from "@bitwarden/common/admin-console/models/domain/organization";
 import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service";
-import { StateService } from "@bitwarden/common/platform/abstractions/state.service";
+import { ActiveUserState, StateProvider } from "@bitwarden/common/platform/state";
 import { CipherService } from "@bitwarden/common/vault/abstractions/cipher.service";
 import { FolderService } from "@bitwarden/common/vault/abstractions/folder/folder.service.abstraction";
 import { CipherType } from "@bitwarden/common/vault/enums";
@@ -24,6 +24,7 @@ import { TreeNode } from "@bitwarden/common/vault/models/domain/tree-node";
 import { CollectionView } from "@bitwarden/common/vault/models/view/collection.view";
 import { FolderView } from "@bitwarden/common/vault/models/view/folder.view";
 import { ServiceUtils } from "@bitwarden/common/vault/service-utils";
+import { COLLAPSED_GROUPINGS } from "@bitwarden/common/vault/services/key-state/collapsed-groupings.state";

 import { CollectionAdminView } from "../../../core/views/collection-admin.view";
 import {
@@ -39,11 +40,6 @@ const NestingDelimiter = "/";

@Injectable()
 export class VaultFilterService implements VaultFilterServiceAbstraction {
-  protected _collapsedFilterNodes = new BehaviorSubject<Set<string>>(null);
-  collapsedFilterNodes$: Observable<Set<string>> = this._collapsedFilterNodes.pipe(
-    switchMap(async (nodes) => nodes ?? (await this.getCollapsedFilterNodes())),
-  );
-
  organizationTree$: Observable<TreeNode<OrganizationFilter>> =
    this.organizationService.memberOrganizations$.pipe(
      switchMap((orgs) => this.buildOrganizationTree(orgs)),
@@ -78,13 +74,19 @@ export class VaultFilterService implements VaultFilterServiceAbstraction {

  cipherTypeTree$: Observable<TreeNode<CipherTypeFilter>> = this.buildCipherTypeTree();

+  private collapsedGroupingsState: ActiveUserState<string[]> =
+    this.stateProvider.getActive(COLLAPSED_GROUPINGS);
+
+  readonly collapsedFilterNodes$: Observable<Set<string>> =
+    this.collapsedGroupingsState.state$.pipe(map((c) => new Set(c)));
+
  constructor(
-    protected stateService: StateService,
    protected organizationService: OrganizationService,
    protected folderService: FolderService,
    protected cipherService: CipherService,
    protected policyService: PolicyService,
    protected i18nService: I18nService,
+    protected stateProvider: StateProvider,
  ) {}

  async reloadCollections(collections: CollectionView[]) {
@@ -97,13 +99,11 @@ export class VaultFilterService implements VaultFilterServiceAbstraction {
  }

  async setCollapsedFilterNodes(collapsedFilterNodes: Set<string>): Promise<void> {
-    await this.stateService.setCollapsedGroupings(Array.from(collapsedFilterNodes));
-    this._collapsedFilterNodes.next(collapsedFilterNodes);
+    await this.collapsedGroupingsState.update(() => Array.from(collapsedFilterNodes));
  }

  protected async getCollapsedFilterNodes(): Promise<Set<string>> {
-    const nodes = new Set(await this.stateService.getCollapsedGroupings());
-    return nodes;
+    return await firstValueFrom(this.collapsedFilterNodes$);
  }

  getOrganizationFilter() {
--- a/apps/web/src/app/vault/org-vault/vault-filter/vault-filter.service.ts
+++ b/apps/web/src/app/vault/org-vault/vault-filter/vault-filter.service.ts
@@ -4,11 +4,11 @@ import { map, Observable, ReplaySubject, Subject } from "rxjs";
 import { OrganizationService } from "@bitwarden/common/admin-console/abstractions/organization/organization.service.abstraction";
 import { PolicyService } from "@bitwarden/common/admin-console/abstractions/policy/policy.service.abstraction";
 import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service";
+import { StateProvider } from "@bitwarden/common/platform/state";
 import { CipherService } from "@bitwarden/common/vault/abstractions/cipher.service";
 import { FolderService } from "@bitwarden/common/vault/abstractions/folder/folder.service.abstraction";
 import { TreeNode } from "@bitwarden/common/vault/models/domain/tree-node";

-import { StateService } from "../../../core";
 import { CollectionAdminView } from "../../../vault/core/views/collection-admin.view";
 import { CollectionAdminService } from "../../core/collection-admin.service";
 import { VaultFilterService as BaseVaultFilterService } from "../../individual-vault/vault-filter/services/vault-filter.service";
@@ -26,21 +26,21 @@ export class VaultFilterService extends BaseVaultFilterService implements OnDest
  );

  constructor(
-    stateService: StateService,
    organizationService: OrganizationService,
    folderService: FolderService,
    cipherService: CipherService,
    policyService: PolicyService,
    i18nService: I18nService,
+    stateProvider: StateProvider,
    protected collectionAdminService: CollectionAdminService,
  ) {
    super(
-      stateService,
      organizationService,
      folderService,
      cipherService,
      policyService,
      i18nService,
+      stateProvider,
    );
  }