Draft
Conversation
…Vault Add full CRUD support for managing collections on Android, accessible via Settings > Vault > Collections. Collections are organization-scoped vault items available on paid plans. Changes include: - Network layer: CollectionsApi, CollectionService, request/response models - Data layer: CollectionManager with encrypt > API > disk > decrypt flow - Permission model: expanded SyncResponseJson.Permissions and Organization with collection-specific permission fields - UI: CollectionsScreen (list with org subtitles, permission-gated FAB), CollectionAddEditScreen (name field, save, delete with confirmation) - Navigation: type-safe routes wired through VaultSettings entry point - VaultDiskSource.deleteCollection and VaultSdkSource.encryptCollection stub Note: encryptCollection is stubbed pending SDK release (SDK changes are implemented but not yet published). Create/update will fail at runtime until the SDK is updated.
Remove the UnsupportedOperationException stub and delegate to the actual SDK collections().encrypt() method. Requires SDK version with collection encryption support (not yet in published 2.0.0-5451).
Add vaultUrl parameter to SsoCookieVendorConfig and handle new KeyConnectorUrl variant in InitUserCryptoMethod when expressions. These changes are required for compatibility with the latest sdk-internal build used for local collection encryption testing.
The PUT endpoint for updating a collection requires groups and users access permissions in the request body. Previously only the encrypted name was sent, causing the server to reject the request with "At least one member or group must have can manage permission." The update flow now fetches collection details via the new /details endpoint before sending the PUT request, echoing back existing groups, users, and externalId. Also fixes collection edit screen passing organizationName instead of organizationId and resolves compile errors from new parameters across tests.
- Add organizationUserId to SyncResponseJson and Organization domain model to identify the current user's org membership ID - Include creating user with manage access in collection create request, matching web client behavior - Add limitCollectionCreation/limitCollectionDeletion to org model - Fix FAB visibility: use canManageCollections computed property that checks role (Owner/Admin) in addition to permissions flags, matching web client logic: !limitCollectionCreation || isAdmin || permissions
Contributor
|
Great job! No new security vulnerabilities introduced in this pull request |
Code review fixes: - Remove duplicate KeyConnectorUrl branch in InitUserCryptoMethodExtensions - Fix CollectionManagerTest createCollection calls to include organizationUserId - Prevent vault sync from overwriting user's in-progress edits in CollectionAddEditViewModel (early return if already in Content state) - Add per-collection canManage permission check before allowing edit navigation, based on collection manage flag and org role - Gitignore .claude/outputs/ to exclude plan documents from commits New tests: - CollectionsViewModelTest: 11 tests covering navigation, state updates, FAB visibility based on permissions, snackbar relay, and error states - CollectionAddEditViewModelTest: 20 tests covering create/edit/delete flows, name validation, dialog states, snackbar relay, and the sync overwrite protection fix Updated test fixtures: - SyncResponseProfileUtil: add organizationUserId, limitCollectionCreation, limitCollectionDeletion fields
quexten
reviewed
Mar 26, 2026
| ), | ||
| ) | ||
| } | ||
| return vaultSdkSource |
Contributor
There was a problem hiding this comment.
Thought: Encrypt-only in SDK was really only meant as a transition phase while API, state were not mature in the SDK, and the expectation is to move state and API to the SDK too. In this case, moving at least API one layer down into the SDK would already reduce this PR by a large margin, and remove redundant work from the corresponding iOS PR and would allow dropping TS clients code.
Given that this is a net new feature, can we consider just moving this functionality into the SDK in the first place instead of duplicating logic? Moving this down later will cause more effort overall compared to doing it here.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

Summary
Test plan
/character is rejected in collection nameNotes
encryptCollectionrequires a local SDK build fromsdk-internalwith collection encryption support (not yet in published SDK)27eab557adapts to local SDK API changes for development; will need adjustment when official SDK is bumpeddocs/COLLECTIONS_REQUIREMENTS.md.claude/outputs/plans/COLLECTION-MANAGEMENT-PLAN.md