Another albumsQuery contributor bug

[darrell-k]

I've just seen this happen while testing some Material Skin changes.

slimserver/Slim/Control/Queries.pm

Lines 846 to 863 in a1719ae

	$tags =~ /S/ && $request->addResultLoopIfValueDefined($loopname, $chunkCount, 'artist_id', $contributorID || $c->{'albums.contributor'});
	if ($tags =~ /a/) {
	# Bug 15313, this used to use $eachitem->artists which
	# contains a lot of extra logic.
	# Bug 17542: If the album artist is different from the current track's artist,
	# use the album artist instead of the track artist (if available)
	if ($contributorID && $c->{'albums.contributor'} && $contributorID != $c->{'albums.contributor'}) {
	$contributorNameSth ||= $dbh->prepare_cached('SELECT name FROM contributors WHERE id = ?');
	my ($name) = @{ $dbh->selectcol_arrayref($contributorNameSth, undef, $c->{'albums.contributor'}) };
	$c->{'contributors.name'} = $name if $name;
	}
	utf8::decode( $c->{'contributors.name'} ) if exists $c->{'contributors.name'};
	$request->addResultLoopIfValueDefined($loopname, $chunkCount, 'artist', $c->{'contributors.name'});
	}

Line 846 sets artist_id to the input parameter if it exists, rather than albums.contributor column.

Then at 854-862, if the input parm and the album.contributor column are different, artist is set to the name of album.contributor, so artist no longer matches artist_id.

So one or the other is wrong, but which way to go? Not sure where these are used, rather than artist_ids and artists, which we've just been working on, but still...

[michaelherger]

We should evaluate what contributor ID to report before the /S/ check, then use the same in both places. The name was evaluated in response to a bug report (which unfortunately is no longer available). So... I'd think the visual part was stronger than the link. We should go with the logic used there?

[darrell-k]

fixed by #1206