From 114dee78e32acc3f8e01fdc69e1d8d7166dec6c7 Mon Sep 17 00:00:00 2001 From: Jared Date: Tue, 12 May 2026 10:51:56 -0400 Subject: [PATCH] [PM-36565] creating new item from new dialog is not working (#20560) * Refactor bidirectional mapping for CollectionType and SDK types Updated the mapping between numeric CollectionType and SDK's numeric variant to ensure type safety. The previous string variant has been replaced with numeric values for better consistency and maintainability. This change enhances type checking in TypeScript and ensures that any additions to the CollectionTypes require corresponding updates in the mapping. * Refactor CollectionType handling in collection models Removed bidirectional mapping for CollectionType and SDK types, simplifying the type assignment in the Collection and CollectionView classes. This change enhances code clarity and reduces complexity by directly using the SDK's numeric variant for type assignments. * Update CollectionType handling in tests to use numeric values Replaced string representations of CollectionType with their corresponding numeric values in the collection SDK mapping tests. This change ensures consistency with the recent refactor of type handling, enhancing type safety and maintainability across the codebase. * Update tests to use CollectionTypes for type assignments Replaced numeric values with the CollectionTypes enum in the collection SDK mapping tests. This change improves code clarity and consistency, aligning with recent refactors to enhance type safety and maintainability across the codebase. * Update Bitwarden SDK dependencies to version 0.2.0-main.739 in package.json and package-lock.json. This change ensures compatibility with the latest features and fixes in the SDK. * Refactor tests to utilize CollectionTypes enum for type assignments Updated test cases in default-collection-encryption.service.spec.ts to replace string literals with the CollectionTypes enum for improved type safety and consistency. This aligns with recent refactors aimed at enhancing code clarity and maintainability. --- ...ault-collection-encryption.service.spec.ts | 8 +++-- .../collection-sdk-mapping.spec.ts | 32 +++++++++---------- .../models/collections/collection.ts | 24 ++------------ .../models/collections/collection.view.ts | 12 ++----- package-lock.json | 16 +++++----- package.json | 4 +-- 6 files changed, 37 insertions(+), 59 deletions(-) diff --git a/libs/admin-console/src/common/collections/services/default-collection-encryption.service.spec.ts b/libs/admin-console/src/common/collections/services/default-collection-encryption.service.spec.ts index b936f0ef0f1..7cd3c7fc3d0 100644 --- a/libs/admin-console/src/common/collections/services/default-collection-encryption.service.spec.ts +++ b/libs/admin-console/src/common/collections/services/default-collection-encryption.service.spec.ts @@ -29,7 +29,7 @@ const stubSdkCollection: SdkCollection = { readOnly: false, manage: false, defaultUserCollectionEmail: undefined, - type: "SharedCollection", + type: CollectionTypes.SharedCollection, }; function makeCollection(overrides: Partial = {}): Collection { @@ -50,7 +50,7 @@ function makeSdkCollectionView(overrides: Partial = {}): SdkC hidePasswords: false, readOnly: false, manage: true, - type: "SharedCollection", + type: CollectionTypes.SharedCollection, ...overrides, }; } @@ -188,7 +188,9 @@ describe("DefaultCollectionEncryptionService", () => { type: CollectionTypes.DefaultUserCollection, }); jest.spyOn(collection, "toSdkCollection").mockReturnValue(stubSdkCollection); - mockDecrypt.mockReturnValue(makeSdkCollectionView({ type: "DefaultUserCollection" })); + mockDecrypt.mockReturnValue( + makeSdkCollectionView({ type: CollectionTypes.DefaultUserCollection }), + ); const [result] = await service.decryptMany([collection], userId); diff --git a/libs/common/src/admin-console/models/collections/collection-sdk-mapping.spec.ts b/libs/common/src/admin-console/models/collections/collection-sdk-mapping.spec.ts index 7b2cff51a31..143ca27bffd 100644 --- a/libs/common/src/admin-console/models/collections/collection-sdk-mapping.spec.ts +++ b/libs/common/src/admin-console/models/collections/collection-sdk-mapping.spec.ts @@ -32,7 +32,7 @@ function makeSdkCollection(overrides: Partial = {}): SdkCollectio readOnly: false, manage: true, defaultUserCollectionEmail: undefined, - type: "SharedCollection", + type: CollectionTypes.SharedCollection, ...overrides, }; } @@ -46,7 +46,7 @@ function makeSdkCollectionView(overrides: Partial = {}): SdkC hidePasswords: false, readOnly: false, manage: true, - type: "SharedCollection", + type: CollectionTypes.SharedCollection, ...overrides, }; } @@ -69,7 +69,7 @@ describe("Collection SDK mapping", () => { readOnly: true, manage: false, defaultUserCollectionEmail: "user@example.com", - type: "DefaultUserCollection", + type: CollectionTypes.DefaultUserCollection, }); const result = Collection.fromSdkCollection(sdkCollection); @@ -84,7 +84,7 @@ describe("Collection SDK mapping", () => { }); it("maps SharedCollection type correctly", () => { - const sdkCollection = makeSdkCollection({ type: "SharedCollection" }); + const sdkCollection = makeSdkCollection({ type: CollectionTypes.SharedCollection }); const result = Collection.fromSdkCollection(sdkCollection); expect(result.type).toBe(CollectionTypes.SharedCollection); }); @@ -114,13 +114,13 @@ describe("Collection SDK mapping", () => { expect(result.readOnly).toBe(true); expect(result.manage).toBe(false); expect(result.defaultUserCollectionEmail).toBe("user@example.com"); - expect(result.type).toBe("DefaultUserCollection"); + expect(result.type).toBe(CollectionTypes.DefaultUserCollection); }); it("maps SharedCollection type correctly", () => { const collection = makeCollection({ type: CollectionTypes.SharedCollection }); const result = collection.toSdkCollection(); - expect(result.type).toBe("SharedCollection"); + expect(result.type).toBe(CollectionTypes.SharedCollection); }); it("sets id to undefined when collection has no id", () => { @@ -148,7 +148,7 @@ describe("CollectionView SDK mapping", () => { hidePasswords: true, readOnly: true, manage: false, - type: "DefaultUserCollection", + type: CollectionTypes.DefaultUserCollection, }); const source = makeCollection(); @@ -164,7 +164,7 @@ describe("CollectionView SDK mapping", () => { }); it("maps SharedCollection type correctly", () => { - const sdkView = makeSdkCollectionView({ type: "SharedCollection" }); + const sdkView = makeSdkCollectionView({ type: CollectionTypes.SharedCollection }); const source = makeCollection(); const result = CollectionView.fromSdkCollectionView(sdkView, source); expect(result.type).toBe(CollectionTypes.SharedCollection); @@ -187,7 +187,7 @@ describe("CollectionView SDK mapping", () => { describe("defaultUserCollectionEmail preservation (canEditName security invariant)", () => { it("copies defaultUserCollectionEmail from the source collection, not the SDK view", () => { const email = "offboarded-user@example.com"; - const sdkView = makeSdkCollectionView({ type: "DefaultUserCollection" }); + const sdkView = makeSdkCollectionView({ type: CollectionTypes.DefaultUserCollection }); const source = makeCollection({ defaultUserCollectionEmail: email, type: CollectionTypes.DefaultUserCollection, @@ -199,7 +199,7 @@ describe("CollectionView SDK mapping", () => { }); it("leaves defaultUserCollectionEmail undefined for regular collections", () => { - const sdkView = makeSdkCollectionView({ type: "SharedCollection" }); + const sdkView = makeSdkCollectionView({ type: CollectionTypes.SharedCollection }); const source = makeCollection({ defaultUserCollectionEmail: undefined }); const result = CollectionView.fromSdkCollectionView(sdkView, source); @@ -208,7 +208,7 @@ describe("CollectionView SDK mapping", () => { }); it("canEditName returns false when defaultUserCollectionEmail is present", () => { - const sdkView = makeSdkCollectionView({ type: "DefaultUserCollection" }); + const sdkView = makeSdkCollectionView({ type: CollectionTypes.DefaultUserCollection }); const source = makeCollection({ defaultUserCollectionEmail: "offboarded@example.com", type: CollectionTypes.DefaultUserCollection, @@ -218,7 +218,7 @@ describe("CollectionView SDK mapping", () => { result.manage = true; // canEditName should be false because defaultUserCollectionEmail is set - const mockOrg: any = { canManageAllCollections: true }; + const mockOrg: any = { id: orgId, canManageAllCollections: true }; expect(result.canEditName(mockOrg)).toBe(false); }); @@ -226,7 +226,7 @@ describe("CollectionView SDK mapping", () => { // This test asserts the security invariant stated in the WARNING on canEditName(): // a DefaultUserCollection with a set defaultUserCollectionEmail must never be editable. const email = "ghost@example.com"; - const sdkView = makeSdkCollectionView({ type: "DefaultUserCollection" }); + const sdkView = makeSdkCollectionView({ type: CollectionTypes.DefaultUserCollection }); const source = makeCollection({ defaultUserCollectionEmail: email, type: CollectionTypes.DefaultUserCollection, @@ -236,7 +236,7 @@ describe("CollectionView SDK mapping", () => { sdkPathResult.manage = true; expect(sdkPathResult.defaultUserCollectionEmail).toBe(email); - const mockOrg: any = { canManageAllCollections: true }; + const mockOrg: any = { id: orgId, canManageAllCollections: true }; expect(sdkPathResult.canEditName(mockOrg)).toBe(false); }); }); @@ -262,14 +262,14 @@ describe("CollectionView SDK mapping", () => { expect(result.hidePasswords).toBe(true); expect(result.readOnly).toBe(false); expect(result.manage).toBe(true); - expect(result.type).toBe("DefaultUserCollection"); + expect(result.type).toBe(CollectionTypes.DefaultUserCollection); }); it("maps SharedCollection type correctly", () => { const view = new CollectionView({ id: collectionId, organizationId: orgId, name: "Test" }); view.type = CollectionTypes.SharedCollection; const result = view.toSdkCollectionView(); - expect(result.type).toBe("SharedCollection"); + expect(result.type).toBe(CollectionTypes.SharedCollection); }); it("sets id to undefined when view has no id", () => { diff --git a/libs/common/src/admin-console/models/collections/collection.ts b/libs/common/src/admin-console/models/collections/collection.ts index 56eebf4ca18..97cdc067306 100644 --- a/libs/common/src/admin-console/models/collections/collection.ts +++ b/libs/common/src/admin-console/models/collections/collection.ts @@ -5,10 +5,7 @@ import { asUuid, uuidAsString } from "@bitwarden/common/platform/abstractions/sd import Domain from "@bitwarden/common/platform/models/domain/domain-base"; import { CollectionId, OrganizationId } from "@bitwarden/common/types/guid"; import { OrgKey } from "@bitwarden/common/types/key"; -import { - Collection as SdkCollection, - CollectionType as SdkCollectionType, -} from "@bitwarden/sdk-internal"; +import { Collection as SdkCollection } from "@bitwarden/sdk-internal"; import { CollectionData } from "./collection.data"; @@ -19,21 +16,6 @@ export const CollectionTypes = { export type CollectionType = (typeof CollectionTypes)[keyof typeof CollectionTypes]; -/** - * Exhaustive bidirectional maps between our numeric CollectionType and the SDK's string variant. - * Typed with `satisfies Record<...>` so TypeScript will fail to compile if either side gains a - * new member without a corresponding entry being added here. - */ -export const sdkTypeToCollectionType = { - SharedCollection: CollectionTypes.SharedCollection, - DefaultUserCollection: CollectionTypes.DefaultUserCollection, -} satisfies Record; - -export const collectionTypeToSdkType = { - [CollectionTypes.SharedCollection]: "SharedCollection", - [CollectionTypes.DefaultUserCollection]: "DefaultUserCollection", -} satisfies Record; - export class Collection extends Domain { id: CollectionId; organizationId: OrganizationId; @@ -112,7 +94,7 @@ export class Collection extends Domain { collection.readOnly = sdkCollection.readOnly; collection.manage = sdkCollection.manage; collection.defaultUserCollectionEmail = sdkCollection.defaultUserCollectionEmail; - collection.type = sdkTypeToCollectionType[sdkCollection.type]; + collection.type = sdkCollection.type; return collection; } @@ -129,7 +111,7 @@ export class Collection extends Domain { readOnly: this.readOnly, manage: this.manage, defaultUserCollectionEmail: this.defaultUserCollectionEmail, - type: collectionTypeToSdkType[this.type], + type: this.type, }; } diff --git a/libs/common/src/admin-console/models/collections/collection.view.ts b/libs/common/src/admin-console/models/collections/collection.view.ts index e5f0cd1847d..7cfc4c9fe48 100644 --- a/libs/common/src/admin-console/models/collections/collection.view.ts +++ b/libs/common/src/admin-console/models/collections/collection.view.ts @@ -10,13 +10,7 @@ import { OrgKey } from "@bitwarden/common/types/key"; import { ITreeNodeObject } from "@bitwarden/common/vault/models/domain/tree-node"; import { CollectionView as SdkCollectionView } from "@bitwarden/sdk-internal"; -import { - Collection, - CollectionType, - CollectionTypes, - collectionTypeToSdkType, - sdkTypeToCollectionType, -} from "./collection"; +import { Collection, CollectionType, CollectionTypes } from "./collection"; import { CollectionAccessDetailsResponse } from "./collection.response"; export const NestingDelimiter = "/"; @@ -204,7 +198,7 @@ export class CollectionView implements View, ITreeNodeObject { view.manage = sdkView.manage; view.assigned = true; view.defaultUserCollectionEmail = sourceCollection.defaultUserCollectionEmail; - view.type = sdkTypeToCollectionType[sdkView.type]; + view.type = sdkView.type; return view; } @@ -224,7 +218,7 @@ export class CollectionView implements View, ITreeNodeObject { hidePasswords: this.hidePasswords, readOnly: this.readOnly, manage: this.manage, - type: collectionTypeToSdkType[this.type], + type: this.type, }; } diff --git a/package-lock.json b/package-lock.json index 9a73044b599..48d168c89af 100644 --- a/package-lock.json +++ b/package-lock.json @@ -23,9 +23,9 @@ "@angular/platform-browser": "20.3.18", "@angular/platform-browser-dynamic": "20.3.18", "@angular/router": "20.3.18", - "@bitwarden/commercial-sdk-internal": "0.2.0-main.737", + "@bitwarden/commercial-sdk-internal": "0.2.0-main.739", "@bitwarden/desktop-napi": "file:apps/desktop/desktop_native/napi", - "@bitwarden/sdk-internal": "0.2.0-main.737", + "@bitwarden/sdk-internal": "0.2.0-main.739", "@electron/fuses": "2.1.1", "@emotion/css": "11.13.5", "@koa/multer": "4.0.0", @@ -5723,9 +5723,9 @@ "link": true }, "node_modules/@bitwarden/commercial-sdk-internal": { - "version": "0.2.0-main.737", - "resolved": "https://registry.npmjs.org/@bitwarden/commercial-sdk-internal/-/commercial-sdk-internal-0.2.0-main.737.tgz", - "integrity": "sha512-Ygz39fTfBB9QtZnMzvdRUesmZ3dOF/ylpvNP4T/Ww7Hk8Lsv/K+hgr5GY7P7KvAciZSEcHH2ST+Zagxpzm2Z2g==", + "version": "0.2.0-main.739", + "resolved": "https://registry.npmjs.org/@bitwarden/commercial-sdk-internal/-/commercial-sdk-internal-0.2.0-main.739.tgz", + "integrity": "sha512-1cTVvrmU6FA/ycF6/NKjSRkeZ9mQX3NqUHra54pY3gsrlSzTtTZGIsflYr4nTKxnSKwwaHvravqRkFUUt299eQ==", "license": "BITWARDEN SOFTWARE DEVELOPMENT KIT LICENSE AGREEMENT", "dependencies": { "type-fest": "^5.0.0" @@ -5843,9 +5843,9 @@ "link": true }, "node_modules/@bitwarden/sdk-internal": { - "version": "0.2.0-main.737", - "resolved": "https://registry.npmjs.org/@bitwarden/sdk-internal/-/sdk-internal-0.2.0-main.737.tgz", - "integrity": "sha512-y3e0bOsmsJ1+T+8tdjgSCLW3IYCUh48QindWmObMB0Lk+rU+fYsyfroDZk/izObaBDyseIUKJH6GSqYp9FNrYw==", + "version": "0.2.0-main.739", + "resolved": "https://registry.npmjs.org/@bitwarden/sdk-internal/-/sdk-internal-0.2.0-main.739.tgz", + "integrity": "sha512-pqlbx3f+Sc+ndJVgAzlBP1dPpFO0UAC0g8m4KMRTEVN69nahfl0gnEg9B2FL2uHEErJzfNYdGvO0EQ++zDvgQw==", "license": "GPL-3.0", "dependencies": { "type-fest": "^5.0.0" diff --git a/package.json b/package.json index fe8d0734c1a..2ab4e2503e8 100644 --- a/package.json +++ b/package.json @@ -173,9 +173,9 @@ "@angular/platform-browser": "20.3.18", "@angular/platform-browser-dynamic": "20.3.18", "@angular/router": "20.3.18", - "@bitwarden/commercial-sdk-internal": "0.2.0-main.737", + "@bitwarden/commercial-sdk-internal": "0.2.0-main.739", "@bitwarden/desktop-napi": "file:apps/desktop/desktop_native/napi", - "@bitwarden/sdk-internal": "0.2.0-main.737", + "@bitwarden/sdk-internal": "0.2.0-main.739", "@electron/fuses": "2.1.1", "@emotion/css": "11.13.5", "@koa/multer": "4.0.0",