Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Album list experiment #1073

Draft
wants to merge 12 commits into
base: public/9.0
Choose a base branch
from
Draft

Album list experiment #1073

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
7356143
wip
darrell-k May 1, 2024
6f3083c
wip
darrell-k May 1, 2024
7a2eb54
more wip
darrell-k May 1, 2024
6092c81
remove debug code
darrell-k May 1, 2024
dc18601
remove debug code
darrell-k May 1, 2024
38c6708
wip
darrell-k May 2, 2024
990159f
wip
darrell-k May 2, 2024
8c90e44
improve handling of multiple artists with same role
darrell-k May 2, 2024
30113d8
Merge branch 'LMS-Community:public/9.0' into album-list-experiment
darrell-k May 3, 2024
c8437cb
Merge branch 'LMS-Community:public/9.0' into album-list-experiment
darrell-k May 12, 2024
bd66ba8
handle absence of ARTIST for works
darrell-k May 12, 2024
72e8d50
Merge branch 'LMS-Community:public/9.0' into album-list-experiment
darrell-k May 13, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions HTML/EN/settings/server/interface.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,13 @@

[% WRAPPER settingSection %]
[% WRAPPER settingGroup title="SETUP_SHOWARTIST" desc="SETUP_SHOWARTIST_DESC" %]
<select class="stdedit" name="pref_showArtist" id="showArtist">
<!-- <select class="stdedit" name="pref_showArtist" id="showArtist">

<option [% IF NOT prefs.pref_showArtist %]selected [% END %]value="0">[% 'DISABLED' | getstring %]</option>
<option [% IF prefs.pref_showArtist %]selected [% END %]value="1">[% 'ENABLED' | getstring %]</option>

</select>
</select> -->
<input type="text" class="stdedit" name="pref_showArtist" id="showArtist]" value="[% prefs.pref_showArtist | html %]" size="40"><br>
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will need sensible default, and migration from the current 1/0 options.

[% END %]

[% WRAPPER settingGroup title="SETUP_SHOWYEAR" desc="SETUP_SHOWYEAR_DESC" %]
Expand Down
120 changes: 51 additions & 69 deletions Slim/Control/Queries.pm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -751,68 +751,22 @@ sub albumsQuery {
if ( $tags =~ /(?:aa|SS)/ ) {
# Override $contributorSql if we're dealing with a Work: output Artist, Orchestra, Conductor in that order.
if ( defined $work ) {
my @roles = ( 'ARTIST', 'BAND', 'CONDUCTOR' );
$contributorSql = sprintf( qq{
WITH temp AS (
SELECT
CASE
WHEN contributor_track.role = 1 THEN 'ARTIST'
WHEN contributor_track.role = 3 THEN 'CONDUCTOR'
WHEN contributor_track.role = 4 THEN 'BAND'
END AS role,
GROUP_CONCAT(DISTINCT contributors.name) AS name,
GROUP_CONCAT(DISTINCT contributors.id) AS id
FROM tracks
JOIN contributor_track ON tracks.id = contributor_track.track
JOIN contributors ON contributors.id = contributor_track.contributor
WHERE tracks.album = :album AND tracks.work = :work AND contributor_track.role IN (%s)
AND ( (:grouping IS NULL AND tracks.grouping IS NULL) OR tracks.grouping = :grouping )
GROUP BY contributor_track.role
ORDER BY role
)
SELECT GROUP_CONCAT(DISTINCT name) AS name, GROUP_CONCAT(DISTINCT id) AS id FROM temp
},
join(',', map { Slim::Schema::Contributor->typeToRole($_) } @roles));
$contributorSql = qq{
SELECT contributor_track.role AS role, contributors.name AS name, contributors.id AS id
FROM tracks
JOIN contributor_track ON tracks.id = contributor_track.track
JOIN contributors ON contributors.id = contributor_track.contributor
WHERE tracks.album = :album AND tracks.work = :work
AND ( (:grouping IS NULL AND tracks.grouping IS NULL) OR tracks.grouping = :grouping )
GROUP BY contributor_track.role, contributors.name, contributors.id
};
} else {
my @roles = ( 'ARTIST', 'ALBUMARTIST' );

if ($prefs->get('useUnifiedArtistsList')) {
# 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, $_;
}
}
}

$contributorSql = sprintf( qq{
SELECT GROUP_CONCAT(contributors.name, ',') AS name, GROUP_CONCAT(contributors.id, ',') AS id
$contributorSql = qq{
SELECT contributor_album.role AS role, contributors.name AS name, contributors.id AS id
FROM contributor_album
JOIN contributors ON contributors.id = contributor_album.contributor
WHERE contributor_album.album = :album AND contributor_album.role IN (%s)
GROUP BY contributor_album.role
ORDER BY contributor_album.role DESC
}, join(',', map { Slim::Schema::Contributor->typeToRole($_) } @roles) );

# when filtering by role, put that role at the head of the list if it wasn't in there yet
if ($roleID) {
unshift @roles, map { Slim::Schema::Contributor->roleToType($_) || $_ } split(/,/, $roleID);
my %seen;
@roles = reverse grep !($seen{$_}++), reverse @roles;

$contributorSql = sprintf( qq{
SELECT GROUP_CONCAT(c.name, ',') AS name, GROUP_CONCAT(c.id, ',') AS id
FROM (
SELECT contributors.name AS name, contributors.id AS id
FROM contributor_album
JOIN contributors ON contributors.id = contributor_album.contributor
WHERE contributor_album.album = :album AND contributor_album.role IN (%s)
GROUP BY contributors.id
ORDER BY contributor_album.role DESC
)
AS c;
}, join(',', map { Slim::Schema::Contributor->typeToRole($_) } @roles) );
}
WHERE contributor_album.album = :album
};
}
}

