From 61b375973622da17f619e42738b96e263aac3722 Mon Sep 17 00:00:00 2001 From: aj-rosado <109146700+aj-rosado@users.noreply.github.com> Date: Tue, 19 Mar 2024 14:19:41 +0000 Subject: [PATCH] [PM-6334] Passing CollectionView or FolderView from Import component to ImportService (#8291) * Passing CollectionView or FolderView from Import component to ImportService * Corrected import service tests * Added tests to validate if the incorrect object type error is thrown on setImportTarget --- .../src/components/import.component.html | 4 +- .../services/import.service.abstraction.ts | 5 +- .../src/services/import.service.spec.ts | 66 ++++++++++++++----- libs/importer/src/services/import.service.ts | 34 +++++----- 4 files changed, 71 insertions(+), 38 deletions(-) diff --git a/libs/importer/src/components/import.component.html b/libs/importer/src/components/import.component.html index 6c24b80f92..836a1d9a1a 100644 --- a/libs/importer/src/components/import.component.html +++ b/libs/importer/src/components/import.component.html @@ -37,7 +37,7 @@ @@ -46,7 +46,7 @@ diff --git a/libs/importer/src/services/import.service.abstraction.ts b/libs/importer/src/services/import.service.abstraction.ts index 95208c9b99..dc77e76390 100644 --- a/libs/importer/src/services/import.service.abstraction.ts +++ b/libs/importer/src/services/import.service.abstraction.ts @@ -1,3 +1,6 @@ +import { CollectionView } from "@bitwarden/common/vault/models/view/collection.view"; +import { FolderView } from "@bitwarden/common/vault/models/view/folder.view"; + import { Importer } from "../importers/importer"; import { ImportOption, ImportType } from "../models/import-options"; import { ImportResult } from "../models/import-result"; @@ -10,7 +13,7 @@ export abstract class ImportServiceAbstraction { importer: Importer, fileContents: string, organizationId?: string, - selectedImportTarget?: string, + selectedImportTarget?: FolderView | CollectionView, canAccessImportExport?: boolean, ) => Promise; getImporter: ( diff --git a/libs/importer/src/services/import.service.spec.ts b/libs/importer/src/services/import.service.spec.ts index 6a2e0a339c..a95b74d792 100644 --- a/libs/importer/src/services/import.service.spec.ts +++ b/libs/importer/src/services/import.service.spec.ts @@ -86,7 +86,7 @@ describe("ImportService", () => { }); it("empty importTarget does nothing", async () => { - await importService["setImportTarget"](importResult, null, ""); + await importService["setImportTarget"](importResult, null, null); expect(importResult.folders.length).toBe(0); }); @@ -99,9 +99,9 @@ describe("ImportService", () => { Promise.resolve([mockImportTargetFolder]), ); - await importService["setImportTarget"](importResult, null, "myImportTarget"); + await importService["setImportTarget"](importResult, null, mockImportTargetFolder); expect(importResult.folders.length).toBe(1); - expect(importResult.folders[0].name).toBe("myImportTarget"); + expect(importResult.folders[0]).toBe(mockImportTargetFolder); }); const mockFolder1 = new FolderView(); @@ -119,16 +119,18 @@ describe("ImportService", () => { mockFolder2, ]); - const myImportTarget = "myImportTarget"; - importResult.folders.push(mockFolder1); importResult.folders.push(mockFolder2); - await importService["setImportTarget"](importResult, null, myImportTarget); + await importService["setImportTarget"](importResult, null, mockImportTargetFolder); expect(importResult.folders.length).toBe(3); - expect(importResult.folders[0].name).toBe(myImportTarget); - expect(importResult.folders[1].name).toBe(`${myImportTarget}/${mockFolder1.name}`); - expect(importResult.folders[2].name).toBe(`${myImportTarget}/${mockFolder2.name}`); + expect(importResult.folders[0]).toBe(mockImportTargetFolder); + expect(importResult.folders[1].name).toBe( + `${mockImportTargetFolder.name}/${mockFolder1.name}`, + ); + expect(importResult.folders[2].name).toBe( + `${mockImportTargetFolder.name}/${mockFolder2.name}`, + ); }); const mockImportTargetCollection = new CollectionView(); @@ -152,9 +154,13 @@ describe("ImportService", () => { mockCollection1, ]); - await importService["setImportTarget"](importResult, organizationId, "myImportTarget"); + await importService["setImportTarget"]( + importResult, + organizationId, + mockImportTargetCollection, + ); expect(importResult.collections.length).toBe(1); - expect(importResult.collections[0].name).toBe("myImportTarget"); + expect(importResult.collections[0]).toBe(mockImportTargetCollection); }); it("passing importTarget sets it as new root for all existing collections", async () => { @@ -164,16 +170,42 @@ describe("ImportService", () => { mockCollection2, ]); - const myImportTarget = "myImportTarget"; - importResult.collections.push(mockCollection1); importResult.collections.push(mockCollection2); - await importService["setImportTarget"](importResult, organizationId, myImportTarget); + await importService["setImportTarget"]( + importResult, + organizationId, + mockImportTargetCollection, + ); expect(importResult.collections.length).toBe(3); - expect(importResult.collections[0].name).toBe(myImportTarget); - expect(importResult.collections[1].name).toBe(`${myImportTarget}/${mockCollection1.name}`); - expect(importResult.collections[2].name).toBe(`${myImportTarget}/${mockCollection2.name}`); + expect(importResult.collections[0]).toBe(mockImportTargetCollection); + expect(importResult.collections[1].name).toBe( + `${mockImportTargetCollection.name}/${mockCollection1.name}`, + ); + expect(importResult.collections[2].name).toBe( + `${mockImportTargetCollection.name}/${mockCollection2.name}`, + ); + }); + + it("passing importTarget as null on setImportTarget with organizationId throws error", async () => { + const setImportTargetMethod = importService["setImportTarget"]( + null, + organizationId, + new Object() as FolderView, + ); + + await expect(setImportTargetMethod).rejects.toThrow("Error assigning target collection"); + }); + + it("passing importTarget as null on setImportTarget throws error", async () => { + const setImportTargetMethod = importService["setImportTarget"]( + null, + "", + new Object() as CollectionView, + ); + + await expect(setImportTargetMethod).rejects.toThrow("Error assigning target folder"); }); }); }); diff --git a/libs/importer/src/services/import.service.ts b/libs/importer/src/services/import.service.ts index e26b768ab6..a6fd233dcf 100644 --- a/libs/importer/src/services/import.service.ts +++ b/libs/importer/src/services/import.service.ts @@ -110,7 +110,7 @@ export class ImportService implements ImportServiceAbstraction { importer: Importer, fileContents: string, organizationId: string = null, - selectedImportTarget: string = null, + selectedImportTarget: FolderView | CollectionView = null, canAccessImportExport: boolean, ): Promise { let importResult: ImportResult; @@ -147,11 +147,7 @@ export class ImportService implements ImportServiceAbstraction { } } - if ( - organizationId && - Utils.isNullOrWhitespace(selectedImportTarget) && - !canAccessImportExport - ) { + if (organizationId && !selectedImportTarget && !canAccessImportExport) { const hasUnassignedCollections = importResult.collectionRelationships.length < importResult.ciphers.length; if (hasUnassignedCollections) { @@ -428,29 +424,30 @@ export class ImportService implements ImportServiceAbstraction { private async setImportTarget( importResult: ImportResult, organizationId: string, - importTarget: string, + importTarget: FolderView | CollectionView, ) { - if (Utils.isNullOrWhitespace(importTarget)) { + if (!importTarget) { return; } if (organizationId) { - const collectionViews: CollectionView[] = await this.collectionService.getAllDecrypted(); - const targetCollection = collectionViews.find((c) => c.id === importTarget); + if (!(importTarget instanceof CollectionView)) { + throw new Error("Error assigning target collection"); + } const noCollectionRelationShips: [number, number][] = []; importResult.ciphers.forEach((c, index) => { if (!Array.isArray(c.collectionIds) || c.collectionIds.length == 0) { - c.collectionIds = [targetCollection.id]; + c.collectionIds = [importTarget.id]; noCollectionRelationShips.push([index, 0]); } }); const collections: CollectionView[] = [...importResult.collections]; - importResult.collections = [targetCollection]; + importResult.collections = [importTarget as CollectionView]; collections.map((x) => { const f = new CollectionView(); - f.name = `${targetCollection.name}/${x.name}`; + f.name = `${importTarget.name}/${x.name}`; importResult.collections.push(f); }); @@ -463,21 +460,22 @@ export class ImportService implements ImportServiceAbstraction { return; } - const folderViews = await this.folderService.getAllDecryptedFromState(); - const targetFolder = folderViews.find((f) => f.id === importTarget); + if (!(importTarget instanceof FolderView)) { + throw new Error("Error assigning target folder"); + } const noFolderRelationShips: [number, number][] = []; importResult.ciphers.forEach((c, index) => { if (Utils.isNullOrEmpty(c.folderId)) { - c.folderId = targetFolder.id; + c.folderId = importTarget.id; noFolderRelationShips.push([index, 0]); } }); const folders: FolderView[] = [...importResult.folders]; - importResult.folders = [targetFolder]; + importResult.folders = [importTarget as FolderView]; folders.map((x) => { - const newFolderName = `${targetFolder.name}/${x.name}`; + const newFolderName = `${importTarget.name}/${x.name}`; const f = new FolderView(); f.name = newFolderName; importResult.folders.push(f);