[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
This commit is contained in:
aj-rosado 2024-03-19 14:19:41 +00:00 committed by GitHub
parent ee22d07474
commit 61b3759736
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 71 additions and 38 deletions

View File

@ -37,7 +37,7 @@
<bit-option [value]="null" label="-- {{ 'selectImportFolder' | i18n }} --" /> <bit-option [value]="null" label="-- {{ 'selectImportFolder' | i18n }} --" />
<bit-option <bit-option
*ngFor="let f of folders$ | async" *ngFor="let f of folders$ | async"
[value]="f.id" [value]="f"
[label]="f.name" [label]="f.name"
icon="bwi-folder" icon="bwi-folder"
/> />
@ -46,7 +46,7 @@
<bit-option [value]="null" label="-- {{ 'selectImportCollection' | i18n }} --" /> <bit-option [value]="null" label="-- {{ 'selectImportCollection' | i18n }} --" />
<bit-option <bit-option
*ngFor="let c of collections$ | async" *ngFor="let c of collections$ | async"
[value]="c.id" [value]="c"
[label]="c.name" [label]="c.name"
icon="bwi-collection" icon="bwi-collection"
/> />

View File

@ -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 { Importer } from "../importers/importer";
import { ImportOption, ImportType } from "../models/import-options"; import { ImportOption, ImportType } from "../models/import-options";
import { ImportResult } from "../models/import-result"; import { ImportResult } from "../models/import-result";
@ -10,7 +13,7 @@ export abstract class ImportServiceAbstraction {
importer: Importer, importer: Importer,
fileContents: string, fileContents: string,
organizationId?: string, organizationId?: string,
selectedImportTarget?: string, selectedImportTarget?: FolderView | CollectionView,
canAccessImportExport?: boolean, canAccessImportExport?: boolean,
) => Promise<ImportResult>; ) => Promise<ImportResult>;
getImporter: ( getImporter: (

View File

@ -86,7 +86,7 @@ describe("ImportService", () => {
}); });
it("empty importTarget does nothing", async () => { it("empty importTarget does nothing", async () => {
await importService["setImportTarget"](importResult, null, ""); await importService["setImportTarget"](importResult, null, null);
expect(importResult.folders.length).toBe(0); expect(importResult.folders.length).toBe(0);
}); });
@ -99,9 +99,9 @@ describe("ImportService", () => {
Promise.resolve([mockImportTargetFolder]), Promise.resolve([mockImportTargetFolder]),
); );
await importService["setImportTarget"](importResult, null, "myImportTarget"); await importService["setImportTarget"](importResult, null, mockImportTargetFolder);
expect(importResult.folders.length).toBe(1); expect(importResult.folders.length).toBe(1);
expect(importResult.folders[0].name).toBe("myImportTarget"); expect(importResult.folders[0]).toBe(mockImportTargetFolder);
}); });
const mockFolder1 = new FolderView(); const mockFolder1 = new FolderView();
@ -119,16 +119,18 @@ describe("ImportService", () => {
mockFolder2, mockFolder2,
]); ]);
const myImportTarget = "myImportTarget";
importResult.folders.push(mockFolder1); importResult.folders.push(mockFolder1);
importResult.folders.push(mockFolder2); 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.length).toBe(3);
expect(importResult.folders[0].name).toBe(myImportTarget); expect(importResult.folders[0]).toBe(mockImportTargetFolder);
expect(importResult.folders[1].name).toBe(`${myImportTarget}/${mockFolder1.name}`); expect(importResult.folders[1].name).toBe(
expect(importResult.folders[2].name).toBe(`${myImportTarget}/${mockFolder2.name}`); `${mockImportTargetFolder.name}/${mockFolder1.name}`,
);
expect(importResult.folders[2].name).toBe(
`${mockImportTargetFolder.name}/${mockFolder2.name}`,
);
}); });
const mockImportTargetCollection = new CollectionView(); const mockImportTargetCollection = new CollectionView();
@ -152,9 +154,13 @@ describe("ImportService", () => {
mockCollection1, mockCollection1,
]); ]);
await importService["setImportTarget"](importResult, organizationId, "myImportTarget"); await importService["setImportTarget"](
importResult,
organizationId,
mockImportTargetCollection,
);
expect(importResult.collections.length).toBe(1); 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 () => { it("passing importTarget sets it as new root for all existing collections", async () => {
@ -164,16 +170,42 @@ describe("ImportService", () => {
mockCollection2, mockCollection2,
]); ]);
const myImportTarget = "myImportTarget";
importResult.collections.push(mockCollection1); importResult.collections.push(mockCollection1);
importResult.collections.push(mockCollection2); 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.length).toBe(3);
expect(importResult.collections[0].name).toBe(myImportTarget); expect(importResult.collections[0]).toBe(mockImportTargetCollection);
expect(importResult.collections[1].name).toBe(`${myImportTarget}/${mockCollection1.name}`); expect(importResult.collections[1].name).toBe(
expect(importResult.collections[2].name).toBe(`${myImportTarget}/${mockCollection2.name}`); `${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");
}); });
}); });
}); });

View File

@ -110,7 +110,7 @@ export class ImportService implements ImportServiceAbstraction {
importer: Importer, importer: Importer,
fileContents: string, fileContents: string,
organizationId: string = null, organizationId: string = null,
selectedImportTarget: string = null, selectedImportTarget: FolderView | CollectionView = null,
canAccessImportExport: boolean, canAccessImportExport: boolean,
): Promise<ImportResult> { ): Promise<ImportResult> {
let importResult: ImportResult; let importResult: ImportResult;
@ -147,11 +147,7 @@ export class ImportService implements ImportServiceAbstraction {
} }
} }
if ( if (organizationId && !selectedImportTarget && !canAccessImportExport) {
organizationId &&
Utils.isNullOrWhitespace(selectedImportTarget) &&
!canAccessImportExport
) {
const hasUnassignedCollections = const hasUnassignedCollections =
importResult.collectionRelationships.length < importResult.ciphers.length; importResult.collectionRelationships.length < importResult.ciphers.length;
if (hasUnassignedCollections) { if (hasUnassignedCollections) {
@ -428,29 +424,30 @@ export class ImportService implements ImportServiceAbstraction {
private async setImportTarget( private async setImportTarget(
importResult: ImportResult, importResult: ImportResult,
organizationId: string, organizationId: string,
importTarget: string, importTarget: FolderView | CollectionView,
) { ) {
if (Utils.isNullOrWhitespace(importTarget)) { if (!importTarget) {
return; return;
} }
if (organizationId) { if (organizationId) {
const collectionViews: CollectionView[] = await this.collectionService.getAllDecrypted(); if (!(importTarget instanceof CollectionView)) {
const targetCollection = collectionViews.find((c) => c.id === importTarget); throw new Error("Error assigning target collection");
}
const noCollectionRelationShips: [number, number][] = []; const noCollectionRelationShips: [number, number][] = [];
importResult.ciphers.forEach((c, index) => { importResult.ciphers.forEach((c, index) => {
if (!Array.isArray(c.collectionIds) || c.collectionIds.length == 0) { if (!Array.isArray(c.collectionIds) || c.collectionIds.length == 0) {
c.collectionIds = [targetCollection.id]; c.collectionIds = [importTarget.id];
noCollectionRelationShips.push([index, 0]); noCollectionRelationShips.push([index, 0]);
} }
}); });
const collections: CollectionView[] = [...importResult.collections]; const collections: CollectionView[] = [...importResult.collections];
importResult.collections = [targetCollection]; importResult.collections = [importTarget as CollectionView];
collections.map((x) => { collections.map((x) => {
const f = new CollectionView(); const f = new CollectionView();
f.name = `${targetCollection.name}/${x.name}`; f.name = `${importTarget.name}/${x.name}`;
importResult.collections.push(f); importResult.collections.push(f);
}); });
@ -463,21 +460,22 @@ export class ImportService implements ImportServiceAbstraction {
return; return;
} }
const folderViews = await this.folderService.getAllDecryptedFromState(); if (!(importTarget instanceof FolderView)) {
const targetFolder = folderViews.find((f) => f.id === importTarget); throw new Error("Error assigning target folder");
}
const noFolderRelationShips: [number, number][] = []; const noFolderRelationShips: [number, number][] = [];
importResult.ciphers.forEach((c, index) => { importResult.ciphers.forEach((c, index) => {
if (Utils.isNullOrEmpty(c.folderId)) { if (Utils.isNullOrEmpty(c.folderId)) {
c.folderId = targetFolder.id; c.folderId = importTarget.id;
noFolderRelationShips.push([index, 0]); noFolderRelationShips.push([index, 0]);
} }
}); });
const folders: FolderView[] = [...importResult.folders]; const folders: FolderView[] = [...importResult.folders];
importResult.folders = [targetFolder]; importResult.folders = [importTarget as FolderView];
folders.map((x) => { folders.map((x) => {
const newFolderName = `${targetFolder.name}/${x.name}`; const newFolderName = `${importTarget.name}/${x.name}`;
const f = new FolderView(); const f = new FolderView();
f.name = newFolderName; f.name = newFolderName;
importResult.folders.push(f); importResult.folders.push(f);