Fix albums not being able to be added to playlists during playlist creation#3385
Fix albums not being able to be added to playlists during playlist creation#3385MarvinSchenkel merged 2 commits intodevfrom
Conversation
|
I thought about this a bit. The current approach uses the knowledge, that the backend unwraps albums, but somehow this feels wrong - the allowed media_types are not album, but tracks for playlist. So my question is, should I revert the change to the playlist_create method, and then adjust the playlist_add_tracks method to only the specific media types? It would then be the client's behavior, i.e. the frontend, to be able to add albums to playlist, by receiving the tracks first (or maybe call a new helper method in the backend), before adding the tracks. Let me know what you think. |
|
My 2cents is that organizational containers should all be considered at once here - albums, genres, seasons, years, artists, search results etc... I suggest that containers that could contain playlist-compatible items should be able to pass the items to the playlist creation (aka tracks, or episodes or stations) so that as future organizational work happens (incoming genre's are a great example) they can provide their own way to give playlist handling the appropriate data... |
|
Kinda agreeing with @chrisuthe here. I think playlists should only be able to hold directly playable items, such as Tracks, Podcast episodes, Audiobooks and radio streams. I think we should postpone support for 'Containers' such as genres, albums etc. when we introduce the 'dynamic playlist' feature we have been talking about for a while now. The (still not spec'ed) idea is that we add some sort of rule-based mechanism for dynamic playlist, e.g. "30% genre Pop + 20% Album X + 50% radio mode over playlist Y". I feel this is the more logical place for these kind of MediaTypes as opposed to static playlists. Thoughts? |
|
The problem is, before my changes adding an album to a playlist was possible, as the backend silently took the individual tracks of an album and added them. The remnants of this code are still in the add_playlist_tracks. The frontend shows an add to playlist as an option for albums as well, which fails now. So my merged changes to frontend/ backend agree with both of you, they are strict to the playable type. On your last comment - I personally like to be able to add a whole album to a static playlist. The cleanest could be a new backend method, which then unwraps the album to single tracks, and adds these? |
If there is existing logic for this I would use that and postpone any improvements to when we introduce the dynamic playlist feature. So with the changes in this PR, does the server automatically add the Album tracks to the Playlist? |
|
Yes. This PR and the one linked for the frontend. Then the previous functionality is back while keeping the new one. |
MarvinSchenkel
left a comment
There was a problem hiding this comment.
In that case, lgtm, thanks @fmunkes
The frontend is not able to add albums to playlist anymore - I introduced that bug accidentally here #3216
If a playlist supports tracks, it supports albums too, as we acquire individual tracks for the playlist anyways.
Frontend PR: music-assistant/frontend#1577