[PM-34918] Get rid of encrypt as any by JaredScar · Pull Request #20472 · bitwarden/clients

JaredScar · 2026-05-01T15:59:50Z

🎟️ Tracking

https://bitwarden.atlassian.net/browse/PM-34918

📔 Objective

Gets rid of as any for encrypt function.

- Bump versions of `@bitwarden/commercial-sdk-internal` and `@bitwarden/sdk-internal` to `0.2.0-main.715` in package.json and package-lock.json. - Refactor `vault.component.ts` to utilize Angular's inject and Signal features, improving dependency injection and state management. - Enhance the `DefaultCollectionEncryptionService` to augment the `@bitwarden/sdk-internal` module for better type definitions related to collection encryption.

github-actions · 2026-05-01T16:01:49Z

🤖 Bitwarden Claude Code Review

Overall Assessment: APPROVE

This PR removes the as any cast around CollectionsClient.encrypt in default-collection-encryption.service.ts by introducing a TypeScript module augmentation, plus a substantial refactor that extracts logic from VaultComponent into three new injectable services (DefaultVaultCollectionService, VaultCipherActionsService, VaultCollectionActionsService) with accompanying spec coverage. The previously raised critical concerns (unresolved imports, template/component sync, missing init() call, and refresh() no longer re-fetching collections) have all been addressed in subsequent commits, and the package-lock.json removal flagged earlier is no longer present in the diff.

Code Review Details

No new findings. Previously raised threads have been resolved:

❌ resolved: Unresolved import paths in vault.component.ts
❌ resolved: Template out of sync with renamed component members
❌ resolved: Missing init() method invocation (outdated)
⚠️ resolved: refresh() now invokes collectionService.reload() to re-fetch collections from the API
❌ resolved: package-lock.json deletions removed from the diff

The declare module "@bitwarden/sdk-internal" augmentation in default-collection-encryption.service.ts is a clear, well-commented bridge with a TODO to remove it once the SDK package types are updated. Test coverage for the two new action services (vault-cipher-actions.service.spec.ts and vault-collection-actions.service.spec.ts) is comprehensive across the major control flows.

- Introduced `DefaultVaultCollectionService` and `VaultCipherActionsService` to manage vault collections and cipher actions, enhancing organization and user interaction. - Implemented unit tests for both services to ensure functionality and reliability. - Added `VaultCollectionActionsService` for managing collection actions, including adding, editing, and deleting collections. - Created specifications for `VaultCollectionActionsService` to validate its behavior and interactions with the dialog components. - Established a base `VaultCollectionService` for shared collection functionalities across services.

- Updated variable names for clarity, changing `refreshingSubject$` to `isRefreshing$`. - Modified the way `activeFilter` is accessed, ensuring it is called as a function. - Adjusted conditions for `showAddAccessToggle` and `activeFilter.isDeleted` to use function calls for consistency. - Enhanced data binding for `addAccessStatus` to utilize `collectionService` directly.

- Removed the initialization of collection actions in ngOnInit for cleaner code structure. - Added subscription to collectionActions.refresh$ to ensure the component refreshes appropriately when collection actions are triggered.

- Added `jest-environment-jsdom` version 30.3.0 to both package.json and package-lock.json for enhanced testing capabilities. - Updated `dev` properties to `devOptional` for better clarity in package-lock.json. - Introduced new modules `@egjs/hammerjs` and `@electron/windows-sign` with their respective dependencies and configurations.

…efresh logic - Removed the `allCollectionsWithoutUnassigned$` observable to streamline the component's state management. - Added a call to `collectionService.reload()` in the refresh method to ensure data is reloaded appropriately during refresh actions.

…ackage-lock.json - Downgraded `jest-environment-jsdom` from version 30.3.0 to 29.7.0 for compatibility reasons. - Added new modules including `@tootallnate/once`, `abab`, `acorn-globals`, `cssom`, and `domexception` with their respective versions and metadata. - Updated dependencies for existing modules to ensure proper functionality and maintainability.

codecov · 2026-05-01T17:30:45Z

Codecov Report

❌ Patch coverage is 61.11111% with 210 lines in your changes missing coverage. Please review.
✅ Project coverage is 47.33%. Comparing base (bfb6c9b) to head (3d68c3a).
⚠️ Report is 1 commits behind head on main.
✅ All tests successful. No failed tests found.

Files with missing lines	Patch %	Lines
...nsole/organizations/collections/vault.component.ts	0.00%	100 Missing ⚠️
...tions/services/default-vault-collection.service.ts	0.00%	89 Missing ⚠️
...llections/services/vault-cipher-actions.service.ts	92.43%	10 Missing and 9 partials ⚠️
...tions/services/vault-collection-actions.service.ts	97.93%	0 Missing and 2 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #20472      +/-   ##
==========================================
+ Coverage   47.09%   47.33%   +0.24%     
==========================================
  Files        3949     3953       +4     
  Lines      119741   119886     +145     
  Branches    18344    18343       -1     
==========================================
+ Hits        56390    56748     +358     
+ Misses      59115    58891     -224     
- Partials     4236     4247      +11

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

…; update dev properties for several modules to true. Clean up package-lock.json by removing unused dependencies including @egjs/hammerjs and @electron/windows-sign.

… versions to 0.2.0-main.712 in package.json and package-lock.json; adjust dev properties for clarity and add new dependencies including @egjs/hammerjs and @electron/windows-sign.

sonarqubecloud · 2026-05-01T18:22:01Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
3.3% Duplication on New Code

See analysis details on SonarQube Cloud

eliykat

This has a few sets of changes in it. The encrypt method is covered by the reversion in #20477. The actions service changes seem unrelated and maybe belong to another branch? I suggest we close this PR and you can open a new one for those changes if needed.

JaredScar requested a review from eliykat May 1, 2026 15:59

JaredScar self-assigned this May 1, 2026

JaredScar requested a review from a team as a code owner May 1, 2026 15:59

JaredScar added the ai-review Request a Claude code review label May 1, 2026