[PM-22375] - [Vault] [Clients] Sort My Items collection to the top of Vault collection filters (#15332)

* WIP - default collection sorting

* apply filtering to popup list filters service.

* add tests. add feature flag checks

* finalize my items collection filters

* fix type error

* re-add service

* re-add comment

* remove unused code

* fix sorting logic

* shorten variable name to fit one line

* fix error

* fix more errors

* abstract logic to vault filter service

* fix test

* export sort as function instead of adding to class

* fix more tests

* add collator arg

* remove ts-ignore. fix type errors

* remove optional param

* fix vault filter service

apps/browser/src/background/main.background.ts

@@ -918,6 +918,8 @@ export default class MainBackground {
       this.policyService,
       this.stateProvider,
       this.accountService,
+      this.configService,
+      this.i18nService,
     );
 
     this.vaultSettingsService = new VaultSettingsService(this.stateProvider);

apps/browser/src/popup/services/services.module.ts

@@ -404,6 +404,8 @@ const safeProviders: SafeProvider[] = [
       PolicyService,
       StateProvider,
       AccountServiceAbstraction,
+      ConfigService,
+      I18nServiceAbstraction,
     ],
   }),
   safeProvider({

apps/browser/src/vault/popup/services/vault-popup-list-filters.service.spec.ts

@@ -5,12 +5,14 @@ import { BehaviorSubject, skipWhile } from "rxjs";
 
 import { CollectionService, CollectionView } from "@bitwarden/admin-console/common";
 import { ViewCacheService } from "@bitwarden/angular/platform/view-cache";
+import * as vaultFilterSvc from "@bitwarden/angular/vault/vault-filter/services/vault-filter.service";
 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 { AccountService } from "@bitwarden/common/auth/abstractions/account.service";
 import { ProductTierType } from "@bitwarden/common/billing/enums";
+import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service";
 import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service";
 import { StateProvider } from "@bitwarden/common/platform/state";
 import { mockAccountServiceWith } from "@bitwarden/common/spec";
@@ -31,6 +33,14 @@ import {
   VaultPopupListFiltersService,
 } from "./vault-popup-list-filters.service";
 
+const configService = {
+  getFeatureFlag$: jest.fn(() => new BehaviorSubject<boolean>(true)),
+} as unknown as ConfigService;
+
+jest.mock("@bitwarden/angular/vault/vault-filter/services/vault-filter.service", () => ({
+  sortDefaultCollections: jest.fn(),
+}));
+
 describe("VaultPopupListFiltersService", () => {
   let service: VaultPopupListFiltersService;
   let _memberOrganizations$ = new BehaviorSubject<Organization[]>([]);
@@ -138,6 +148,10 @@ describe("VaultPopupListFiltersService", () => {
           provide: RestrictedItemTypesService,
           useValue: restrictedItemTypesService,
         },
+        {
+          provide: ConfigService,
+          useValue: configService,
+        },
       ],
     });
 
@@ -399,6 +413,29 @@ describe("VaultPopupListFiltersService", () => {
         done();
       });
     });
