search

LMS-Community / slimserver

Server for Squeezebox and compatible players. This server is also called Lyrion Music Server.
https://lyrion.org
Other
1.1k stars 279 forks source link

Fixes for playlist add via Release Type groups #1083

Closed darrell-k closed 1 month ago

darrell-k commented 1 month ago

This fixes some bugs I found during regression testing of my forthcoming "year fixes" PR:

Commands.pm::playlistcontrolCommand

  1. Do not restrict on grouping if no grouping param passed at all (in contrast to when it is passed but null).
  2. Handle multiple albums in album_ids (adding "all songs" to playlist was broken within Release Type groups).

Queries.pm::worksQuery

  1. Remove selection on work_id - this is purely a passthrough parameter.
  2. Add selection on year, used when this query is run from within a Years category.

Queries.pm::_getTagDataForTracks

  1. Handle workId of -1 (used when adding all works to the playlist from the Release Type category)

Releases.pm:

  1. @michaelherger previously mentioned the possibility of getting rid of $searchTags - this is now done, we now use $pt->{searchTags}, with the removal of role_id and the addition of tags specific to each group. This ensures any other input search tags are passed through.
  2. Note the removal of push @searchTags, 'role_id:' . join(',', Slim::Schema::Contributor->contributorRoleIds); - this is not needed in the grouping queries, because no role_id is the same as passing all roles.
darrell-k commented 1 month ago

Would it be worth backporting this to 8.5?

michaelherger commented 1 month ago

"Works" aren't supported in 8.5.x, are they? Wouldn't this be a whole series of changes which eventually would conflict with the ones in 9.0?

darrell-k commented 1 month ago

It would just be the multiple album I'd fix for playlists and possibly the Releases.pm refactoring.

michaelherger commented 1 month ago

If you feel like it's a manageable amount of changes, then please create a PR. And at the same time try to merge that updated 8.5.2 to 9.0 to see about the impact trying to move forward. I personally lack the oversight to imagine what the changes you suggest would be...