LMS-Community/slimserver

noRoleFilter should mean noRoleFilter!

Opened this issue · 2 comments

However, when using the combined list, even if noRoleFilter is true (so no role_id is passed), albumsQuery still restricts to active combined list roles:

if ($roleID) {
@roles = split /,/, $roleID;
push @roles, 'ARTIST' if $roleID eq 'ALBUMARTIST' && !$prefs->get('useUnifiedArtistsList');
}
elsif ($prefs->get('useUnifiedArtistsList')) {
@roles = Slim::Schema::Contributor->activeContributorRoles(1);
}
else {
@roles = Slim::Schema::Contributor->contributorRoles();
}

Of course the PR for this would take about 5 minutes, but is it controversial? Possibly!

From looking at how noRoleFilter et al. is implemented it seems to completely rely on the browse menu code. And Queries then would assume "standard roles", rather than "no filter". I believe that's still the old assumption that there's basically one kind of artist, "the" artist. And what that artist is we did make some assumptions. The pref therefore has got a slightly different meaning with the arrival of more roles. Now the question is: should the pref be redefined, or the behaviour adjusted?

The problem would be that all callers of the albums query would have to pass the roles they want. That would be a breaking change. We should probably introduce a new param to tell when a caller really doesn't want any role filter at all.

Good point, and stuff to think about. At least it's now captured here in the issues.