feat(library): tenant-scoped CRUD nested under profile (Phase 1.b.5a-B)#8
Merged
Conversation
Wires `PostgresLibraryRepository` (introduced in waveflow#185) to a new
`/api/v1/profiles/{profile_id}/libraries/*` resource. Same tenancy
pattern as profiles: every handler reads `UserId` from the request
extension, the path supplies `profile_id`, and the repo SQL validates
the `library -> profile -> user` chain inline. A foreign profile id
or non-owned library 404s — no existence leak.
- New migration `20260530000002_library.sql` (BIGSERIAL pk, profile_id
FK with ON DELETE CASCADE, composite index on profile_id + updated_at).
- New `src/api/libraries.rs` with list/create/get/update/delete + full
OpenAPI annotations including 500s.
- Wired in `src/api/mod.rs` behind the same dev-auth gate as profiles:
503 in prod, `require_user_id` middleware when WAVEFLOW_DEV_AUTH=1.
- Integration tests cover auth gate, default color/icon fall-back,
tenant isolation across both list and id-based reads, partial PATCH
preservation, ON DELETE CASCADE from profile, and the prod 503 gate.
- Belt-and-braces: library table existence canary in `ready.rs` and
path-presence asserts in `openapi.rs`.
- Bumps waveflow-core rev to the merge commit of waveflow#185.
Signed-off-by: InstaZDLL <github.105mh@8shield.net>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughCe PR ajoute une API REST tenant-scoped ChangesSubsystem de gestion des bibliothèques multi-tenant
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/api/libraries.rs`:
- Around line 181-211: Dans create_library, valider côté serveur que le nom
n'est pas vide après trim: récupère req.name.trim(), si la chaîne résultante est
vide retourne immédiatement une réponse 400 (StatusCode::BAD_REQUEST) avec un
message d'erreur clair (ex. "name is required"), et n'appelle pas
PostgresLibraryRepository::insert_for_profile; construis LibraryDraft.name avec
la version trimée; enfin mets à jour l'annotation #[utoipa::path] pour
documenter la réponse 400 pour ce point d'entrée (create_library /
CreateLibraryRequest).
In `@tests/libraries.rs`:
- Around line 456-502: The test profile_delete_cascades_to_libraries currently
only checks the library via the deleted profile (p1) so rename the unused _p2 to
p2 (so it’s available) and after deleting p1 perform the GET against
/api/v1/profiles/{p2}/libraries/{lib_id} as well, asserting the response status
is StatusCode::NOT_FOUND to verify the library row was actually removed by the
ON DELETE CASCADE; update references to p2 in that added request and keep
existing checks for p1.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 27fb89e5-6856-4390-b62c-b5c657f242c3
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock,!Cargo.lock,!*.lock
📒 Files selected for processing (7)
Cargo.tomlmigrations/20260530000002_library.sqlsrc/api/libraries.rssrc/api/mod.rstests/libraries.rstests/openapi.rstests/ready.rs
- create_library: trim req.name and reject empty / whitespace-only payloads with 400 before the storage round-trip. Update the CreateLibraryRequest doc comment to reflect the new boundary validation and document the 400 in the OpenAPI #[utoipa::path] responses(...). Persist the trimmed form. - profile_delete_cascades_to_libraries: promote the previously-unused _p2 to p2 and after deleting p1 also GET the library via p2 to prove the row is truly gone via CASCADE (not just that the deleted profile path no longer resolves). - Add create_rejects_empty_name integration test covering the new 400 path across "", " ", and "\t\n " inputs, plus a follow-up list assertion confirming nothing was persisted. Signed-off-by: InstaZDLL <github.105mh@8shield.net>
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/api/libraries.rs (1)
284-309:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winLa validation
namemanque dansupdate_library.
create_libraryrejette les noms vides/whitespace avec 400, maisupdate_librarypassereq.namedirectement au repo sans vérification. Un PATCH{ "name": "" }ou{ "name": " " }contournerait la validation.🛡️ Correction proposée
async fn update_library( State(state): State<AppState>, Extension(UserId(user_id)): Extension<UserId>, Path((profile_id, id)): Path<(i64, i64)>, Json(req): Json<UpdateLibraryRequest>, ) -> impl IntoResponse { + // Same boundary validation as create — reject blank names. + let name = match req.name { + Some(n) => { + let trimmed = n.trim(); + if trimmed.is_empty() { + return (StatusCode::BAD_REQUEST, "name must not be empty").into_response(); + } + Some(trimmed.to_string()) + } + None => None, + }; let patch = LibraryUpdate { - name: req.name, + name, description: req.description, color_id: req.color_id, icon_id: req.icon_id, };Pense à documenter la réponse 400 dans l'annotation
#[utoipa::path]pourupdate_libraryégalement.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/api/libraries.rs` around lines 284 - 309, The update_library handler fails to validate the incoming name allowing empty or whitespace names to pass into LibraryUpdate and repo.update_for_profile; modify update_library to trim and validate req.name before constructing LibraryUpdate (reject empty/whitespace with an early 400 response), mirror the same validation used in create_library, and ensure the utoipa #[utoipa::path] annotation for update_library documents the 400 response for invalid name.tests/libraries.rs (1)
362-401: 🧹 Nitpick | 🔵 Trivial | 💤 Low valueAjouter un test pour la validation
namelors du PATCH.Si la validation dans
update_libraryest ajoutée (cf. commentaire sursrc/api/libraries.rs), un test analogue àcreate_rejects_empty_nameserait utile pour couvrir le cheminPATCH { "name": "" }→ 400.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/libraries.rs` around lines 362 - 401, Add a new test similar to create_rejects_empty_name that exercises update_library by sending a PATCH to /api/v1/profiles/{profile_id}/libraries/{id} with { "name": "" } and asserting a 400 response; locate the test scaffolding used in update_renames_in_place (spawn_app, mint_user, mint_profile) to create a library first, then issue the PATCH and assert error_for_status or explicitly check status==400 and validate the error body matches the validation error for name.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@src/api/libraries.rs`:
- Around line 284-309: The update_library handler fails to validate the incoming
name allowing empty or whitespace names to pass into LibraryUpdate and
repo.update_for_profile; modify update_library to trim and validate req.name
before constructing LibraryUpdate (reject empty/whitespace with an early 400
response), mirror the same validation used in create_library, and ensure the
utoipa #[utoipa::path] annotation for update_library documents the 400 response
for invalid name.
In `@tests/libraries.rs`:
- Around line 362-401: Add a new test similar to create_rejects_empty_name that
exercises update_library by sending a PATCH to
/api/v1/profiles/{profile_id}/libraries/{id} with { "name": "" } and asserting a
400 response; locate the test scaffolding used in update_renames_in_place
(spawn_app, mint_user, mint_profile) to create a library first, then issue the
PATCH and assert error_for_status or explicitly check status==400 and validate
the error body matches the validation error for name.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 972d3607-9bc1-422f-aafc-6b55e2792475
📒 Files selected for processing (2)
src/api/libraries.rstests/libraries.rs
Same boundary check as create_library: a PATCH whose `name` is
Some("") / Some(" ") trims to empty and short-circuits to 400
before the storage round-trip. An omitted name (None) stays
legitimate — COALESCE in the repo preserves the existing value.
- OpenAPI annotation for PATCH documents the new 400 response.
- UpdateLibraryRequest doc-comment notes the trim+validate rule.
- update_rejects_empty_name integration test exercises "", " "
and "\t\n " through the PATCH path, plus a follow-up GET
confirming the existing name wasn't blanked.
Signed-off-by: InstaZDLL <github.105mh@8shield.net>
Owner
Author
|
@coderabbitai bien vu, tu as raison sur les deux outside-diff findings. J'avais sciemment skip Appliqué dans c5c2cad :
|
|
Tip For best results, initiate chat on the files or code changes.
🐇 ✓ |
This was referenced May 30, 2026
3 tasks
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
1 participant
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
Wires the new
PostgresLibraryRepository(from waveflow#185, merged at04736478) to a nested HTTP resource at/api/v1/profiles/{profile_id}/libraries/*. Same tenancy contract as profiles, extended one level deeper:profile_id, middleware suppliesUserId.library → profile → userownership inline — a foreign profile id or non-owned library 404s without ever loading the row.INSERT … SELECT … WHERE p.user_id = $andUPDATE … RETURNING *eliminate the check-then-write / update-then-read races.Changes
20260530000002_library.sql—librarytable withid BIGSERIAL,profile_id BIGINT NOT NULL REFERENCES profile(id) ON DELETE CASCADE, name/description/color_id/icon_id, created_at/updated_at. Composite index(profile_id, updated_at DESC)for the per-tenant MRU list; covers the FK fan-out so no secondprofile_id-only index.src/api/libraries.rs— 5 verbs (list/create/get/update/delete) with full#[utoipa::path]annotations including 500 responses. Wire format dropsprofile_idsince the client always has it from the URL.src/api/mod.rs—libraries_routergated identically to profiles: 503 in production,require_user_idwhenWAVEFLOW_DEV_AUTH=1.tests/libraries.rs— 9 integration tests covering: 401 auth gate, default color/icon fall-back, create-list-get round-trip, 404 on foreign profile, tenant isolation (user B can't see, GET, PATCH or DELETE user A's library even when proxying through user A's profile id), partial PATCH preservation, delete returns 204 then 404, ON DELETE CASCADE from profile, and the production 503 gate.tests/ready.rs+tests/openapi.rs— Canary on thelibrarytable existence + path-presence assertions on the new OpenAPI routes.Cargo.toml— bumps waveflow-core rev to04736478(#185 merge commit).Test plan
cargo check --all-targetscargo fmt --all --checkcargo clippy --all-targets -- -D warningscargo test --all(CI exercises this against a Postgres service container — local Postgres isn't always available)Refs: waveflow#185, RFC-001 §6.5.
Summary by CodeRabbit