Expand Down Expand Up @@ -862,6 +816,7 @@ sub albumsQuery {
if ( !$work ) {
$tags =~ /S/ && $request->addResultLoopIfValueDefined($loopname, $chunkCount, 'artist_id', $contributorID || $c->{'albums.contributor'});

# !!!!!!!!!!!! This is just wrong - it may replace the searchTag artist with the album artist, but leave artist_id as it is!!!!!
if ($tags =~ /a/) {
# Bug 15313, this used to use $eachitem->artists which
# contains a lot of extra logic.
Expand All @@ -878,6 +833,7 @@ sub albumsQuery {

$request->addResultLoopIfValueDefined($loopname, $chunkCount, 'artist', $c->{'contributors.name'});
}
# !!!!!!!!!!!! So which way to go????
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know, but we need to go one way or the other...

}

if ($tags =~ /s/) {
Expand All @@ -897,25 +853,51 @@ sub albumsQuery {
if ( $contributorSql && $c->{'albums.contributor'} != $vaObjId && !$c->{'albums.compilation'} ) {
$contributorSth ||= $dbh->prepare_cached($contributorSql);
$contributorSth->bind_param(":album", $c->{'albums.id'});

if ( $work ) {
$contributorSth->bind_param(":work", $work);
$contributorSth->bind_param(":grouping", $c->{'tracks.grouping'}||undef);
}
$contributorSth->execute();
my $contributorArray = $dbh->selectall_arrayref($contributorSth,{ Slice => {} });

my $contributorHash = {};
foreach (@$contributorArray) {
push @{$contributorHash->{$_->{'role'}}->{'id'}}, $_->{'id'};
push @{$contributorHash->{$_->{'role'}}->{'name'}}, $_->{'name'};
}

my @displayRoles = $work ? ('ARTIST','BAND','CONDUCTOR') : split(/[,\s]+/,$prefs->get('showArtist'));

my $contributor = $contributorSth->fetchrow_hashref;
$contributorSth->finish;
# if the user wants ARTIST, but all we have is ALBUMARTIST or TRACKARTIST, try to be helpful...
if ( grep(/ARTIST/, @displayRoles) && !exists($contributorHash->{'1'}) ) {
if ( exists($contributorHash->{'5'}) ) {
unshift(@displayRoles,'ALBUMARTIST') if !grep(/ALBUMARTIST/, @displayRoles)
} else {
unshift(@displayRoles,'TRACKARTIST') if !grep(/TRACKARTIST/, @displayRoles);
}
}
my @artists;
my @artistIds;
foreach my $role ( map { Slim::Schema::Contributor->typeToRole($_) } @displayRoles ) {
foreach my $name ( @{$contributorHash->{$role}->{'name'}} ) {
push @artists, $name unless grep(/$name/, @artists);
}
foreach my $id ( @{$contributorHash->{$role}->{'id'}} ) {
push @artistIds, $id unless grep(/$id/, @artistIds);
}
}

# XXX - what if the artist name itself contains ','?
if ( $tags =~ /aa/ && $contributor->{name} ) {
utf8::decode($contributor->{name});
$request->addResultLoopIfValueDefined($loopname, $chunkCount, 'artist', (split(/,/, $contributor->{name}))[0]) if $work;
$request->addResultLoopIfValueDefined($loopname, $chunkCount, 'artists', $contributor->{name});
if ( $tags =~ /aa/ && scalar @artists ) {
my $artists = join(',',@artists);
utf8::decode($artists);
$request->addResultLoopIfValueDefined($loopname, $chunkCount, 'artist', (split(/,/, $artists))[0]) if $work;
$request->addResultLoopIfValueDefined($loopname, $chunkCount, 'artists', $artists);
}

if ( $tags =~ /SS/ && $contributor->{id} ) {
$request->addResultLoopIfValueDefined($loopname, $chunkCount, 'artist_id', (split(/,/, $contributor->{id}))[0]) if $work;
$request->addResultLoopIfValueDefined($loopname, $chunkCount, 'artist_ids', $contributor->{id});
if ( $tags =~ /SS/ && scalar @artistIds ) {
$request->addResultLoopIfValueDefined($loopname, $chunkCount, 'artist_id', @artistIds[0]) if $work;
$request->addResultLoopIfValueDefined($loopname, $chunkCount, 'artist_ids', join(',',@artistIds));
}
}

Expand Down
11 changes: 7 additions & 4 deletions Slim/Menu/BrowseLibrary.pm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1442,6 +1442,7 @@ sub _albumsOrReleases {

sub _albums {
my ($client, $callback, $args, $pt) = @_;

my @searchTags = $pt->{'searchTags'} ? @{$pt->{'searchTags'}} : ();
my $sort = $pt->{'sort'};
my $search = $pt->{'search'};
Expand Down Expand Up @@ -1532,15 +1533,17 @@ sub _albums {
}
else {
$_->{'artists'} = [ $_->{'artist'} ];
$_->{'artist_ids'} = [ $_->{'id'} ];
$_->{'artist_ids'} = [ $_->{'artist_id'} ];
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a bug - we were setting the artist_id to the album id

}

# If an artist was not used in the selection criteria or if one was
# used but is different to that of the primary artist, then provide
# the primary artist name in name2.
if (!$artistId || $artistId != $_->{'artist_id'} || $trackArtistOnly || $_->{'work_id'}) {
$_->{'name2'} = join(', ', @{$_->{'artists'} || []}) || $_->{'artist'};
}

### Seems wrong: setting name2 just forces the artist name(s) into the list, regardless of the "showAlbums" setting (see HTML/Default/xmlbrowser.html line 417)
# if (!$artistId || $artistId != $_->{'artist_id'} || $trackArtistOnly || $_->{'work_id'}) {
# $_->{'name2'} = join(', ', @{$_->{'artists'} || []}) || $_->{'artist'};
# }

if (!$wantMeta) {
delete $_->{'artist'};
Expand Down
10 changes: 5 additions & 5 deletions strings.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6061,14 +6061,14 @@ SETUP_SHOWYEAR_DESC
SETUP_SHOWARTIST
CS Zobrazení interpreta s alby
DA Vis kunstner ved album
DE Interpreten mit Alben anzeigen
EN Show Artist with Albums
ES Mostrar artista con álbumes
DE Interpreten-Rolle mit Alben anzeigen
EN Show Artist roles with Albums
ES Mostrar roles de artista con álbumes
FI Näytä artisti levyn kanssa
FR Affichage de l'artiste avec l'album
FR Affichage de rôle de l'artiste avec l'album
HE הצגת מבצע עם אלבומים
IT Visualizza artista assieme agli album
NL Artiest bij albums tonen
NL Artiestenrollen bij albums tonen
NO Vis artist ved album
PL Pokazuj wykonawców z albumami
RU Показать исполнителя с альбомами
Expand Down