fix #294[,#294]#461
Conversation
There was a problem hiding this comment.
Pull request overview
Adds backend support for including playlists and tracks in Collections (a.k.a. pages) so they can be validated, stored, and recovered/serialized consistently across the API and homepage.
Changes:
- Extend
validate_page_items/recover_page_itemsto supportplaylistandtrackitem types. - Update Collections API schemas (defaults + examples) and wrap validation errors as HTTP 400 responses.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
src/swingmusic/lib/pagelib.py |
Adds validation + recovery logic for playlist/track items and updates album/artist lookups to store getters. |
src/swingmusic/api/collections.py |
Makes collection request fields more permissive (defaults) and returns 400 on validation errors with improved examples. |
| indexed = set(create_hash(json.dumps(item)) for item in existing) | ||
|
|
||
| for item in items: | ||
| if create_hash(json.dumps(item)) in indexed: | ||
| continue |
There was a problem hiding this comment.
In validate_page_items, the dedupe check uses create_hash(json.dumps(item)) before any normalization/coercion of the payload. For playlist IDs, hash may arrive as an int (e.g. 1) but be stored as a string (e.g. "1"), which makes the JSON hash differ and allows duplicates to slip through even though you later normalize the stored value. Consider canonicalizing type/hash (e.g. coerce playlist hash to str(int(...))) before computing the dedupe hash/index key (or dedupe based on a (type, canonical_hash) tuple).
| playlist = PlaylistTable.get_by_id(playlist_id) | ||
| if playlist is not None: | ||
| # Normalize hash for stable dedupe/remove behavior. | ||
| normalized_item = dict(item) | ||
| normalized_item["hash"] = str(playlist_id) | ||
| validated.append(normalized_item) |
There was a problem hiding this comment.
The playlist branch normalizes hash to a string before persisting. With the current JSON-hash based equality used elsewhere (e.g. remove_page_items), clients sending {type:"playlist", hash: 1} vs {..., hash: "1"} will no longer match what’s stored, so removing the item (and some dedupe scenarios) can silently fail. To avoid type-mismatch bugs, ensure playlist hashes are canonicalized consistently in all code paths (add + remove), or switch comparison/dedupe to use (type, canonical_hash) instead of hashing raw json.dumps output.
| if not playlist.has_image: | ||
| tracks = TrackStore.get_tracks_by_trackhashes(playlist.trackhashes) | ||
| playlist.images = get_first_4_images(tracks) | ||
|
|
||
| playlist.clear_lists() | ||
| item = serialize_playlist(playlist) | ||
| item["type"] = "playlist" |
There was a problem hiding this comment.
For playlist items, get_first_4_images returns a list of {image,color} dicts, and this code assigns it directly to playlist.images. On the homepage, other playlist recovery code (lib/home/recover_items.py) converts images to a list of image strings, so collections rendered via recover_page_items(..., for_homepage=True) will return a different shape for item.images than the other homepage sections. Align the response shape (either always dicts or always strings) to avoid breaking consumers that expect one format.
What kind of change does this PR introduce? (check at least one)
The PR fulfills these requirements:
fix #xxx[,#xxx], where "xxx" is the issue number)If adding a new feature, the PR's description includes:
Other information:
I added the serverside implementation of collections and added playlist and track adding to collections.