From 2a72d2e74d08c4ea05ac22ac5414d9484e5ab0fb Mon Sep 17 00:00:00 2001 From: Jordan Aasen <166539328+jaasen-livefront@users.noreply.github.com> Date: Thu, 12 Feb 2026 13:52:29 -0800 Subject: [PATCH] [PM-25685][PM-31077] - Migrate all Folder models (#17077) * enforce strict types on folders * fix folder api service * fix tests * fix test * fix type issue * fix test * add extra checks for folders. add specs * fix folder.id checks * fix id logic * remove unecessary check * name name and id optional in folder model * fix tests * Update folder and folderview * fix folder with id export * fix tests * fix tests * more defensive typing * fix tests * no need to check for presence * check for empty name in folder toDomain * fixes to folder * initialize id in folder constructor. fix failing tests * remove optional param to folder constructor * fix folder * fix test * remove remaining checks for null folder id * fix logic * pass null for empty folder ids * make id more explicit * fix failing test * fix failing test * fix "No Folder" filter --- .../vault-popup-list-filters.service.spec.ts | 4 +- .../vault-popup-list-filters.service.ts | 12 ++---- .../models/vault-filter.model.spec.ts | 11 +++++ .../vault-filter/models/vault-filter.model.ts | 15 +++++-- .../services/vault-filter.service.ts | 2 +- .../common/src/models/export/folder.export.ts | 10 ++--- .../src/vault/models/data/folder.data.ts | 19 +++++---- .../src/vault/models/domain/folder.spec.ts | 42 +++++++++++++------ libs/common/src/vault/models/domain/folder.ts | 39 +++++++---------- .../models/request/folder-with-id.request.ts | 2 +- .../src/vault/models/view/folder.view.ts | 21 ++++++---- .../services/folder/folder-api.service.ts | 6 +-- .../services/folder/folder.service.spec.ts | 11 ++--- .../src/components/import.component.ts | 2 +- libs/importer/src/importers/base-importer.ts | 7 ++-- .../importers/keepass2-xml-importer.spec.ts | 2 +- .../password-depot-17-xml-importer.spec.ts | 9 +++- .../individual-vault-export.service.spec.ts | 20 ++++----- .../individual-vault-export.service.ts | 6 +-- .../add-edit-folder-dialog.component.spec.ts | 2 +- libs/vault/src/models/filter-function.spec.ts | 8 ++++ libs/vault/src/models/filter-function.ts | 6 ++- .../routed-vault-filter-bridge.model.ts | 2 +- libs/vault/src/models/vault-filter.model.ts | 2 +- .../routed-vault-filter-bridge.service.ts | 2 +- .../src/services/vault-filter.service.spec.ts | 4 +- .../src/services/vault-filter.service.ts | 4 +- 27 files changed, 157 insertions(+), 113 deletions(-) diff --git a/apps/browser/src/vault/popup/services/vault-popup-list-filters.service.spec.ts b/apps/browser/src/vault/popup/services/vault-popup-list-filters.service.spec.ts index 1358c5faebe..52703284679 100644 --- a/apps/browser/src/vault/popup/services/vault-popup-list-filters.service.spec.ts +++ b/apps/browser/src/vault/popup/services/vault-popup-list-filters.service.spec.ts @@ -438,7 +438,7 @@ describe("VaultPopupListFiltersService", () => { describe("folders$", () => { it('returns no folders when "No Folder" is the only option', (done) => { - folderViews$.next([{ id: null, name: "No Folder" }]); + folderViews$.next([{ id: "", name: "No Folder" }]); service.folders$.subscribe((folders) => { expect(folders).toEqual([]); @@ -448,7 +448,7 @@ describe("VaultPopupListFiltersService", () => { it('moves "No Folder" to the end of the list', (done) => { folderViews$.next([ - { id: null, name: "No Folder" }, + { id: "", name: "No Folder" }, { id: "2345", name: "Folder 2" }, { id: "1234", name: "Folder 1" }, ]); diff --git a/apps/browser/src/vault/popup/services/vault-popup-list-filters.service.ts b/apps/browser/src/vault/popup/services/vault-popup-list-filters.service.ts index 85c415d01fe..3b220e4719c 100644 --- a/apps/browser/src/vault/popup/services/vault-popup-list-filters.service.ts +++ b/apps/browser/src/vault/popup/services/vault-popup-list-filters.service.ts @@ -387,7 +387,7 @@ export class VaultPopupListFiltersService { FolderView[], PopupCipherViewLike[], ] => { - if (folders.length === 1 && folders[0].id === null) { + if (folders.length === 1 && !folders[0].id) { // Do not display folder selections when only the "no folder" option is available. return [filters as PopupListFilter, [], cipherViews]; } @@ -396,7 +396,7 @@ export class VaultPopupListFiltersService { folders.sort(Utils.getSortFunction(this.i18nService, "name")); let arrangedFolders = folders; - const noFolder = folders.find((f) => f.id === null); + const noFolder = folders.find((f) => !f.id); if (noFolder) { // Update `name` of the "no folder" option to "Items with no folder" @@ -406,7 +406,7 @@ export class VaultPopupListFiltersService { }; // Move the "no folder" option to the end of the list - arrangedFolders = [...folders.filter((f) => f.id !== null), updatedNoFolder]; + arrangedFolders = [...folders.filter((f) => f.id), updatedNoFolder]; } return [filters as PopupListFilter, arrangedFolders, cipherViews]; }, @@ -545,11 +545,7 @@ export class VaultPopupListFiltersService { // When the organization filter changes and a folder is already selected, // reset the folder filter if the folder does not belong to the new organization filter - if ( - currentFilters.folder && - currentFilters.folder.id !== null && - organization.id !== MY_VAULT_ID - ) { + if (currentFilters.folder && currentFilters.folder.id && organization.id !== MY_VAULT_ID) { // Get all ciphers that belong to the new organization const orgCiphers = this.cipherViews.filter((c) => c.organizationId === organization.id); diff --git a/libs/angular/src/vault/vault-filter/models/vault-filter.model.spec.ts b/libs/angular/src/vault/vault-filter/models/vault-filter.model.spec.ts index a2f8aa7a352..b45472ad01c 100644 --- a/libs/angular/src/vault/vault-filter/models/vault-filter.model.spec.ts +++ b/libs/angular/src/vault/vault-filter/models/vault-filter.model.spec.ts @@ -143,6 +143,17 @@ describe("VaultFilter", () => { expect(result).toBe(true); }); + + it("should return true when filtering on unassigned folder via empty string id", () => { + const filterFunction = createFilterFunction({ + selectedFolder: true, + selectedFolderId: "", + }); + + const result = filterFunction(cipher); + + expect(result).toBe(true); + }); }); describe("given an organizational cipher (with organization and collections)", () => { diff --git a/libs/angular/src/vault/vault-filter/models/vault-filter.model.ts b/libs/angular/src/vault/vault-filter/models/vault-filter.model.ts index d3ad29142e2..e99cb5e9eeb 100644 --- a/libs/angular/src/vault/vault-filter/models/vault-filter.model.ts +++ b/libs/angular/src/vault/vault-filter/models/vault-filter.model.ts @@ -63,10 +63,19 @@ export class VaultFilter { if (this.cipherType != null && cipherPassesFilter) { cipherPassesFilter = CipherViewLikeUtils.getType(cipher) === this.cipherType; } - if (this.selectedFolder && this.selectedFolderId == null && cipherPassesFilter) { - cipherPassesFilter = cipher.folderId == null; + if ( + this.selectedFolder && + (this.selectedFolderId == null || this.selectedFolderId === "") && + cipherPassesFilter + ) { + cipherPassesFilter = cipher.folderId == null || cipher.folderId === ""; } - if (this.selectedFolder && this.selectedFolderId != null && cipherPassesFilter) { + if ( + this.selectedFolder && + this.selectedFolderId != null && + this.selectedFolderId !== "" && + cipherPassesFilter + ) { cipherPassesFilter = cipher.folderId === this.selectedFolderId; } if (this.selectedCollection && this.selectedCollectionId == null && cipherPassesFilter) { diff --git a/libs/angular/src/vault/vault-filter/services/vault-filter.service.ts b/libs/angular/src/vault/vault-filter/services/vault-filter.service.ts index 9b34890cbce..4a12bed1c66 100644 --- a/libs/angular/src/vault/vault-filter/services/vault-filter.service.ts +++ b/libs/angular/src/vault/vault-filter/services/vault-filter.service.ts @@ -83,7 +83,7 @@ export class VaultFilterService implements DeprecatedVaultFilterServiceAbstracti const ciphers = await this.cipherService.getAllDecrypted(userId); const orgCiphers = ciphers.filter((c) => c.organizationId == organizationId); folders = storedFolders.filter( - (f) => orgCiphers.some((oc) => oc.folderId == f.id) || f.id == null, + (f) => orgCiphers.some((oc) => oc.folderId == f.id) || !f.id, ); } diff --git a/libs/common/src/models/export/folder.export.ts b/libs/common/src/models/export/folder.export.ts index 96f0f1058b8..1bffcee8c2d 100644 --- a/libs/common/src/models/export/folder.export.ts +++ b/libs/common/src/models/export/folder.export.ts @@ -1,5 +1,3 @@ -// FIXME: Update this file to be type safe and remove this and next line -// @ts-strict-ignore import { EncString } from "../../key-management/crypto/models/enc-string"; import { Folder as FolderDomain } from "../../vault/models/domain/folder"; import { FolderView } from "../../vault/models/view/folder.view"; @@ -7,6 +5,8 @@ import { FolderView } from "../../vault/models/view/folder.view"; import { safeGetString } from "./utils"; export class FolderExport { + name: string = ""; + static template(): FolderExport { const req = new FolderExport(); req.name = "Folder name"; @@ -19,14 +19,12 @@ export class FolderExport { } static toDomain(req: FolderExport, domain = new FolderDomain()) { - domain.name = req.name != null ? new EncString(req.name) : null; + domain.name = new EncString(req.name); return domain; } - name: string; - // Use build method instead of ctor so that we can control order of JSON stringify for pretty print build(o: FolderView | FolderDomain) { - this.name = safeGetString(o.name); + this.name = safeGetString(o.name ?? "") ?? ""; } } diff --git a/libs/common/src/vault/models/data/folder.data.ts b/libs/common/src/vault/models/data/folder.data.ts index c2eb585a6f4..5358cd713b3 100644 --- a/libs/common/src/vault/models/data/folder.data.ts +++ b/libs/common/src/vault/models/data/folder.data.ts @@ -1,5 +1,3 @@ -// FIXME: Update this file to be type safe and remove this and next line -// @ts-strict-ignore import { Jsonify } from "type-fest"; import { FolderResponse } from "../response/folder.response"; @@ -10,12 +8,19 @@ export class FolderData { revisionDate: string; constructor(response: Partial) { - this.name = response?.name; - this.id = response?.id; - this.revisionDate = response?.revisionDate; + this.name = response.name ?? ""; + this.id = response.id ?? ""; + this.revisionDate = response.revisionDate ?? new Date().toISOString(); } - static fromJSON(obj: Jsonify) { - return Object.assign(new FolderData({}), obj); + static fromJSON(obj: Jsonify) { + if (obj == null) { + return null; + } + return new FolderData({ + id: obj.id, + name: obj.name, + revisionDate: obj.revisionDate, + }); } } diff --git a/libs/common/src/vault/models/domain/folder.spec.ts b/libs/common/src/vault/models/domain/folder.spec.ts index d9e9e265d91..fd1455dbb66 100644 --- a/libs/common/src/vault/models/domain/folder.spec.ts +++ b/libs/common/src/vault/models/domain/folder.spec.ts @@ -8,7 +8,7 @@ import { mockFromJson, } from "../../../../spec"; import { EncryptService } from "../../../key-management/crypto/abstractions/encrypt.service"; -import { EncryptedString, EncString } from "../../../key-management/crypto/models/enc-string"; +import { EncString } from "../../../key-management/crypto/models/enc-string"; import { FolderData } from "../../models/data/folder.data"; import { Folder } from "../../models/domain/folder"; @@ -49,6 +49,30 @@ describe("Folder", () => { }); }); + describe("constructor", () => { + it("initializes properties from FolderData", () => { + const revisionDate = new Date("2022-08-04T01:06:40.441Z"); + const folder = new Folder({ + id: "id", + name: "name", + revisionDate: revisionDate.toISOString(), + }); + + expect(folder.id).toBe("id"); + expect(folder.revisionDate).toEqual(revisionDate); + expect(folder.name).toBeInstanceOf(EncString); + expect((folder.name as EncString).encryptedString).toBe("name"); + }); + + it("initializes empty properties when no FolderData is provided", () => { + const folder = new Folder(); + + expect(folder.id).toBe(""); + expect(folder.name).toBeInstanceOf(EncString); + expect(folder.revisionDate).toBeInstanceOf(Date); + }); + }); + describe("fromJSON", () => { jest.mock("../../../key-management/crypto/models/enc-string"); jest.spyOn(EncString, "fromJSON").mockImplementation(mockFromJson); @@ -57,17 +81,13 @@ describe("Folder", () => { const revisionDate = new Date("2022-08-04T01:06:40.441Z"); const actual = Folder.fromJSON({ revisionDate: revisionDate.toISOString(), - name: "name" as EncryptedString, + name: "name", id: "id", }); - const expected = { - revisionDate: revisionDate, - name: "name_fromJSON", - id: "id", - }; - - expect(actual).toMatchObject(expected); + expect(actual?.id).toBe("id"); + expect(actual?.revisionDate).toEqual(revisionDate); + expect(actual?.name).toBe("name_fromJSON"); }); }); @@ -89,9 +109,7 @@ describe("Folder", () => { const view = await folder.decryptWithKey(key, encryptService); - expect(view).toEqual({ - name: "encName", - }); + expect(view.name).toBe("encName"); }); it("assigns the folder id and revision date", async () => { diff --git a/libs/common/src/vault/models/domain/folder.ts b/libs/common/src/vault/models/domain/folder.ts index c336095f15d..5f7f17ee751 100644 --- a/libs/common/src/vault/models/domain/folder.ts +++ b/libs/common/src/vault/models/domain/folder.ts @@ -1,5 +1,3 @@ -// FIXME: Update this file to be type safe and remove this and next line -// @ts-strict-ignore import { Jsonify } from "type-fest"; import { EncryptService } from "../../../key-management/crypto/abstractions/encrypt.service"; @@ -9,16 +7,10 @@ import { SymmetricCryptoKey } from "../../../platform/models/domain/symmetric-cr import { FolderData } from "../data/folder.data"; import { FolderView } from "../view/folder.view"; -export class Test extends Domain { - id: string; - name: EncString; - revisionDate: Date; -} - export class Folder extends Domain { - id: string; - name: EncString; - revisionDate: Date; + id: string = ""; + name: EncString = new EncString(""); + revisionDate: Date = new Date(); constructor(obj?: FolderData) { super(); @@ -26,17 +18,9 @@ export class Folder extends Domain { return; } - this.buildDomainModel( - this, - obj, - { - id: null, - name: null, - }, - ["id"], - ); - - this.revisionDate = obj.revisionDate != null ? new Date(obj.revisionDate) : null; + this.id = obj.id; + this.name = new EncString(obj.name); + this.revisionDate = new Date(obj.revisionDate); } decrypt(key: SymmetricCryptoKey): Promise { @@ -62,7 +46,14 @@ export class Folder extends Domain { } static fromJSON(obj: Jsonify) { - const revisionDate = obj.revisionDate == null ? null : new Date(obj.revisionDate); - return Object.assign(new Folder(), obj, { name: EncString.fromJSON(obj.name), revisionDate }); + if (obj == null) { + return null; + } + + const folder = new Folder(); + folder.id = obj.id; + folder.name = EncString.fromJSON(obj.name); + folder.revisionDate = new Date(obj.revisionDate); + return folder; } } diff --git a/libs/common/src/vault/models/request/folder-with-id.request.ts b/libs/common/src/vault/models/request/folder-with-id.request.ts index 9d8078c12c1..8af890048ba 100644 --- a/libs/common/src/vault/models/request/folder-with-id.request.ts +++ b/libs/common/src/vault/models/request/folder-with-id.request.ts @@ -7,6 +7,6 @@ export class FolderWithIdRequest extends FolderRequest { constructor(folder: Folder) { super(folder); - this.id = folder.id; + this.id = folder.id ?? ""; } } diff --git a/libs/common/src/vault/models/view/folder.view.ts b/libs/common/src/vault/models/view/folder.view.ts index bc908e98eb8..6052ae9df37 100644 --- a/libs/common/src/vault/models/view/folder.view.ts +++ b/libs/common/src/vault/models/view/folder.view.ts @@ -1,19 +1,17 @@ -// FIXME: Update this file to be type safe and remove this and next line -// @ts-strict-ignore import { Jsonify } from "type-fest"; import { View } from "../../../models/view/view"; -import { DecryptedObject } from "../../../platform/models/domain/domain-base"; import { Folder } from "../domain/folder"; import { ITreeNodeObject } from "../domain/tree-node"; export class FolderView implements View, ITreeNodeObject { - id: string = null; - name: string = null; - revisionDate: Date = null; + id: string = ""; + name: string = ""; + revisionDate: Date; - constructor(f?: Folder | DecryptedObject) { + constructor(f?: Folder) { if (!f) { + this.revisionDate = new Date(); return; } @@ -22,7 +20,12 @@ export class FolderView implements View, ITreeNodeObject { } static fromJSON(obj: Jsonify) { - const revisionDate = obj.revisionDate == null ? null : new Date(obj.revisionDate); - return Object.assign(new FolderView(), obj, { revisionDate }); + const folderView = new FolderView(); + folderView.id = obj.id ?? ""; + folderView.name = obj.name ?? ""; + if (obj.revisionDate != null) { + folderView.revisionDate = new Date(obj.revisionDate); + } + return folderView; } } diff --git a/libs/common/src/vault/services/folder/folder-api.service.ts b/libs/common/src/vault/services/folder/folder-api.service.ts index fe9c3218a84..d5bd7fe9847 100644 --- a/libs/common/src/vault/services/folder/folder-api.service.ts +++ b/libs/common/src/vault/services/folder/folder-api.service.ts @@ -17,11 +17,11 @@ export class FolderApiService implements FolderApiServiceAbstraction { const request = new FolderRequest(folder); let response: FolderResponse; - if (folder.id == null) { + if (folder.id) { + response = await this.putFolder(folder.id, request); + } else { response = await this.postFolder(request); folder.id = response.id; - } else { - response = await this.putFolder(folder.id, request); } const data = new FolderData(response); diff --git a/libs/common/src/vault/services/folder/folder.service.spec.ts b/libs/common/src/vault/services/folder/folder.service.spec.ts index a520fd4852d..412e67e77d7 100644 --- a/libs/common/src/vault/services/folder/folder.service.spec.ts +++ b/libs/common/src/vault/services/folder/folder.service.spec.ts @@ -122,6 +122,7 @@ describe("Folder Service", () => { encryptedString: "ENC", encryptionType: 0, }, + revisionDate: expect.any(Date), }); }); @@ -132,7 +133,7 @@ describe("Folder Service", () => { expect(result).toEqual({ id: "1", name: makeEncString("ENC_STRING_" + 1), - revisionDate: null, + revisionDate: expect.any(Date), }); }); @@ -150,12 +151,12 @@ describe("Folder Service", () => { { id: "1", name: makeEncString("ENC_STRING_" + 1), - revisionDate: null, + revisionDate: expect.any(Date), }, { id: "2", name: makeEncString("ENC_STRING_" + 2), - revisionDate: null, + revisionDate: expect.any(Date), }, ]); }); @@ -167,7 +168,7 @@ describe("Folder Service", () => { { id: "4", name: makeEncString("ENC_STRING_" + 4), - revisionDate: null, + revisionDate: expect.any(Date), }, ]); }); @@ -203,7 +204,7 @@ describe("Folder Service", () => { const folderViews = await firstValueFrom(folderService.folderViews$(mockUserId)); expect(folderViews.length).toBe(1); - expect(folderViews[0].id).toBeNull(); // Should be the "No Folder" folder + expect(folderViews[0].id).toEqual(""); // Should be the "No Folder" folder }); describe("getRotatedData", () => { diff --git a/libs/importer/src/components/import.component.ts b/libs/importer/src/components/import.component.ts index d58859ac163..86f5d765d31 100644 --- a/libs/importer/src/components/import.component.ts +++ b/libs/importer/src/components/import.component.ts @@ -354,7 +354,7 @@ export class ImportComponent implements OnInit, OnDestroy, AfterViewInit { switchMap((userId) => { return this.folderService.folderViews$(userId); }), - map((folders) => folders.filter((f) => f.id != null)), + map((folders) => folders.filter((f) => !!f.id)), ); this.formGroup.controls.targetSelector.disable(); diff --git a/libs/importer/src/importers/base-importer.ts b/libs/importer/src/importers/base-importer.ts index a32a53f3e60..9c617971f8f 100644 --- a/libs/importer/src/importers/base-importer.ts +++ b/libs/importer/src/importers/base-importer.ts @@ -2,12 +2,12 @@ // @ts-strict-ignore import * as papa from "papaparse"; -import { CollectionView, Collection } from "@bitwarden/common/admin-console/models/collections"; +import { CollectionView } from "@bitwarden/common/admin-console/models/collections"; import { normalizeExpiryYearFormat } from "@bitwarden/common/autofill/utils"; import { LogService } from "@bitwarden/common/platform/abstractions/log.service"; import { Utils } from "@bitwarden/common/platform/misc/utils"; import { ConsoleLogService } from "@bitwarden/common/platform/services/console-log.service"; -import { OrganizationId } from "@bitwarden/common/types/guid"; +import { CollectionId, OrganizationId } from "@bitwarden/common/types/guid"; import { FieldType, SecureNoteType, CipherType } from "@bitwarden/common/vault/enums"; import { CipherView } from "@bitwarden/common/vault/models/view/cipher.view"; import { FieldView } from "@bitwarden/common/vault/models/view/field.view"; @@ -277,8 +277,7 @@ export abstract class BaseImporter { const collection = new CollectionView({ name: f.name, organizationId: this.organizationId, - // FIXME: Folder.id may be null, this should be changed when refactoring Folders to be ts-strict - id: Collection.isCollectionId(f.id) ? f.id : null, + id: f.id && f.id !== "" ? (f.id as CollectionId) : null, }); return collection; }); diff --git a/libs/importer/src/importers/keepass2-xml-importer.spec.ts b/libs/importer/src/importers/keepass2-xml-importer.spec.ts index c1c0947936b..e934a442ff5 100644 --- a/libs/importer/src/importers/keepass2-xml-importer.spec.ts +++ b/libs/importer/src/importers/keepass2-xml-importer.spec.ts @@ -23,7 +23,7 @@ describe("KeePass2 Xml Importer", () => { const actual = [folder]; const result = await importer.parse(TestData); - expect(result.folders).toEqual(actual); + expect(result.folders[0].name).toEqual(actual[0].name); }); it("parse XML should contains login details", async () => { diff --git a/libs/importer/src/importers/password-depot/password-depot-17-xml-importer.spec.ts b/libs/importer/src/importers/password-depot/password-depot-17-xml-importer.spec.ts index 8b78b33c154..121c1b5dd66 100644 --- a/libs/importer/src/importers/password-depot/password-depot-17-xml-importer.spec.ts +++ b/libs/importer/src/importers/password-depot/password-depot-17-xml-importer.spec.ts @@ -57,10 +57,15 @@ describe("Password Depot 17 Xml Importer", () => { const importer = new PasswordDepot17XmlImporter(); const folder = new FolderView(); folder.name = "tempDB"; - const actual = [folder]; const result = await importer.parse(PasswordTestData); - expect(result.folders).toEqual(actual); + expect(result.folders).toEqual([ + expect.objectContaining({ + id: "", + name: "tempDB", + revisionDate: expect.any(Date), + }), + ]); }); it("should parse password type into logins", async () => { diff --git a/libs/tools/export/vault-export/vault-export-core/src/services/individual-vault-export.service.spec.ts b/libs/tools/export/vault-export/vault-export-core/src/services/individual-vault-export.service.spec.ts index 7d28d857403..55ad39bc8e0 100644 --- a/libs/tools/export/vault-export/vault-export-core/src/services/individual-vault-export.service.spec.ts +++ b/libs/tools/export/vault-export/vault-export-core/src/services/individual-vault-export.service.spec.ts @@ -138,7 +138,7 @@ function expectEqualCiphers(ciphers: CipherView[] | Cipher[], jsonResult: string } function expectEqualFolderViews(folderViews: FolderView[] | Folder[], jsonResult: string) { - const actual = JSON.stringify(JSON.parse(jsonResult).folders); + const actual = JSON.parse(jsonResult).folders; const folders: FolderResponse[] = []; folderViews.forEach((c) => { const folder = new FolderResponse(); @@ -148,21 +148,19 @@ function expectEqualFolderViews(folderViews: FolderView[] | Folder[], jsonResult }); expect(actual.length).toBeGreaterThan(0); - expect(actual).toEqual(JSON.stringify(folders)); + expect(actual).toEqual(folders); } function expectEqualFolders(folders: Folder[], jsonResult: string) { - const actual = JSON.stringify(JSON.parse(jsonResult).folders); - const items: Folder[] = []; - folders.forEach((c) => { - const item = new Folder(); - item.id = c.id; - item.name = c.name; - items.push(item); - }); + const actual = JSON.parse(jsonResult).folders; + + const expected = folders.map((c) => ({ + id: c.id, + name: c.name?.encryptedString, + })); expect(actual.length).toBeGreaterThan(0); - expect(actual).toEqual(JSON.stringify(items)); + expect(actual).toEqual(expected); } describe("VaultExportService", () => { diff --git a/libs/tools/export/vault-export/vault-export-core/src/services/individual-vault-export.service.ts b/libs/tools/export/vault-export/vault-export-core/src/services/individual-vault-export.service.ts index b30f14872ca..a5e9f8aea6e 100644 --- a/libs/tools/export/vault-export/vault-export-core/src/services/individual-vault-export.service.ts +++ b/libs/tools/export/vault-export/vault-export-core/src/services/individual-vault-export.service.ts @@ -240,7 +240,7 @@ export class IndividualVaultExportService }; folders.forEach((f) => { - if (f.id == null) { + if (!f.id) { return; } const folder = new FolderWithIdExport(); @@ -268,7 +268,7 @@ export class IndividualVaultExportService private buildCsvExport(decFolders: FolderView[], decCiphers: CipherView[]): string { const foldersMap = new Map(); decFolders.forEach((f) => { - if (f.id != null) { + if (f.id) { foldersMap.set(f.id, f); } }); @@ -302,7 +302,7 @@ export class IndividualVaultExportService }; decFolders.forEach((f) => { - if (f.id == null) { + if (!f.id) { return; } const folder = new FolderWithIdExport(); diff --git a/libs/vault/src/components/add-edit-folder-dialog/add-edit-folder-dialog.component.spec.ts b/libs/vault/src/components/add-edit-folder-dialog/add-edit-folder-dialog.component.spec.ts index 9bf53826333..a783bdc7406 100644 --- a/libs/vault/src/components/add-edit-folder-dialog/add-edit-folder-dialog.component.spec.ts +++ b/libs/vault/src/components/add-edit-folder-dialog/add-edit-folder-dialog.component.spec.ts @@ -101,7 +101,7 @@ describe("AddEditFolderDialogComponent", () => { const newFolder = new FolderView(); newFolder.name = "New Folder"; - expect(encrypt).toHaveBeenCalledWith(newFolder, ""); + expect(encrypt).toHaveBeenCalledWith(expect.objectContaining({ name: "New Folder" }), ""); expect(save).toHaveBeenCalled(); }); diff --git a/libs/vault/src/models/filter-function.spec.ts b/libs/vault/src/models/filter-function.spec.ts index 1ffc1b119a8..de544a1a0d5 100644 --- a/libs/vault/src/models/filter-function.spec.ts +++ b/libs/vault/src/models/filter-function.spec.ts @@ -116,6 +116,14 @@ describe("createFilter", () => { expect(result).toBe(true); }); + + it("should return true when filtering on empty-string folder id", () => { + const filterFunction = createFilterFunction({ folderId: "" }); + + const result = filterFunction(cipher); + + expect(result).toBe(true); + }); }); describe("given an organizational cipher (with organization and collections)", () => { diff --git a/libs/vault/src/models/filter-function.ts b/libs/vault/src/models/filter-function.ts index 0252ef13094..bbd4127863b 100644 --- a/libs/vault/src/models/filter-function.ts +++ b/libs/vault/src/models/filter-function.ts @@ -55,8 +55,11 @@ export function createFilterFunction( return false; } } + const isNoFolderFilter = filter.folderId === Unassigned || filter.folderId === ""; + const cipherHasFolder = cipher.folderId != null && cipher.folderId !== ""; + // No folder - if (filter.folderId === Unassigned && cipher.folderId != null) { + if (isNoFolderFilter && cipherHasFolder) { return false; } // Folder @@ -64,6 +67,7 @@ export function createFilterFunction( filter.folderId !== undefined && filter.folderId !== All && filter.folderId !== Unassigned && + filter.folderId !== "" && cipher.folderId !== filter.folderId ) { return false; diff --git a/libs/vault/src/models/routed-vault-filter-bridge.model.ts b/libs/vault/src/models/routed-vault-filter-bridge.model.ts index 1d6d73ba7c5..32523684125 100644 --- a/libs/vault/src/models/routed-vault-filter-bridge.model.ts +++ b/libs/vault/src/models/routed-vault-filter-bridge.model.ts @@ -87,7 +87,7 @@ export class RoutedVaultFilterBridge implements VaultFilter { return this.legacyFilter.selectedFolderNode; } set selectedFolderNode(value: TreeNode) { - const folderId = value != null && value.node.id === null ? Unassigned : value?.node.id; + const folderId = value?.node.id ? value.node.id : Unassigned; this.bridgeService.navigate({ ...this.routedFilter, folderId, diff --git a/libs/vault/src/models/vault-filter.model.ts b/libs/vault/src/models/vault-filter.model.ts index 4617102cebe..5452a9f5c38 100644 --- a/libs/vault/src/models/vault-filter.model.ts +++ b/libs/vault/src/models/vault-filter.model.ts @@ -134,7 +134,7 @@ export class VaultFilter { if (this.selectedFolderNode) { // No folder if (this.folderId === null && cipherPassesFilter) { - cipherPassesFilter = cipher.folderId === null; + cipherPassesFilter = cipher.folderId == null || cipher.folderId === ""; } // Folder if (this.folderId !== null && cipherPassesFilter) { diff --git a/libs/vault/src/services/routed-vault-filter-bridge.service.ts b/libs/vault/src/services/routed-vault-filter-bridge.service.ts index 1bff764964e..25c75f464f0 100644 --- a/libs/vault/src/services/routed-vault-filter-bridge.service.ts +++ b/libs/vault/src/services/routed-vault-filter-bridge.service.ts @@ -145,7 +145,7 @@ function createLegacyFilterForEndUser( ); } - if (filter.folderId !== undefined && filter.folderId === Unassigned) { + if (filter.folderId !== undefined && (filter.folderId === Unassigned || filter.folderId === "")) { legacyFilter.selectedFolderNode = ServiceUtils.getTreeNodeObject(folderTree, null); } else if (filter.folderId !== undefined && filter.folderId !== Unassigned) { legacyFilter.selectedFolderNode = ServiceUtils.getTreeNodeObject(folderTree, filter.folderId); diff --git a/libs/vault/src/services/vault-filter.service.spec.ts b/libs/vault/src/services/vault-filter.service.spec.ts index 90af45e571f..537e9c9f542 100644 --- a/libs/vault/src/services/vault-filter.service.spec.ts +++ b/libs/vault/src/services/vault-filter.service.spec.ts @@ -195,8 +195,8 @@ describe("vault filter service", () => { ]; folderViews.next(storedFolders); - await expect(firstValueFrom(vaultFilterService.filteredFolders$)).resolves.toEqual([ - createFolderView("folder test id", "test"), + await expect(firstValueFrom(vaultFilterService.filteredFolders$)).resolves.toMatchObject([ + { id: "folder test id", name: "test" }, ]); }); diff --git a/libs/vault/src/services/vault-filter.service.ts b/libs/vault/src/services/vault-filter.service.ts index 445764827eb..5dbab72e1d3 100644 --- a/libs/vault/src/services/vault-filter.service.ts +++ b/libs/vault/src/services/vault-filter.service.ts @@ -290,9 +290,7 @@ export class VaultFilterService implements VaultFilterServiceAbstraction { // Otherwise, show only folders that have ciphers from the selected org and the "no folder" folder const orgCiphers = ciphers.filter((c) => c.organizationId == org?.id); - return storedFolders.filter( - (f) => orgCiphers.some((oc) => oc.folderId == f.id) || f.id == null, - ); + return storedFolders.filter((f) => orgCiphers.some((oc) => oc.folderId == f.id) || !f.id); } protected buildFolderTree(folders?: FolderView[]): TreeNode {