From d015d41818b6786869425559fd8b4a14b4f38837 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 --- .../models/vault-filter.model.spec.ts | 11 +++++++++++ .../vault-filter/models/vault-filter.model.ts | 15 ++++++++++++--- .../vault-filter/services/vault-filter.service.ts | 2 +- libs/importer/src/components/import.component.ts | 2 +- libs/importer/src/importers/base-importer.ts | 7 +++---- .../services/individual-vault-export.service.ts | 6 +++--- libs/vault/src/models/filter-function.spec.ts | 8 ++++++++ libs/vault/src/models/filter-function.ts | 6 +++++- .../models/routed-vault-filter-bridge.model.ts | 2 +- libs/vault/src/models/vault-filter.model.ts | 2 +- .../routed-vault-filter-bridge.service.ts | 2 +- libs/vault/src/services/vault-filter.service.ts | 4 +--- 12 files changed, 48 insertions(+), 19 deletions(-) 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/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/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 0a13809c299..c7c03889625 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/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.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 {