[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.
pull/20085/merge
Jared 7 days ago committed by GitHub
parent 34b633f685
commit 114dee78e3
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194

@ -29,7 +29,7 @@ const stubSdkCollection: SdkCollection = {
readOnly: false,
manage: false,
defaultUserCollectionEmail: undefined,
type: "SharedCollection",
type: CollectionTypes.SharedCollection,
};
function makeCollection(overrides: Partial<Collection> = {}): Collection {
@ -50,7 +50,7 @@ function makeSdkCollectionView(overrides: Partial<SdkCollectionView> = {}): 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);

@ -32,7 +32,7 @@ function makeSdkCollection(overrides: Partial<SdkCollection> = {}): SdkCollectio
readOnly: false,
manage: true,
defaultUserCollectionEmail: undefined,
type: "SharedCollection",
type: CollectionTypes.SharedCollection,
...overrides,
};
}
@ -46,7 +46,7 @@ function makeSdkCollectionView(overrides: Partial<SdkCollectionView> = {}): 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", () => {

@ -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<SdkCollectionType, CollectionType>;
export const collectionTypeToSdkType = {
[CollectionTypes.SharedCollection]: "SharedCollection",
[CollectionTypes.DefaultUserCollection]: "DefaultUserCollection",
} satisfies Record<CollectionType, SdkCollectionType>;
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,
};
}

@ -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,
};
}

16
package-lock.json generated

@ -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"

@ -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",

Loading…
Cancel
Save