[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
2026-02-19 02:44:01 +00:00 · 2026-02-12 13:52:29 -08:00
parent 5a8d8b7ef0
commit d015d41818
12 changed files with 48 additions and 19 deletions
--- 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)", () => {
--- 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) {
--- 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,
        );
      }

--- 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();
--- 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;
    });
--- 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<string, FolderView>();
    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();
--- 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)", () => {
--- 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;
--- 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<FolderFilter>) {
-    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,
--- 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) {
--- 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);
--- 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<FolderFilter> {