Merge pull request #1169 from darrell-k/user-defined-role-selection

Also select user-defined roles for artists and tracks lists
LMS-Community · Oct 15, 2024 · 6a4944a · 6a4944a
2 parents 691aa9c + bd8c627
commit 6a4944a
Show file tree

Hide file tree

Showing 13 changed files with 96 additions and 100 deletions.
diff --git a/Slim/Control/Queries.pm b/Slim/Control/Queries.pm
@@ -31,13 +31,13 @@ L<Slim::Control::Queries> implements most Lyrion Music Server queries and is des
 use strict;
 
 use File::Basename qw(basename);
-use Storable;
+use Storable ();
 use JSON::XS::VersionOneAndTwo;
 use Digest::MD5 qw(md5_hex);
 use List::Util qw(first);
-use MIME::Base64 qw(encode_base64 decode_base64);
+use MIME::Base64 ();
 use Scalar::Util qw(blessed);
-use URI::Escape;
+use URI::Escape ();
 use Tie::Cache::LRU::Expires;
 
 use Slim::Music::VirtualLibraries;
@@ -366,14 +366,7 @@ sub albumsQuery {
 					push @roles, 'ARTIST' if $roleID eq 'ALBUMARTIST' && !$prefs->get('useUnifiedArtistsList');
 				}
 				elsif ($prefs->get('useUnifiedArtistsList')) {
-					@roles = ( 'ARTIST', 'TRACKARTIST', 'ALBUMARTIST' );
-
-					# Loop through each pref to see if the user wants to show that contributor role. Also include user-defined roles.
-					foreach (Slim::Schema::Contributor->contributorRoles) {
-						if ( $prefs->get(lc($_) . 'InArtists') || Slim::Schema::Contributor->typeToRole($_) > 20 ) {
-							push @roles, $_;
-						}
-					}
+					@roles = Slim::Schema::Contributor->unifiedArtistsListRoles();
 				}
 				else {
 					@roles = Slim::Schema::Contributor->contributorRoles();
@@ -1043,10 +1036,14 @@ sub artistsQuery {
 			$roles = [ map { Slim::Schema::Contributor->typeToRole($_) } split(/,/, $roleID ) ];
 		}
 		elsif ($prefs->get('useUnifiedArtistsList')) {
-			$roles = Slim::Schema->artistOnlyRoles('TRACKARTIST');
+			# include user-defined roles that user wants in artist list
+			$roles = Slim::Schema->artistOnlyRoles( Slim::Schema::Contributor->getUserDefinedRolesToInclude(), 'TRACKARTIST' );
 		}
 		else {
-			$roles = [ map { Slim::Schema::Contributor->typeToRole($_) } Slim::Schema::Contributor->contributorRoles() ];
+			# include user-defined roles that user wants in artist list
+			$roles = [ map {
+				Slim::Schema::Contributor->typeToRole($_);
+			} Slim::Schema::Contributor->defaultContributorRoles(), Slim::Schema::Contributor->getUserDefinedRolesToInclude() ];
 		}
 
 		if ( defined $genreID ) {
@@ -6058,16 +6055,7 @@ sub _getTagDataForTracks {
 			push @roles, 'ARTIST' if $args->{roleId} eq 'ALBUMARTIST' && !$prefs->get('useUnifiedArtistsList');
 		}
 		elsif ($prefs->get('useUnifiedArtistsList')) {
-			# Tag 'a' returns either ARTIST or TRACKARTIST role
-			# Bug 16791: Need to include ALBUMARTIST too
-			@roles = ( 'ARTIST', 'TRACKARTIST', 'ALBUMARTIST' );
-
-			# Loop through each pref to see if the user wants to show that contributor role.
-			foreach (Slim::Schema::Contributor->contributorRoles) {
-				if ($prefs->get(lc($_) . 'InArtists')) {
-					push @roles, $_;
-				}
-			}
+			@roles = Slim::Schema::Contributor->unifiedArtistsListRoles();
 		}
 		else {
 			@roles = Slim::Schema::Contributor->contributorRoles();

diff --git a/Slim/Menu/AlbumInfo.pm b/Slim/Menu/AlbumInfo.pm
@@ -238,7 +238,7 @@ sub infoContributors {
 			allAvailableActionsDefined => 1,
 			items => {
 				command     => ['browselibrary', 'items'],
-				fixedParams => { mode => 'albums', artist_id => $id, library_id => $library_id },
+				fixedParams => { mode => 'albums', artist_id => $id, library_id => $library_id, role_id => $role},
 			},
 			play => {
 				command     => ['playlistcontrol'],
@@ -272,13 +272,6 @@ sub infoContributors {
 	if ($album->isa('Slim::Schema::Album')) {
 		my @roles = Slim::Schema::Contributor->contributorRoles;
 
-		# Loop through each pref to see if the user wants to link to that contributor role.
-		my %linkRoles = map {$_ => $prefs->get(lc($_) . 'InArtists')} @roles;
-		$linkRoles{'ARTIST'} = 1;
-		$linkRoles{'TRACKARTIST'} = 1;
-		$linkRoles{'ALBUMARTIST'} = 1;
-
-
 		# Loop through the contributor types and append
 		for my $role (@roles) {
 			for my $contributor ( $album->artistsForRoles($role) ) {
@@ -287,16 +280,7 @@ sub infoContributors {
 
 				next if $filter->{work_id} && !$album->artistPerformsOnWork($filter->{work_id}, $filter->{performance}, $contributor->id);
 
-				if ($linkRoles{$role}) {
-					$_addContributorItem->($items, $contributor, $role);
-				} else {
-					my $item = {
-						type    => 'text',
-						name    => $contributor->name,
-						label   => uc $role,
-					};
-					push @{$items}, $item;
-				}
+				$_addContributorItem->($items, $contributor, $role);
 			}
 		}
 	}

diff --git a/Slim/Menu/BrowseLibrary.pm b/Slim/Menu/BrowseLibrary.pm
@@ -1101,15 +1101,7 @@ sub _artists {
 			} @roles  if @roles;
 
 			if ( $mode && $mode eq 'artists' ) {
-				push @roles, 'ARTIST', 'TRACKARTIST', 'ALBUMARTIST';
-
-				# Loop through each pref to see if the user wants to show that contributor role.
-				foreach (Slim::Schema::Contributor->contributorRoles) {
-					if (_getPref(lc($_) . 'InArtists', $remote_library)) {
-						push @roles, $_;
-					}
-				}
-
+				push @roles, Slim::Schema::Contributor->unifiedArtistsListRoles();
 				push @ptSearchTags, 'role_id:' . join(',', @roles);
 			}
 		}

diff --git a/Slim/Menu/BrowseLibrary/Releases.pm b/Slim/Menu/BrowseLibrary/Releases.pm
@@ -52,7 +52,7 @@ sub _releases {
 	main::INFOLOG && $log->is_info && $log->info("$query ($index, $quantity): tags ->", join(', ', @searchTags));
 
 	# get the artist's albums list to create releases sub-items etc.
-	my $request = Slim::Control::Request->new( undef, [ $query, 0, MAX_ALBUMS, @searchTags, 'role_id:'. $menuRoles ? $menuRoles : join(',',Slim::Schema::Contributor->contributorRoles) ] );
+	my $request = Slim::Control::Request->new( undef, [ $query, 0, MAX_ALBUMS, @searchTags, 'role_id:'. ($menuRoles ? $menuRoles : join(',',Slim::Schema::Contributor->contributorRoles)) ] );
 	$request->execute();
 
 	$log->error($request->getStatusText()) if $request->isStatusError();

diff --git a/Slim/Menu/TrackInfo.pm b/Slim/Menu/TrackInfo.pm
@@ -361,7 +361,7 @@ sub infoContributors {
 			allAvailableActionsDefined => 1,
 			items => {
 				command     => ['browselibrary', 'items'],
-				fixedParams => { mode => 'albums', artist_id => $id, library_id => $library_id },
+				fixedParams => { mode => 'albums', artist_id => $id, library_id => $library_id, role_id => $role },
 			},
 			play => {
 				command     => ['playlistcontrol'],
@@ -396,25 +396,10 @@ sub infoContributors {
 	if ( $track->isa('Slim::Schema::Track') ) {
 		my @roles = Slim::Schema::Contributor->contributorRoles;
 
-		# Loop through each pref to see if the user wants to link to that contributor role.
-		my %linkRoles = map {$_ => $prefs->get(lc($_) . 'InArtists')} @roles;
-		$linkRoles{'ARTIST'} = 1;
-		$linkRoles{'TRACKARTIST'} = 1;
-		$linkRoles{'ALBUMARTIST'} = 1;
-
 		# Loop through the contributor types and append
 		for my $role ( @roles ) {
 			for my $contributor ( $track->contributorsOfType($role) ) {
-				if ($linkRoles{$role}) {
-					$_addContributorItem->($items, $contributor, $role);
-				} else {
-					my $item = {
-						type    => 'text',
-						name    => $contributor->name,
-						label   => uc $role,
-					};
-					push @{$items}, $item;
-				}
+				$_addContributorItem->($items, $contributor, $role);
 			}
 		}
 	}

diff --git a/.../Plugin/ExtendedBrowseModes/HTML/EN/plugins/ExtendedBrowseModes/settings/browsemodes.html b/.../Plugin/ExtendedBrowseModes/HTML/EN/plugins/ExtendedBrowseModes/settings/browsemodes.html
@@ -74,16 +74,19 @@
 			<tr>
 				<th>[% "PLUGIN_EXTENDED_BROWSEMODES_ROLE" | string %]</th>
 				<th>[% "PLUGIN_EXTENDED_BROWSEMODES_ROLE_TEXT" | string %]</th>
+				<th>[% "PLUGIN_EXTENDED_BROWSEMODES_ROLE_INCLUDE" | string %]</th>
 			</tr>
 			[% FOREACH tag = customTags.keys.sort %]
 				<tr>
 					<td><input type="text" name="[% tag | html %]_tag" value="[% tag | html %]" class="stdedit"/></td>
 					<td><input type="text" name="[% tag | html %]_name" value="[% customTags.$tag.name | html %]" class="stdedit" style="width: 200px;"/></td>
+					<td style="text-align:center;"><input type="checkbox" [% IF customTags.$tag.include %]checked="1" [% END %] name="[% tag | html %]_include" value="1" class="stdedit" /></td>
 				</tr>
 			[% END %]
 			<tr>
 				<td><input type="text" name="new_tag" value="" class="stdedit"/></td>
 				<td><input type="text" name="new_name" value="" class="stdedit" style="width: 200px;"/></td>
+				<td style="text-align:center;"><input type="checkbox" name="new_include" class="stdedit" value="1" checked="1"/></td>
 			</tr>
 		</table>
 	[% END; END %]

diff --git a/Slim/Plugin/ExtendedBrowseModes/Settings.pm b/Slim/Plugin/ExtendedBrowseModes/Settings.pm
@@ -8,11 +8,10 @@ package Slim::Plugin::ExtendedBrowseModes::Settings;
 
 use strict;
 use base qw(Slim::Web::Settings);
-use Storable;
+use Storable ();
 
 use Slim::Music::VirtualLibraries;
 use Slim::Plugin::ExtendedBrowseModes::Plugin;
-use Slim::Utils::Log;
 use Slim::Utils::Misc;
 use Slim::Utils::Strings qw(string);
 use Slim::Utils::Prefs;
@@ -32,7 +31,6 @@ use constant AUDIOBOOKS_MENUS => [{
 	weight  => 15,
 	enabled => 0,
 }];
-
 my $prefs = preferences('plugin.extendedbrowsemodes');
 my $serverPrefs = preferences('server');
 
@@ -55,8 +53,9 @@ sub handler {
 	if ($params->{'saveSettings'} && !$class->needsClient) {
 		# custom role handling
 		my $currentRoles = $serverPrefs->get('userDefinedRoles');
+		my $customRoleId = Slim::Schema::Contributor->getMinCustomRoleId();
+
 		my $customTags = {};
-		my $id = 21;
 		my $changed = 0;
 
 		foreach my $pref (keys %{$params}) {
@@ -67,23 +66,25 @@ sub handler {
 				if ( $tag ) {
 					$customTags->{$tag} = {
 						name => $params->{$key . '_name'} || $tag,
-						id => $id,
+						id => $currentRoles->{$tag} ? $currentRoles->{$tag}->{id} : $customRoleId++,
+						include => $params->{$key . '_include'},
 					};
-					if ( !$currentRoles->{$tag} || $currentRoles->{$tag}->{name} ne $customTags->{$tag}->{name} ) {
+
+					if ( !$currentRoles->{$tag} || $currentRoles->{$tag}->{name} ne $customTags->{$tag}->{name}
+						|| $currentRoles->{$tag}->{include} ne $customTags->{$tag}->{include}
+					) {
 						Slim::Utils::Strings::storeExtraStrings([{
 							strings => { EN => $customTags->{$tag}->{name}},
 							token   => $tag,
-						}]) if !Slim::Utils::Strings::stringExists($tag);
+						}]);
 						$changed = 1;
 					}
-					$id++;
 				}
 			}
 		}
 
-		foreach my $old (keys %{$currentRoles}) {
-			$changed = 1 if !$customTags->{$old};
-		}
+		# set changed flag if we removed an item from the list
+		$changed ||= grep { !$customTags->{$_} } keys %$currentRoles;
 
 		if ( $changed ) {
 			$serverPrefs->set('userDefinedRoles', $customTags);

diff --git a/Slim/Plugin/ExtendedBrowseModes/strings.txt b/Slim/Plugin/ExtendedBrowseModes/strings.txt
@@ -340,10 +340,18 @@ PLUGIN_EXTENDED_BROWSEMODES_ROLE
 	NL	Rol tag
 
 PLUGIN_EXTENDED_BROWSEMODES_ROLE_TEXT
-	EN	Display Text
+	EN	Display text
 	DE	Text anzeigen
 	FR	Afficher le texte
 	ES	Mostrar texto
 	HU	Szöveg megjelenítése
 	NL	Tekst weergeven
 
+PLUGIN_EXTENDED_BROWSEMODES_ROLE_INCLUDE
+	EN	Include with artist lists
+	DE	In Künstlerlisten aufnehmen
+	FR	Inclure avec les listes d'artistes
+	ES	Incluir con listas de artistas
+	HU	Tartalmazza az előadók listáját
+	NL	Opnemen bij artiestenlijsten
+
diff --git a/Slim/Schema/Contributor.pm b/Slim/Schema/Contributor.pm
@@ -4,20 +4,24 @@ package Slim::Schema::Contributor;
 use strict;
 use base 'Slim::Schema::DBI';
 
-use Scalar::Util qw(blessed);
+use List::Util qw(max);
 
 use Slim::Schema::ResultSet::Contributor;
 
 use Slim::Utils::Log;
 use Slim::Utils::Misc;
 use Slim::Utils::Prefs;
 
+use constant MIN_CUSTOM_ROLE_ID => 21;
+
 my %contributorToRoleMap;
 my @contributorRoles;
 my @contributorRoleIds;
 my $totalContributorRoles;
 my %roleToContributorMap;
 
+my $prefs = preferences('server');
+
 initializeRoles();
 
 {
@@ -66,11 +70,9 @@ sub initializeRoles {
 		'ALBUMARTIST' => 5,
 		'TRACKARTIST' => 6,
 	);
-	my $prefs = preferences('server');
-	if ( my $userDefinedRoles = $prefs->get('userDefinedRoles') ) {
-		while ( my($k, $v) = each (%$userDefinedRoles) ) {
-			$contributorToRoleMap{$k} = $v->{id};
-		}
+
+	while ( my ($k, $v) = each %{ $prefs->get('userDefinedRoles') } ) {
+		$contributorToRoleMap{$k} ||= $v->{id};
 	}
 
 	@contributorRoles = sort keys %contributorToRoleMap;
@@ -83,6 +85,20 @@ sub contributorRoles {
 	return @contributorRoles;
 }
 
+sub defaultContributorRoles {
+	return grep { __PACKAGE__->typeToRole($_) < MIN_CUSTOM_ROLE_ID } contributorRoles();
+}
+
+sub unifiedArtistsListRoles {
+	my @roles = ( 'ARTIST', 'TRACKARTIST', 'ALBUMARTIST' );
+
+	# Loop through each pref to see if the user wants to show that contributor role. Also include user-defined roles.
+	push @roles, grep { $prefs->get(lc($_) . 'InArtists') } contributorRoles();
+	push @roles, getUserDefinedRolesToInclude();
+
+	return grep { $_ } @roles;
+}
+
 sub contributorRoleIds {
 	return @contributorRoleIds;
 }
@@ -103,6 +119,16 @@ sub roleToType {
 	return $roleToContributorMap{$_[1]};
 }
 
+sub getUserDefinedRolesToInclude {
+	# de-reference the pref so we don't accidentally change it below
+	my %udr = %{$prefs->get('userDefinedRoles')};
+	return grep { $udr{$_}->{include} } contributorRoles();
+}
+
+sub getMinCustomRoleId {
+	return max(MIN_CUSTOM_ROLE_ID, max(contributorRoleIds()) + 1);
+}
+
 sub extIds {
 	my ($self) = @_;
 	return [ split(',', $self->extid || '') ];

diff --git a/Slim/Schema/ResultSet/Contributor.pm b/Slim/Schema/ResultSet/Contributor.pm
@@ -45,7 +45,9 @@ sub countTotal {
 
 	my $cond  = {};
 	my @joins = ();
-	my $roles = $prefs->get('useUnifiedArtistsList') ? Slim::Schema->artistOnlyRoles('TRACKARTIST') : [ Slim::Schema::Contributor->contributorRoleIds ];
+	my $roles = $prefs->get('useUnifiedArtistsList')
+		? Slim::Schema->artistOnlyRoles(Slim::Schema::Contributor::getUserDefinedRolesToInclude(), 'TRACKARTIST')
+		: [ Slim::Schema::Contributor->contributorRoleIds ];
 
 	# The user may not want to include all the composers / conductors
 	if ($roles) {