From a92afe1efb9ca5dadb027cf1849a37b6a7716c67 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rui=20Tom=C3=A9?= <108268980+r-tome@users.noreply.github.com> Date: Wed, 30 Apr 2025 11:40:55 +0100 Subject: [PATCH] [PM-17690] Improve collection search to consider nested collections (#14420) * Add getFlatCollectionTree function and corresponding tests - Implemented getFlatCollectionTree to flatten a tree structure of collections. - Added unit tests for getFlatCollectionTree to verify functionality. * Refactor VaultComponent to utilize getFlatCollectionTree to search within all sub-levels - Updated vault.component.ts to import and use getFlatCollectionTree for flattening collection nodes during search. - Ensured consistent handling of collections across both vault and admin-console components. --- .../utils/collection-utils.spec.ts | 62 ++++++++++++++++++- .../collections/utils/collection-utils.ts | 21 +++++++ .../collections/vault.component.ts | 24 ++++--- .../vault/individual-vault/vault.component.ts | 27 +++++--- 4 files changed, 116 insertions(+), 18 deletions(-) diff --git a/apps/web/src/app/admin-console/organizations/collections/utils/collection-utils.spec.ts b/apps/web/src/app/admin-console/organizations/collections/utils/collection-utils.spec.ts index 0354a08c285..abd99d37355 100644 --- a/apps/web/src/app/admin-console/organizations/collections/utils/collection-utils.spec.ts +++ b/apps/web/src/app/admin-console/organizations/collections/utils/collection-utils.spec.ts @@ -1,6 +1,7 @@ import { CollectionView } from "@bitwarden/admin-console/common"; +import { TreeNode } from "@bitwarden/common/vault/models/domain/tree-node"; -import { getNestedCollectionTree } from "./collection-utils"; +import { getNestedCollectionTree, getFlatCollectionTree } from "./collection-utils"; describe("CollectionUtils Service", () => { describe("getNestedCollectionTree", () => { @@ -36,4 +37,63 @@ describe("CollectionUtils Service", () => { expect(result).toEqual([]); }); }); + + describe("getFlatCollectionTree", () => { + it("should flatten a tree node with no children", () => { + // Arrange + const collection = new CollectionView(); + collection.name = "Test Collection"; + collection.id = "test-id"; + + const treeNodes: TreeNode[] = [ + new TreeNode(collection, null), + ]; + + // Act + const result = getFlatCollectionTree(treeNodes); + + // Assert + expect(result.length).toBe(1); + expect(result[0]).toBe(collection); + }); + + it("should flatten a tree node with children", () => { + // Arrange + const parentCollection = new CollectionView(); + parentCollection.name = "Parent"; + parentCollection.id = "parent-id"; + + const child1Collection = new CollectionView(); + child1Collection.name = "Child 1"; + child1Collection.id = "child1-id"; + + const child2Collection = new CollectionView(); + child2Collection.name = "Child 2"; + child2Collection.id = "child2-id"; + + const grandchildCollection = new CollectionView(); + grandchildCollection.name = "Grandchild"; + grandchildCollection.id = "grandchild-id"; + + const parentNode = new TreeNode(parentCollection, null); + const child1Node = new TreeNode(child1Collection, parentNode); + const child2Node = new TreeNode(child2Collection, parentNode); + const grandchildNode = new TreeNode(grandchildCollection, child1Node); + + parentNode.children = [child1Node, child2Node]; + child1Node.children = [grandchildNode]; + + const treeNodes: TreeNode[] = [parentNode]; + + // Act + const result = getFlatCollectionTree(treeNodes); + + // Assert + expect(result.length).toBe(4); + expect(result[0]).toBe(parentCollection); + expect(result).toContain(child1Collection); + expect(result).toContain(child2Collection); + expect(result).toContain(grandchildCollection); + }); + }); }); diff --git a/apps/web/src/app/admin-console/organizations/collections/utils/collection-utils.ts b/apps/web/src/app/admin-console/organizations/collections/utils/collection-utils.ts index 2926ff3acee..95ae911bbf6 100644 --- a/apps/web/src/app/admin-console/organizations/collections/utils/collection-utils.ts +++ b/apps/web/src/app/admin-console/organizations/collections/utils/collection-utils.ts @@ -37,6 +37,27 @@ export function getNestedCollectionTree( return nodes; } +export function getFlatCollectionTree( + nodes: TreeNode[], +): CollectionAdminView[]; +export function getFlatCollectionTree(nodes: TreeNode[]): CollectionView[]; +export function getFlatCollectionTree( + nodes: TreeNode[], +): (CollectionView | CollectionAdminView)[] { + if (!nodes || nodes.length === 0) { + return []; + } + + return nodes.flatMap((node) => { + if (!node.children || node.children.length === 0) { + return [node.node]; + } + + const children = getFlatCollectionTree(node.children); + return [node.node, ...children]; + }); +} + function cloneCollection(collection: CollectionView): CollectionView; function cloneCollection(collection: CollectionAdminView): CollectionAdminView; function cloneCollection( diff --git a/apps/web/src/app/admin-console/organizations/collections/vault.component.ts b/apps/web/src/app/admin-console/organizations/collections/vault.component.ts index ccb97e2a703..7d159da917c 100644 --- a/apps/web/src/app/admin-console/organizations/collections/vault.component.ts +++ b/apps/web/src/app/admin-console/organizations/collections/vault.component.ts @@ -121,7 +121,7 @@ import { BulkCollectionsDialogResult, } from "./bulk-collections-dialog"; import { CollectionAccessRestrictedComponent } from "./collection-access-restricted.component"; -import { getNestedCollectionTree } from "./utils"; +import { getNestedCollectionTree, getFlatCollectionTree } from "./utils"; import { VaultFilterModule } from "./vault-filter/vault-filter.module"; import { VaultHeaderComponent } from "./vault-header/vault-header.component"; @@ -432,23 +432,33 @@ export class VaultComponent implements OnInit, OnDestroy { } this.showAddAccessToggle = false; - let collectionsToReturn = []; + let searchableCollectionNodes: TreeNode[] = []; if (filter.collectionId === undefined || filter.collectionId === All) { - collectionsToReturn = collections.map((c) => c.node); + searchableCollectionNodes = collections; } else { const selectedCollection = ServiceUtils.getTreeNodeObjectFromList( collections, filter.collectionId, ); - collectionsToReturn = selectedCollection?.children.map((c) => c.node) ?? []; + searchableCollectionNodes = selectedCollection?.children ?? []; } + let collectionsToReturn: CollectionAdminView[] = []; + if (await this.searchService.isSearchable(this.userId, searchText)) { + // Flatten the tree for searching through all levels + const flatCollectionTree: CollectionAdminView[] = + getFlatCollectionTree(searchableCollectionNodes); + collectionsToReturn = this.searchPipe.transform( - collectionsToReturn, + flatCollectionTree, searchText, - (collection: CollectionAdminView) => collection.name, - (collection: CollectionAdminView) => collection.id, + (collection) => collection.name, + (collection) => collection.id, + ); + } else { + collectionsToReturn = searchableCollectionNodes.map( + (treeNode: TreeNode): CollectionAdminView => treeNode.node, ); } diff --git a/apps/web/src/app/vault/individual-vault/vault.component.ts b/apps/web/src/app/vault/individual-vault/vault.component.ts index 67bd6a6a526..55bbd0c0651 100644 --- a/apps/web/src/app/vault/individual-vault/vault.component.ts +++ b/apps/web/src/app/vault/individual-vault/vault.component.ts @@ -79,7 +79,10 @@ import { PasswordRepromptService, } from "@bitwarden/vault"; -import { getNestedCollectionTree } from "../../admin-console/organizations/collections"; +import { + getNestedCollectionTree, + getFlatCollectionTree, +} from "../../admin-console/organizations/collections"; import { CollectionDialogAction, CollectionDialogTabType, @@ -372,31 +375,35 @@ export class VaultComponent implements OnInit, OnDestroy { if (filter.collectionId === undefined || filter.collectionId === Unassigned) { return []; } - let collectionsToReturn = []; + let searchableCollectionNodes: TreeNode[] = []; if (filter.organizationId !== undefined && filter.collectionId === All) { - collectionsToReturn = collections - .filter((c) => c.node.organizationId === filter.organizationId) - .map((c) => c.node); + searchableCollectionNodes = collections.filter( + (c) => c.node.organizationId === filter.organizationId, + ); } else if (filter.collectionId === All) { - collectionsToReturn = collections.map((c) => c.node); + searchableCollectionNodes = collections; } else { const selectedCollection = ServiceUtils.getTreeNodeObjectFromList( collections, filter.collectionId, ); - collectionsToReturn = selectedCollection?.children.map((c) => c.node) ?? []; + searchableCollectionNodes = selectedCollection?.children ?? []; } if (await this.searchService.isSearchable(activeUserId, searchText)) { - collectionsToReturn = this.searchPipe.transform( - collectionsToReturn, + // Flatten the tree for searching through all levels + const flatCollectionTree: CollectionView[] = + getFlatCollectionTree(searchableCollectionNodes); + + return this.searchPipe.transform( + flatCollectionTree, searchText, (collection) => collection.name, (collection) => collection.id, ); } - return collectionsToReturn; + return searchableCollectionNodes.map((treeNode: TreeNode) => treeNode.node); }), shareReplay({ refCount: true, bufferSize: 1 }), );