Conversation
Replaces the global unique index on collections.name with a per-project (project_id, name) index, allowing sandboxes to share collection names with their parent. Updates the schema constraint to match and adds conflict detection to get_collection/1, plus a new get_collection/2 that takes a project_id for unambiguous lookup.
Adds a :conflict clause to FallbackController so that when get_collection/1 finds the same collection name in multiple projects, all actions return a 409 with a message directing clients to use the v2 API.
Fixes the create_collection test to match the new constraint name (collections_project_id_name_index). Adds tests for get_collection/1 returning :conflict when the same name exists in multiple projects, and for get_collection/2 resolving unambiguously by project_id. Adds controller tests asserting all v1 endpoints return 409 when a name conflict exists.
Replaces the explicit collections routes with a single wildcard that dispatches to v1 or v2 logic based on the X-API-VERSION header. V2 routes include project_id in the path, resolving collections unambiguously without name-collision risk. Also migrates the download endpoint to v2 (/:project_id/:name) and updates the LiveView download link accordingly.
Covers all v2 endpoints (GET item, stream, PUT, POST, DELETE item, DELETE all), including access control with personal and run tokens, and the key scenario where v2 resolves correctly by project_id when the same collection name exists in multiple projects.
Adds clone_collections_from_parent/2 to the sandbox provisioning pipeline. When a sandbox is forked, empty collection records matching each parent collection name are created in the new project. Items are not copied. Uses on_conflict: :nothing so re-provisioning is safe.
Adds sync_collections/2, called after a successful merge. Collections present in the sandbox but not the parent are created (empty) in the parent. Collections present in the parent but not the sandbox are deleted from the parent along with all their items. Collections in both are left untouched. Data is never merged.
Extends the merge confirmation warning to explain that collection names will be synchronized: new sandbox collections are created empty in the target, and collections deleted in the sandbox are permanently removed from the target including all their data.
The single dispatch function had a cyclomatic complexity of 14 (max 9). Splitting by version brings each function to 7 branches.
|
Testing and QA notes:
|
|
@elias-ba one thing I didn't think to check is access control. A project should not be able to access the collections of another project. How can we enforce this? Using the CLI you can send any access token you want and so long as the project is within that access token's scope, you're good. In the worker, we send a JWT which I believe is scoped only to access for that workflow. That must be sufficient right? We just look at the scope of the access token? |
Address review feedback on the sandbox collections work: - Add a plug that validates the x-api-version header early and returns 400 for unsupported values instead of silently falling back to v1 - Consolidate v1/v2 controller actions through a shared resolve/1 helper, dropping ~100 lines of duplication - Return 422 with a clear error on missing value/items in request body (previously raised FunctionClauseError) - Move sync_collections into the Sandboxes context, wrap creates and deletes in a single transaction, and replace the N delete_collection calls with a single batch delete_all - Scope the collection unique constraint error to :name instead of :project_id so the user-facing field reports the conflict - Tone down the merge warning copy while keeping the essential info Adds tests for unsupported/garbage x-api-version values, malformed request bodies, and transaction rollback behavior for sync_collections.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #4613 +/- ##
==========================================
+ Coverage 89.57% 89.60% +0.02%
==========================================
Files 444 445 +1
Lines 21505 21567 +62
==========================================
+ Hits 19263 19325 +62
Misses 2242 2242 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Good question @josephjclark , and yes, the access token scope is sufficient. The controller already enforces it. Every action calls
So a CLI user with a PAT can only touch collections in projects they're a member of, and a worker's run token can only touch collections in the same project as the run it was issued for. There's a test that specifically covers the worker case ( One subtle thing worth noting: v2 makes this tighter by design. With v1, a request with a stale or leaked token could at least look up a collection by name globally; v2 requires you to know (and be authorized for) the project_id upfront. |
There was a problem hiding this comment.
Hey @josephjclark this is now looking good to me and it has all things we discussed this morning. Please do test / QA this extensively and let me know if you find anything weird
Security ReviewClaude hit the max-turns limit or encountered an error before posting findings. See the workflow run for details. |
- Log a warning when collection sync fails after merge instead of silently ignoring the error - Remove on_conflict: :nothing from insert_empty_collections since both callers guarantee no duplicates can exist - Rename misleading test to match what it actually verifies
2 participants
Description
Sandboxes can't support projects that use collections because collection names are globally unique, and the v1 collections API has no way to disambiguate between a parent and its sandbox. This PR relaxes the uniqueness constraint to per-project and adds a v2 API that resolves collections by
(project_id, name).On the lifecycle side, empty collection records are now cloned into a sandbox when it's provisioned so that
collections.get('my-collection')resolves correctly without copying production data. When a sandbox is merged back into its parent, collection names are synchronised (new collections in the sandbox are created empty in the parent, collections missing from the sandbox are removed from the parent). Collection data is never synced.V1 continues to work for any project that doesn't have name collisions. When a v1 lookup finds the same name across multiple projects, it returns 409 Conflict pointing clients at v2. API version is selected via the
x-api-versionheader (missing or"1"= v1,"2"= v2, anything else = 400 Bad Request). Header-based versioning was chosen in discussion with @stuartc and @josephjclark to keep a single collections base path.Closes #3548
Validation steps
patients, add some items, then spawn a sandbox from it. The sandbox should have an emptypatientscollection.patientscollection, then merge the sandbox. Parent project'spatientscollection should still only have the original items (no data sync).staging-onlyin a sandbox, then merge. The parent should now have an emptystaging-onlycollection.GET /collections/patientswith a token that has access to both the parent and sandbox, should return 409. Repeat withx-api-version: 2andGET /collections/:project_id/patients, should resolve correctly.x-api-version: 99, should return 400 Bad Request.Additional notes for the reviewer
AI Usage
Pre-submission checklist
/reviewwith Claude Code)