+
+    it("calls vaultFilterService.sortDefaultCollections", (done) => {
+      const collections = [
+        { id: "1234", name: "Default Collection", organizationId: "org1" },
+        { id: "5678", name: "Shared Collection", organizationId: "org2" },
+      ] as CollectionView[];
+
+      const orgs = [
+        { id: "org1", name: "Organization 1" },
+        { id: "org2", name: "Organization 2" },
+      ] as Organization[];
+
+      createSeededVaultPopupListFiltersService(orgs, collections, [], {});
+
+      service.collections$.subscribe(() => {
+        expect(vaultFilterSvc.sortDefaultCollections).toHaveBeenCalledWith(
+          collections,
+          orgs,
+          i18nService.collator,
+        );
+        done();
+      });
+    });
   });
 
   describe("folders$", () => {
@@ -573,6 +610,8 @@ describe("VaultPopupListFiltersService", () => {
 
       const seededOrganizations: Organization[] = [
         { id: MY_VAULT_ID, name: "Test Org" } as Organization,
+        { id: "org1", name: "Default User Collection Org 1" } as Organization,
+        { id: "org2", name: "Default User Collection Org 2" } as Organization,
       ];
       const seededCollections: CollectionView[] = [
         {
@@ -752,6 +791,7 @@ function createSeededVaultPopupListFiltersService(
       accountServiceMock,
       viewCacheServiceMock,
       restrictedItemTypesServiceMock,
+      configService,
     );
   });
 

apps/browser/src/vault/popup/services/vault-popup-list-filters.service.ts

@@ -6,6 +6,7 @@ import {
   debounceTime,
   distinctUntilChanged,
   filter,
+  from,
   map,
   Observable,
   shareReplay,
@@ -17,6 +18,7 @@ import {
 import { CollectionService, CollectionView } from "@bitwarden/admin-console/common";
 import { ViewCacheService } from "@bitwarden/angular/platform/view-cache";
 import { DynamicTreeNode } from "@bitwarden/angular/vault/vault-filter/models/dynamic-tree-node.model";
+import { sortDefaultCollections } from "@bitwarden/angular/vault/vault-filter/services/vault-filter.service";
 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";
@@ -24,6 +26,8 @@ import { Organization } from "@bitwarden/common/admin-console/models/domain/orga
 import { AccountService } from "@bitwarden/common/auth/abstractions/account.service";
 import { getUserId } from "@bitwarden/common/auth/services/account.service";
 import { ProductTierType } from "@bitwarden/common/billing/enums";
+import { FeatureFlag } from "@bitwarden/common/enums/feature-flag.enum";
+import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service";
 import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service";
 import { Utils } from "@bitwarden/common/platform/misc/utils";
 import {
@@ -181,6 +185,7 @@ export class VaultPopupListFiltersService {
     private accountService: AccountService,
     private viewCacheService: ViewCacheService,
     private restrictedItemTypesService: RestrictedItemTypesService,
+    private configService: ConfigService,
   ) {
     this.filterForm.controls.organization.valueChanges
       .pipe(takeUntilDestroyed())
@@ -424,39 +429,47 @@ export class VaultPopupListFiltersService {
   /**
    * Collection array structured to be directly passed to `ChipSelectComponent`
    */
-  collections$: Observable<ChipSelectOption<CollectionView>[]> = combineLatest([
-    this.filters$.pipe(
-      distinctUntilChanged(
-        (previousFilter, currentFilter) =>
-          // Only update the collections when the organizationId filter changes
-          previousFilter.organization?.id === currentFilter.organization?.id,
+  collections$: Observable<ChipSelectOption<CollectionView>[]> =
+    this.accountService.activeAccount$.pipe(
+      getUserId,
+      switchMap((userId) =>
+        combineLatest([
+          this.filters$.pipe(
+            distinctUntilChanged((prev, curr) => prev.organization?.id === curr.organization?.id),
+          ),
+          this.collectionService.decryptedCollections$,
+          this.organizationService.memberOrganizations$(userId),
+          this.configService.getFeatureFlag$(FeatureFlag.CreateDefaultLocation),
+        ]),
       ),
-    ),
-    this.collectionService.decryptedCollections$,
-  ]).pipe(
-    map(([filters, allCollections]) => {
-      const organizationId = filters.organization?.id ?? null;
-      // When the organization filter is selected, filter out collections that do not belong to the selected organization
-      const collections =
-        organizationId === null
-          ? allCollections
-          : allCollections.filter((c) => c.organizationId === organizationId);
+      map(([filters, allCollections, orgs, defaultVaultEnabled]) => {
+        const orgFilterId = filters.organization?.id ?? null;
+        // When the organization filter is selected, filter out collections that do not belong to the selected organization
+        const filtered = orgFilterId
+          ? allCollections.filter((c) => c.organizationId === orgFilterId)
+          : allCollections;
 
-      return collections;
-    }),
-    switchMap(async (collections) => {
-      const nestedCollections = await this.collectionService.getAllNested(collections);
-
-      return new DynamicTreeNode<CollectionView>({
-        fullList: collections,
-        nestedList: nestedCollections,
-      });
-    }),
-    map((collections) =>
-      collections.nestedList.map((c) => this.convertToChipSelectOption(c, "bwi-collection-shared")),
-    ),
-    shareReplay({ refCount: true, bufferSize: 1 }),
-  );
+        if (!defaultVaultEnabled) {
+          return filtered;
+        }
+        return sortDefaultCollections(filtered, orgs, this.i18nService.collator);
+      }),
+      switchMap((collections) => {
+        return from(this.collectionService.getAllNested(collections)).pipe(
+          map(
+            (nested) =>
+              new DynamicTreeNode<CollectionView>({
+                fullList: collections,
+                nestedList: nested,
+              }),
+          ),
+        );
+      }),
+      map((tree) =>
+        tree.nestedList.map((c) => this.convertToChipSelectOption(c, "bwi-collection-shared")),
+      ),
+      shareReplay({ bufferSize: 1, refCount: true }),
+    );
 
   /** Organizations, collection, folders filters. */
   allFilters$ = combineLatest([this.organizations$, this.collections$, this.folders$]);

apps/browser/src/vault/services/vault-filter.service.ts

@@ -6,6 +6,8 @@ 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 { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service";
+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";
@@ -25,6 +27,8 @@ export class VaultFilterService extends BaseVaultFilterService {
     policyService: PolicyService,
     stateProvider: StateProvider,
     accountService: AccountService,
+    configService: ConfigService,
+    i18nService: I18nService,
   ) {
     super(
       organizationService,
@@ -34,6 +38,8 @@ export class VaultFilterService extends BaseVaultFilterService {
       policyService,
       stateProvider,
       accountService,
+      configService,
+      i18nService,
     );
     this.vaultFilter.myVaultOnly = false;
     this.vaultFilter.selectedOrganizationId = null;

apps/web/src/app/admin-console/organizations/collections/vault-filter/vault-filter.service.ts

@@ -5,6 +5,7 @@ import { CollectionAdminView, CollectionService } from "@bitwarden/admin-console
 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 { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service";
 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";
@@ -34,6 +35,7 @@ export class VaultFilterService extends BaseVaultFilterService implements OnDest
     stateProvider: StateProvider,
     collectionService: CollectionService,
     accountService: AccountService,
+    configService: ConfigService,
   ) {
     super(
       organizationService,
@@ -44,6 +46,7 @@ export class VaultFilterService extends BaseVaultFilterService implements OnDest
       stateProvider,
       collectionService,
       accountService,
+      configService,
     );
   }
 

apps/web/src/app/vault/individual-vault/vault-filter/services/vault-filter.service.spec.ts

@@ -5,13 +5,20 @@ import {
 import { FakeSingleUserState } 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 } from "rxjs";
+import { firstValueFrom, of, ReplaySubject } from "rxjs";
 
-import { CollectionService, CollectionView } from "@bitwarden/admin-console/common";
+import {
+  CollectionService,
+  CollectionType,
+  CollectionTypes,
+  CollectionView,
+} from "@bitwarden/admin-console/common";
+import * as vaultFilterSvc from "@bitwarden/angular/vault/vault-filter/services/vault-filter.service";
 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 { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service";
 import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service";
 import { Utils } from "@bitwarden/common/platform/misc/utils";
 import { UserId } from "@bitwarden/common/types/guid";
@@ -23,6 +30,10 @@ import { COLLAPSED_GROUPINGS } from "@bitwarden/common/vault/services/key-state/
 
 import { VaultFilterService } from "./vault-filter.service";
 
+jest.mock("@bitwarden/angular/vault/vault-filter/services/vault-filter.service", () => ({
+  sortDefaultCollections: jest.fn(() => []),
+}));
+
 describe("vault filter service", () => {
   let vaultFilterService: VaultFilterService;
 
@@ -39,6 +50,7 @@ describe("vault filter service", () => {
   let organizationDataOwnershipPolicy: ReplaySubject<boolean>;
   let singleOrgPolicy: ReplaySubject<boolean>;
   let stateProvider: FakeStateProvider;
+  let configService: MockProxy<ConfigService>;
 
   const mockUserId = Utils.newGuid() as UserId;
   let accountService: FakeAccountService;
@@ -54,6 +66,7 @@ describe("vault filter service", () => {
     stateProvider = new FakeStateProvider(accountService);
     i18nService.collator = new Intl.Collator("en-US");
     collectionService = mock<CollectionService>();
+    configService = mock<ConfigService>();
 
     organizations = new ReplaySubject<Organization[]>(1);
     folderViews = new ReplaySubject<FolderView[]>(1);
@@ -62,6 +75,7 @@ describe("vault filter service", () => {
     organizationDataOwnershipPolicy = new ReplaySubject<boolean>(1);
     singleOrgPolicy = new ReplaySubject<boolean>(1);
 
+    configService.getFeatureFlag$.mockReturnValue(of(true));
     organizationService.memberOrganizations$.mockReturnValue(organizations);
     folderService.folderViews$.mockReturnValue(folderViews);
     collectionService.decryptedCollections$ = collectionViews;
@@ -82,8 +96,10 @@ describe("vault filter service", () => {
       stateProvider,
       collectionService,
       accountService,
+      configService,
     );
     collapsedGroupingsState = stateProvider.singleUser.getFake(mockUserId, COLLAPSED_GROUPINGS);
+    organizations.next([]);
   });
 
   describe("collapsed filter nodes", () => {
@@ -285,6 +301,40 @@ describe("vault filter service", () => {
         const c3 = c1.children[0];
         expect(c3.parent.node.id).toEqual("id-1");
       });
+
+      it.only("calls sortDefaultCollections with the correct args", async () => {
+        const storedOrgs = [
+          createOrganization("id-defaultOrg1", "org1"),
+          createOrganization("id-defaultOrg2", "org2"),
+        ];
+        organizations.next(storedOrgs);
+
+        const storedCollections = [
+          createCollectionView("id-2", "Collection 2", "org test id"),
+          createCollectionView("id-1", "Collection 1", "org test id"),
+          createCollectionView(
+            "id-3",
+            "Default User Collection - Org 2",
+            "id-defaultOrg2",
+            CollectionTypes.DefaultUserCollection,
+          ),
+          createCollectionView(
+            "id-4",
+            "Default User Collection - Org 1",
+            "id-defaultOrg1",
+            CollectionTypes.DefaultUserCollection,
+          ),
+        ];
+        collectionViews.next(storedCollections);
+
+        await firstValueFrom(vaultFilterService.collectionTree$);
+
+        expect(vaultFilterSvc.sortDefaultCollections).toHaveBeenCalledWith(
+          storedCollections,
+          storedOrgs,
+          i18nService.collator,
+        );
+      });
     });
   });
 
@@ -312,11 +362,17 @@ describe("vault filter service", () => {
     return folder;
   }
 
-  function createCollectionView(id: string, name: string, orgId: string): CollectionView {
+  function createCollectionView(
+    id: string,
+    name: string,
+    orgId: string,
+    type?: CollectionType,
+  ): CollectionView {
     const collection = new CollectionView();
     collection.id = id;
     collection.name = name;
     collection.organizationId = orgId;
+    collection.type = type || CollectionTypes.SharedCollection;
     return collection;
   }
 });

apps/web/src/app/vault/individual-vault/vault-filter/services/vault-filter.service.ts

@@ -18,12 +18,15 @@ import {
   CollectionService,
   CollectionView,
 } from "@bitwarden/admin-console/common";
+import { sortDefaultCollections } from "@bitwarden/angular/vault/vault-filter/services/vault-filter.service";
 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 { 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 { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service";
 import { SingleUserState, StateProvider } from "@bitwarden/common/platform/state";
 import { UserId } from "@bitwarden/common/types/guid";
@@ -104,8 +107,14 @@ export class VaultFilterService implements VaultFilterServiceAbstraction {
       }),
     );
 
-  collectionTree$: Observable<TreeNode<CollectionFilter>> = this.filteredCollections$.pipe(
-    map((collections) => this.buildCollectionTree(collections)),
+  collectionTree$: Observable<TreeNode<CollectionFilter>> = combineLatest([
+    this.filteredCollections$,
+    this.memberOrganizations$,
+    this.configService.getFeatureFlag$(FeatureFlag.CreateDefaultLocation),
+  ]).pipe(
+    map(([collections, organizations, defaultCollectionsFlagEnabled]) =>
+      this.buildCollectionTree(collections, organizations, defaultCollectionsFlagEnabled),
+    ),
   );
 
   cipherTypeTree$: Observable<TreeNode<CipherTypeFilter>> = this.buildCipherTypeTree();
@@ -123,6 +132,7 @@ export class VaultFilterService implements VaultFilterServiceAbstraction {
     protected stateProvider: StateProvider,
     protected collectionService: CollectionService,
     protected accountService: AccountService,
+    protected configService: ConfigService,
   ) {}
 
   async getCollectionNodeFromTree(id: string) {
@@ -227,31 +237,39 @@ export class VaultFilterService implements VaultFilterServiceAbstraction {
       : storedCollections;
   }
 
-  protected buildCollectionTree(collections?: CollectionView[]): TreeNode<CollectionFilter> {
+  protected buildCollectionTree(
+    collections?: CollectionView[],
+    orgs?: Organization[],
+    defaultCollectionsFlagEnabled?: boolean,
+  ): TreeNode<CollectionFilter> {
     const headNode = this.getCollectionFilterHead();
     if (!collections) {
       return headNode;
     }
     const nodes: TreeNode<CollectionFilter>[] = [];
-    collections
-      .sort((a, b) => this.i18nService.collator.compare(a.name, b.name))
-      .forEach((c) => {
-        const collectionCopy = new CollectionView() as CollectionFilter;
-        collectionCopy.id = c.id;
-        collectionCopy.organizationId = c.organizationId;
-        collectionCopy.icon = "bwi-collection-shared";
-        if (c instanceof CollectionAdminView) {
-          collectionCopy.groups = c.groups;
-          collectionCopy.assigned = c.assigned;
-        }
-        const parts =
-          c.name != null ? c.name.replace(/^\/+|\/+$/g, "").split(NestingDelimiter) : [];
-        ServiceUtils.nestedTraverse(nodes, 0, parts, collectionCopy, null, NestingDelimiter);
-      });
+
+    if (defaultCollectionsFlagEnabled) {
+      collections = sortDefaultCollections(collections, orgs, this.i18nService.collator);
+    }
+
+    collections.forEach((c) => {
+      const collectionCopy = new CollectionView() as CollectionFilter;
+      collectionCopy.id = c.id;
+      collectionCopy.organizationId = c.organizationId;
+      collectionCopy.icon = "bwi-collection-shared";
+      if (c instanceof CollectionAdminView) {
+        collectionCopy.groups = c.groups;
+        collectionCopy.assigned = c.assigned;
+      }
+      const parts = c.name != null ? c.name.replace(/^\/+|\/+$/g, "").split(NestingDelimiter) : [];
+      ServiceUtils.nestedTraverse(nodes, 0, parts, collectionCopy, null, NestingDelimiter);
+    });
+
     nodes.forEach((n) => {
       n.parent = headNode;
       headNode.children.push(n);
     });
+
     return headNode;
   }
 

apps/web/src/locales/en/messages.json

@@ -864,6 +864,9 @@
   "me": {
     "message": "Me"
   },
+  "myItems": {
+    "message": "My items"
+  },
   "myVault": {
     "message": "My vault"
   },