feat(core): project track.album_id in postgres list + get + insert + update - #209
Conversation
…update Closes the asymmetry documented on the server's CLAUDE.md and in the desktop's 4.d.0.4 follow-ups note. Phase 4.d.0.1 added the `album` table + the nullable `track.album_id` FK; phase 4.d.0.4 added the album drill-down endpoint which projects the real value. The legacy `/tracks` collection still NULL-projected the column because the SELECT lived here in waveflow-core and the migration predates the album surface. Swap every `NULL::bigint AS album_id` for `t.album_id` (or `album_id` in the RETURNING blocks) in: - `PostgresTrackRepository::list_for_library` - `PostgresTrackRepository::get_for_library` - `PostgresTrackRepository::insert_for_library` (RETURNING) - `PostgresTrackRepository::update_for_library` (RETURNING) INSERT doesn't set `album_id` (the column list deliberately excludes it — track materialisation owns that field via the sync apply pipeline, not the CRUD path), so RETURNING projects NULL for freshly inserted rows. UPDATE doesn't touch it either, so RETURNING projects whatever's stored. Both correct. Refresh the module-level doc comment to call out the asymmetry closure — the other NULL casts (`album_title`, `artist_*`, `artwork_*`) stay until those materialisation paths land. waveflow-server picks this up via a follow-up rev bump in `Cargo.toml`; no behavior change on the desktop side (this crate is also consumed by `crates/app` via the `sqlite` feature, which doesn't go through these methods). Signed-off-by: InstaZDLL <github.105mh@8shield.net>
📝 WalkthroughWalkthroughLe dépôt PostgreSQL projette maintenant ChangesProjection correcte d'album_id dans le référentiel de pistes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~5 minutes Possibly related PRs
Suggested labels
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 |
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 (1)
src-tauri/crates/core/src/repository/postgres/track.rs (1)
281-285:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
update_for_libraryrenvoie encorealbum_idàNULLLa clause
RETURNINGde l’UPDATE projetteNULL::bigint AS album_id. Ça crée une incohérence aveclist_for_library,get_for_libraryetinsert_for_library, et peut faire perdrealbum_iddans la réponse juste après une mise à jour.💡 Correctif proposé
RETURNING id, library_id, title, - NULL::bigint AS album_id, + album_id, NULL::text AS album_title, NULL::bigint AS artist_id,🤖 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-tauri/crates/core/src/repository/postgres/track.rs` around lines 281 - 285, La clause RETURNING de update_for_library renvoie album_id et album_title comme NULL (NULL::bigint AS album_id, NULL::text AS album_title), ce qui casse la cohérence avec list_for_library, get_for_library et insert_for_library et fait perdre album_id après un UPDATE; modifie la requête dans la fonction update_for_library pour retourner les colonnes réelles album_id et album_title (pas des NULL) dans la clause RETURNING afin d'aligner le format de la réponse avec list_for_library/get_for_library/insert_for_library et préserver la valeur d'album_id après la mise à jour.
🤖 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-tauri/crates/core/src/repository/postgres/track.rs`:
- Around line 281-285: La clause RETURNING de update_for_library renvoie
album_id et album_title comme NULL (NULL::bigint AS album_id, NULL::text AS
album_title), ce qui casse la cohérence avec list_for_library, get_for_library
et insert_for_library et fait perdre album_id après un UPDATE; modifie la
requête dans la fonction update_for_library pour retourner les colonnes réelles
album_id et album_title (pas des NULL) dans la clause RETURNING afin d'aligner
le format de la réponse avec list_for_library/get_for_library/insert_for_library
et préserver la valeur d'album_id après la mise à jour.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 3359636b-d892-4fda-a866-3bed466c5238
📒 Files selected for processing (1)
src-tauri/crates/core/src/repository/postgres/track.rs
CR catch on PR #209: the previous commit projected `album_id` in list / get / insert but missed `update_for_library`'s RETURNING clause — `replace_all` matched the 19-space indent of the other RETURNING block but not the 20-space indent here, so the UPDATE path silently kept `NULL::bigint AS album_id` and a successful PATCH would have nulled the field in the response. Swap that one site too. `album_title` stays NULL across all four methods (matches the documented "remaining NULL casts stay until their materialisation path lands" — only `album_id` had a schema-side column to project against). Signed-off-by: InstaZDLL <github.105mh@8shield.net>
|
@coderabbitai resolved
|
Summary
Closes the asymmetry documented in the server's CLAUDE.md (and in the desktop `project_sprint_4_web_progress.md` follow-ups note).
Phase 4.d.0.1 (server PR #34) added the `album` table + the nullable `track.album_id` FK. Phase 4.d.0.4 (server PR #36) added the album drill-down endpoint which projects the real value. The legacy `/api/v1/profiles/{p}/libraries/{l}/tracks` collection still NULL-projected the column because the SELECT lives here in waveflow-core and predates the album surface — so the desktop / web client saw `album_id = null` on the per-library track list even when the row was materialised by the apply pipeline.
This PR swaps every `NULL::bigint AS album_id` for `t.album_id` (or `album_id` in the RETURNING blocks) in 4 sites:
INSERT doesn't set `album_id` (the column list deliberately excludes it — track→album materialisation owns that field via the sync apply pipeline, not the CRUD write path), so RETURNING projects `null` for freshly inserted rows. UPDATE doesn't touch it either, so RETURNING projects whatever's stored. Both correct.
The module-level doc comment now calls out that `album_id` is the projected-real case; the remaining NULL casts (`album_title`, `artist_`, `artwork_`) stay until their materialisation paths land server-side.
Test plan
No desktop behavior change — `crates/app` consumes `waveflow-core` via the `sqlite` feature flag which routes through the sqlite repos, not the postgres ones touched here.
Follow-up
A small PR on waveflow-server bumps the `waveflow-core` git rev in `Cargo.toml` to pick this up once this lands on `main`.
Summary by CodeRabbit