[Slim-Checkins] r8784 - in /trunk/server: ./ Slim/ Slim/Control/ Slim/Schema/ Slim/Schema/ResultSet/ Slim/Web/ Slim/Web/Pages/

dsully at svn.slimdevices.com dsully at svn.slimdevices.com
Thu Aug 3 17:44:33 PDT 2006


Author: dsully
Date: Thu Aug  3 17:44:29 2006
New Revision: 8784

URL: http://svn.slimdevices.com?rev=8784&view=rev
Log:
Bug: 3824 & 3835
Description: Various Artists rework:

* Initial patch from Fred Thomas, with updates from Dan Sully

* Fix grouping of Various Artists on and off.

* Remove 'contributor' member from Albums.

* Join on tracks to find the URL basename for determining when to create a new
  album. Should result in far fewer (if any) duplicate albums, if Albums are
  arranged in a standard Foo/Album/Tracks directory scheme.

* Don't store the "Various Artists" object id for any albums, use the
  compilation flag from the albums table instead which allows for a dynamic approach.

* Don't constrain on contributor.id when dealing with a VA album.

* Rework the "attrs" handling for Pages::BrowseDB to use a hash for easier comparison.

* Logic to make sure that the pagebar & breadcrumb list is correct in all VA cases.

* Fixes for when we generate a 'TRACKARTIST' role - allow it to be searched,
  displayed and linked to in the UI, but don't show those artists in the Artist list.

Modified:
    trunk/server/Changelog6.html
    trunk/server/Slim/Control/Commands.pm
    trunk/server/Slim/Schema.pm
    trunk/server/Slim/Schema/Album.pm
    trunk/server/Slim/Schema/Contributor.pm
    trunk/server/Slim/Schema/ResultSet/Album.pm
    trunk/server/Slim/Schema/ResultSet/Contributor.pm
    trunk/server/Slim/Schema/Track.pm
    trunk/server/Slim/Web/Pages.pm
    trunk/server/Slim/Web/Pages/BrowseDB.pm
    trunk/server/Slim/Web/Pages/LiveSearch.pm
    trunk/server/Slim/Web/Pages/Search.pm

Modified: trunk/server/Changelog6.html
URL: http://svn.slimdevices.com/trunk/server/Changelog6.html?rev=8784&r1=8783&r2=8784&view=diff
==============================================================================
--- trunk/server/Changelog6.html (original)
+++ trunk/server/Changelog6.html Thu Aug  3 17:44:29 2006
@@ -449,10 +449,12 @@
 		<li><a href="http://bugs.slimdevices.com/show_bug.cgi?id=3815">#3815</a> - 'Various Artists' displayed instead of ALBUMARTIST when COMPILATION=1</li>
 		<li><a href="http://bugs.slimdevices.com/show_bug.cgi?id=3816">#3816</a> - basic search crasher</li>
 		<li><a href="http://bugs.slimdevices.com/show_bug.cgi?id=3820">#3820</a> - DBD::mysql::st execute failed: Table 'slimserver.years' doesn't exist</li>
+		<li><a href="http://bugs.slimdevices.com/show_bug.cgi?id=3824">#3824</a> - TRACKARTIST (and ALBUMARTIST) problems</li>
 		<li><a href="http://bugs.slimdevices.com/show_bug.cgi?id=3831">#3831</a> - help-&gt;faq open in new window</li>
 		<li><a href="http://bugs.slimdevices.com/show_bug.cgi?id=3832">#3832</a> - Play All Songs on Player search</li>
 		<li><a href="http://bugs.slimdevices.com/show_bug.cgi?id=3833">#3833</a> - Clean install crashes with undefined value in Import.pm</li>
 		<li><a href="http://bugs.slimdevices.com/show_bug.cgi?id=3834">#3834</a> - build-perl-modules.pl assumes that which exits with an error status</li>
+		<li><a href="http://bugs.slimdevices.com/show_bug.cgi?id=3835">#3835</a> - Cannot not group Various Artists</li>
 		<li><a href="http://bugs.slimdevices.com/show_bug.cgi?id=3837">#3837</a> - Digital Input plugin should only appear on transporters</li>
 		<li><a href="http://bugs.slimdevices.com/show_bug.cgi?id=3842">#3842</a> - Playing a magic favorite has some problems.</li>
 		<li><a href="http://bugs.slimdevices.com/show_bug.cgi?id=3843">#3843</a> - Undefined subroutine &Slim::Networking::Async::Socket::UDP::msg</li>

Modified: trunk/server/Slim/Control/Commands.pm
URL: http://svn.slimdevices.com/trunk/server/Slim/Control/Commands.pm?rev=8784&r1=8783&r2=8784&view=diff
==============================================================================
--- trunk/server/Slim/Control/Commands.pm (original)
+++ trunk/server/Slim/Control/Commands.pm Thu Aug  3 17:44:29 2006
@@ -2159,6 +2159,12 @@
 			delete $find{'contributor.id'};
 		}
 
+		if ($find{'album.id'} && $find{'contributor.id'} && 
+			$find{'contributor.id'} == Slim::Schema->variousArtistsObject->id) {
+
+			delete $find{'contributor.id'};
+		}
+
 		if ($find{'playlist.id'}) {
 		
 			delete $find{'playlist.id'};

Modified: trunk/server/Slim/Schema.pm
URL: http://svn.slimdevices.com/trunk/server/Slim/Schema.pm?rev=8784&r1=8783&r2=8784&view=diff
==============================================================================
--- trunk/server/Slim/Schema.pm (original)
+++ trunk/server/Slim/Schema.pm Thu Aug  3 17:44:29 2006
@@ -743,14 +743,11 @@
 	my $find  = shift;
 
 	my %attr = ( 'group_by' => 'me.id' );
-	my @join = ( 'contributorAlbums' );
+	my @join = ();
 
 	# We always want to search for compilation
 	$find->{'me.compilation'} = 1;
 
-	# And the VA object.
-	$find->{'contributorAlbums.contributor'} = $class->variousArtistsObject->id;
-
 	if (exists $find->{'genre.id'}) {
 
 		$find->{'genreTracks.genre'} = delete $find->{'genre.id'};
@@ -759,11 +756,6 @@
 	} elsif (exists $find->{'genre.name'}) {
 
 		push @join, { 'tracks' => { 'genreTracks' => 'genre' } };
-	}
-
-	if (my $roles = $class->artistOnlyRoles) {
-
-		$find->{'contributorAlbums.role'} = { 'in' => $roles };
 	}
 
 	$attr{'join'} = \@join;
@@ -794,7 +786,7 @@
 # to identify albums which are compilations / various artist albums - by
 # virtue of having more than one artist.
 sub mergeVariousArtistsAlbums {
-        my $self = shift;
+	my $self = shift;
 
 	my $vaObjId = $self->variousArtistsObject->id;
 	my $role    = Slim::Schema::Contributor->typeToRole('ARTIST');
@@ -844,7 +836,8 @@
 			}
 		}
 
-		# Bug 2418 - If the tracks have a hardcoded artist of 'Various Artists' - mark the album as a compilation.
+		# Bug 2418 was fixed here -- but we don't do it anymore
+		# (hardcoded artist of 'Various Artists' making the album a compilation)
 		if (scalar values %trackArtists > 1) {
 
 			$markAsCompilation = 1;
@@ -864,20 +857,11 @@
 
 		if ($markAsCompilation) {
 
-			$::d_import && !$progress && msgf("Import: Marking album: [%s] as Various Artists.\n", $albumObj->title);
+			$::d_import && !$progress && msgf("Import: Marking album: [%s] as a compilation.\n", $albumObj->title);
 			$::d_info && $_dump_postprocess_logic && msg("--- Album is a VA\n");
 
 			$albumObj->compilation(1);
-			$albumObj->contributor($vaObjId);
 			$albumObj->update;
-
-			# And update the contributor_albums table.
-			$self->resultset('ContributorAlbum')->find_or_create({
-				'album'       => $albumObj->id,
-				'contributor' => $vaObjId,
-				'role'        => $role,
-			});
-
 		}
 
 		$progress->update if $progress;
@@ -1071,29 +1055,31 @@
 	return $attributesHash;
 }
 
-# The user may want to constrain their browse view by either or both of
-# 'composer' and 'track artists'.
+# The user may want to constrain their browse view by either or both of 'composer' and 'track artists'.
 sub artistOnlyRoles {
 	my $self  = shift;
-	my $find  = shift || {};
+	my @add   = shift || ();
 
 	my %roles = (
 		'ARTIST'      => 1,
 		'ALBUMARTIST' => 1,
 	);
 
+	# If the user has requested explict roles to be added, do so.
+	for my $role (@add) {
+
+		$roles{$role} = 1;
+	}
+
+	# And if the user has asked for ALL, give them it.
+	if ($roles{'ALL'}) {
+		return undef;
+	}
+
 	# Loop through each pref to see if the user wants to show that contributor role.
 	for my $role (Slim::Schema::Contributor->contributorRoles) {
 
-		my $pref = sprintf('%sInArtists', lc($role));
-
-		if (Slim::Utils::Prefs::get($pref)) {
-
-			$roles{$role} = 1;
-		}
-
-		# If the user has requested that a specific role be added.
-		if (exists $find->{'contributor.role'} && $find->{'contributor.role'} eq $role) {
+		if (Slim::Utils::Prefs::get(sprintf('%sInArtists', lc($role)))) {
 
 			$roles{$role} = 1;
 		}
@@ -1664,14 +1650,14 @@
 			# the server behaviour changes depending on the track order!
 			# Maybe we need a preference?
 			my $search = {
-				'title' => $album,
+				'me.title' => $album,
 				#'year'  => $track->year,
 			};
 
 			# Add disc to the search criteria if needed
 			if ($checkDisc) {
 
-				$search->{'disc'} = $disc;
+				$search->{'me.disc'} = $disc;
 
 			} elsif ($discc && $discc > 1) {
 
@@ -1679,13 +1665,13 @@
 				# groupdiscs mode, check discc if it exists,
 				# in the case where there are multiple albums
 				# of the same name by the same artist. bug3254
-				$search->{'discc'} = $discc;
+				$search->{'me.discc'} = $discc;
 			}
 
 			# Bug 3662 - Only check for undefined/null values if the
 			# values are undefined.
-			$search->{'disc'}  = undef if !defined $disc; 
-			$search->{'discc'} = undef if !defined $disc && !defined $discc;
+			$search->{'me.disc'}  = undef if !defined $disc; 
+			$search->{'me.discc'} = undef if !defined $disc && !defined $discc;
 
 			# If we have a compilation bit set - use that instead
 			# of trying to match on the artist. Having the
@@ -1694,16 +1680,22 @@
 			if (defined $isCompilation) {
 
 				# in the database this is 0 or 1
-				$search->{'compilation'} = $isCompilation;
-
-			} else {
-
-				if (blessed($contributor)) {
-					$search->{'contributor'} = $contributor->id;
-				}
+				$search->{'me.compilation'} = $isCompilation;
 			}
 
-			$albumObj = $self->single('Album', $search);
+			my $attr = {
+				'group_by' => 'me.id',
+			};
+
+			# Join on tracks with the same basename to determine a unique album.
+			if (!defined $disc || !defined $discc) {
+
+				$search->{'tracks.url'} = { 'like' => "$basename%" };
+
+				$attr->{'join'} = 'tracks';
+			}
+
+			$albumObj = $self->search('Album', $search, $attr)->single;
 
 			if ($::d_info && $_dump_postprocess_logic) {
 
@@ -1728,8 +1720,7 @@
 			# album object is valid.
 			if ($albumObj && $checkDisc && !defined $isCompilation) {
 
-				my %tracks     = map { $_->tracknum, $_ } $albumObj->tracks;
-				my $matchTrack = $tracks{ $track->tracknum };
+				my $matchTrack = $albumObj->tracks({ 'tracknum' => $track->tracknum });
 
 				if (defined $matchTrack && dirname($matchTrack->url) ne dirname($track->url)) {
 
@@ -1760,17 +1751,6 @@
 		my $sortable_title = Slim::Utils::Text::ignoreCaseArticles($attributes->{'ALBUMSORT'} || $album);
 
 		my %set = ();
-
-		# Add an album artist if it exists.
-		# But set the album artist to various if there is no album artist.
-		if ($isCompilation && !$contributors->{'ALBUMARTIST'}) {
-
-			$set{'contributor'} = $self->variousArtistsObject->id;
-
-		} elsif (blessed($contributor)) {
-
-			$set{'contributor'} = $contributor->id;
-		}
 
 		# Always normalize the sort, as ALBUMSORT could come from a TSOA tag.
 		$set{'titlesort'}   = $sortable_title;
@@ -1873,14 +1853,6 @@
 		while (my ($role, $contributorList) = each %{$contributors}) {
 
 			for my $contributorObj (@{$contributorList}) {
-
-				# XXXX - is this correct? VA albums need to be
-				# added to the contributor_album table somehow.
-				#
-				# Bug 3815 - Only set to VA if there isn't an explict ALBUMARTIST.
-				if ($isCompilation && !exists $contributors->{'ALBUMARTIST'}) {
-					$contributorObj = $self->variousArtistsObject;
-				}
 
 				$self->resultset('ContributorAlbum')->find_or_create({
 					'album'       => $albumObj->id,

Modified: trunk/server/Slim/Schema/Album.pm
URL: http://svn.slimdevices.com/trunk/server/Slim/Schema/Album.pm?rev=8784&r1=8783&r2=8784&view=diff
==============================================================================
--- trunk/server/Slim/Schema/Album.pm (original)
+++ trunk/server/Slim/Schema/Album.pm Thu Aug  3 17:44:29 2006
@@ -15,7 +15,6 @@
 	$class->add_columns(qw(
 		id
 		titlesort
-		contributor
 		compilation
 		year
 		artwork
@@ -31,14 +30,8 @@
 	$class->set_primary_key('id');
 	$class->add_unique_constraint('titlesearch' => [qw/id titlesearch/]);
 
-	$class->belongs_to('contributor' => 'Slim::Schema::Contributor');
-
-	$class->has_many(
-		'tracks' => 'Slim::Schema::Track', undef,
-		{ 'order_by' => [qw(disc tracknum titlesort)] }
-	);
-
-	$class->has_many('contributorAlbums' => 'Slim::Schema::ContributorAlbum');
+	$class->has_many('tracks'            => 'Slim::Schema::Track'            => 'album');
+	$class->has_many('contributorAlbums' => 'Slim::Schema::ContributorAlbum' => 'album');
 
 	if ($] > 5.007) {
 		$class->utf8_columns(qw/title titlesort/);
@@ -120,7 +113,7 @@
 		# contributors in the album view.
 		# if ($form->{'hierarchy'} ne 'contributor,album,track') {
 
-			if (my $contributor = $self->contributor) {
+			if (my $contributor = $self->contributors->first) {
 
 				$form->{'artist'}        = $contributor;
 				#$form->{'includeArtist'} = defined $findCriteria->{'artist'} ? 0 : 1;
@@ -175,9 +168,9 @@
 
 	} elsif (scalar @artists == 0) {
 
-		$::d_info && msgf("\t\%artists == 0 && \$self->contributor - returning: [%s]\n", $self->contributor);
-
-		@artists = $self->contributor;
+		$::d_info && msgf("\t\%artists == 0 && \$self->contributor - returning: [%s]\n", $self->contributors);
+
+		@artists = $self->contributors;
 	}
 
 	return @artists;

Modified: trunk/server/Slim/Schema/Contributor.pm
URL: http://svn.slimdevices.com/trunk/server/Slim/Schema/Contributor.pm?rev=8784&r1=8783&r2=8784&view=diff
==============================================================================
--- trunk/server/Slim/Schema/Contributor.pm (original)
+++ trunk/server/Slim/Schema/Contributor.pm Thu Aug  3 17:44:29 2006
@@ -69,7 +69,7 @@
 	my $class = shift;
 	my $type  = shift;
 
-	return $contributorToRoleMap{$type};
+	return $contributorToRoleMap{$type} || $type;
 }
 
 sub displayAsHTML {

Modified: trunk/server/Slim/Schema/ResultSet/Album.pm
URL: http://svn.slimdevices.com/trunk/server/Slim/Schema/ResultSet/Album.pm?rev=8784&r1=8783&r2=8784&view=diff
==============================================================================
--- trunk/server/Slim/Schema/ResultSet/Album.pm (original)
+++ trunk/server/Slim/Schema/ResultSet/Album.pm Thu Aug  3 17:44:29 2006
@@ -76,15 +76,6 @@
 	my $sort = shift;
 
 	my @join = ();
-	my $roles = Slim::Schema->artistOnlyRoles;
-
-	# The user may not want to include all the composers / conductors
-	if ($roles) {
-
-		$cond->{'contributorAlbums.role'} = { 'in' => $roles };
-
-		push @join, 'contributorAlbums';
-	}
 
 	# This sort/join logic is here to handle the 'Sort Browse Artwork'
 	# feature - which applies to albums, as artwork is just a view on the
@@ -103,12 +94,7 @@
 
 		if ($sort =~ /contributor/) {
 
-			# Only join contributorAlbums once.
-			if (!$roles) {
-				push @join, { 'contributorAlbums' => 'contributor' };
-			} else {
-				push @join, 'contributor';
-			}
+			push @join, { 'contributorAlbums' => 'contributor' };
 		}
 
 		if ($sort =~ /genre/) {

Modified: trunk/server/Slim/Schema/ResultSet/Contributor.pm
URL: http://svn.slimdevices.com/trunk/server/Slim/Schema/ResultSet/Contributor.pm?rev=8784&r1=8783&r2=8784&view=diff
==============================================================================
--- trunk/server/Slim/Schema/ResultSet/Contributor.pm (original)
+++ trunk/server/Slim/Schema/ResultSet/Contributor.pm Thu Aug  3 17:44:29 2006
@@ -53,7 +53,7 @@
 	};
 
 	# Bug: 2479 - Don't include roles if the user has them unchecked.
-	if (my $roles = Slim::Schema->artistOnlyRoles) {
+	if (my $roles = Slim::Schema->artistOnlyRoles('TRACKARTIST')) {
 
 		$cond->{'contributorAlbums.role'} = { 'in' => $roles };
 		push @joins, 'contributorAlbums';
@@ -108,16 +108,23 @@
 		'order_by' => "concat('0', album.titlesort), album.disc",
 	};
 
-	if (my $roles = Slim::Schema->artistOnlyRoles) {
+	if (my $roles = Slim::Schema->artistOnlyRoles($find->{'contributor.role'})) {
 
 		$cond->{'contributorAlbums.role'} = { 'in' => $roles };
 	}
 
 	# Bug: 2192 - Don't filter out compilation
 	# albums at the artist level - we want to see all of them for an artist.
-	if ($cond->{'me.id'} && $find->{'album.compilation'} && $find->{'album.compilation'} != 1) {
+	my $albumCond = {};
 
-		# $cond->{'album.compilation'} = $find->{'album.compilation'};
+	if (defined $find->{'album.compilation'}) {
+
+		if ($cond->{'me.id'} && $cond->{'me.id'} == Slim::Schema->variousArtistsObject->id) {
+
+			delete $cond->{'me.id'};
+		}
+
+		$albumCond->{'album.compilation'} = $find->{'album.compilation'};
 	}
 
 	$rs = $rs->search_related('contributorAlbums', $rs->fixupFindKeys($cond));
@@ -125,16 +132,11 @@
 	# Constrain on the genre if it exists.
 	if (my $genre = $find->{'genre.id'}) {
 
+		$albumCond->{'genreTracks.genre'} = $genre;
 		$attr->{'join'} = { 'tracks' => 'genreTracks' };
-
-		$rs = $rs->search_related('album', { 'genreTracks.genre' => $genre }, $attr);
-
-	} else {
-
-		$rs = $rs->search_related('album', {}, $attr);
 	}
 
-	return $rs
+	return $rs->search_related('album', $albumCond, $attr);
 }
 
 1;

Modified: trunk/server/Slim/Schema/Track.pm
URL: http://svn.slimdevices.com/trunk/server/Slim/Schema/Track.pm?rev=8784&r1=8783&r2=8784&view=diff
==============================================================================
--- trunk/server/Slim/Schema/Track.pm (original)
+++ trunk/server/Slim/Schema/Track.pm Thu Aug  3 17:44:29 2006
@@ -88,29 +88,39 @@
 sub artist {
 	my $self = shift;
 
-	return $self->contributorsOfType('ARTIST')->single;
+	# Bug 3824 - check for both types, in the case that an ALBUMARTIST was set.
+	return $self->contributorsOfType('ARTIST')->single ||
+	       $self->contributorsOfType('TRACKARTIST')->single;
 }
 
 sub artists {
 	my $self = shift;
 
+	# XXXX - not sure who the callers are here, and if this should include
+	# TRACKARTISTS or not.
 	return $self->contributorsOfType('ARTIST')->all;
 }
 
 sub artistsWithAttributes {
 	my $self = shift;
 
-	my @artists = ();
-
-	for my $artist ($self->artists) {
-
-		push @artists, {
-			'artist'     => $artist,
-			'attributes' => join('=', 'contributor.id', $artist->id),
-		};
-	}
-
-	return \@artists;
+	my @objs = ();
+
+	for my $type (qw(ARTIST TRACKARTIST)) {
+
+		for my $contributor ($self->contributorsOfType($type)->all) {
+
+			push @objs, {
+				'artist'     => $contributor,
+				'attributes' => join('&', 
+					join('=', 'contributor.id', $contributor->id),
+					join('=', 'contributor.role', $type),
+				),
+			};
+		}
+	}
+
+	return \@objs;
 }
 
 sub composer {
@@ -337,15 +347,13 @@
 }
 
 sub contributorsOfType {
-	my $self = shift;
-	my $type = shift;
-
-	# Check for valid roles.
-	my $role = Slim::Schema::Contributor->typeToRole($type) || return ();
+	my ($self, @types) = @_;
+
+	my @roles = map { Slim::Schema::Contributor->typeToRole($_) } @types;
 
 	return $self
-		->search_related('contributorTracks', { 'role' => $role })
-		->search_related('contributor');
+		->search_related('contributorTracks', { 'role' => { 'in' => \@roles } }, { 'order_by' => 'role desc' })
+		->search_related('contributor')->distinct;
 }
 
 sub contributorRoles {

Modified: trunk/server/Slim/Web/Pages.pm
URL: http://svn.slimdevices.com/trunk/server/Slim/Web/Pages.pm?rev=8784&r1=8783&r2=8784&view=diff
==============================================================================
--- trunk/server/Slim/Web/Pages.pm (original)
+++ trunk/server/Slim/Web/Pages.pm Thu Aug  3 17:44:29 2006
@@ -137,7 +137,7 @@
 	if ($level eq 'album') {
 
 		$counts{'album'}       = $rs;
-		$counts{'contributor'} = $rs->search_related('contributor');
+		$counts{'contributor'} = $rs->search_related('contributorAlbums')->search_related('contributor');
 		$counts{'track'}       = $rs->search_related('tracks');
 
 	} elsif ($level eq 'contributor' && $previousLevel && $previousLevel eq 'genre') {
@@ -155,13 +155,17 @@
 	} else {
 
 		$counts{'album'}       = Slim::Schema->resultset('Album')->browse;
-		$counts{'track'}       = Slim::Schema->resultset('Track')->browse({ 'me.audio' => 1 });
+		$counts{'track'}       = Slim::Schema->resultset('Track')->browse;
 		$counts{'contributor'} = Slim::Schema->resultset('Contributor')->browse;
 	}
 
 	$params->{'song_count'}   = $class->_lcPlural($counts{'track'}->distinct->count, 'SONG', 'SONGS');
 	$params->{'album_count'}  = $class->_lcPlural($counts{'album'}->distinct->count, 'ALBUM', 'ALBUMS');
 	$params->{'artist_count'} = $class->_lcPlural($counts{'contributor'}->distinct->count, 'ARTIST', 'ARTISTS');
+
+	$::d_sql && msgf("->addLibraryStats() found %d songs, %d albums & %d artists\n", 
+		$params->{'song_count'}, $params->{'album_count'}, $params->{'artist_count'}
+	);
 }
 
 sub addPlayerList {

Modified: trunk/server/Slim/Web/Pages/BrowseDB.pm
URL: http://svn.slimdevices.com/trunk/server/Slim/Web/Pages/BrowseDB.pm?rev=8784&r1=8783&r2=8784&view=diff
==============================================================================
--- trunk/server/Slim/Web/Pages/BrowseDB.pm (original)
+++ trunk/server/Slim/Web/Pages/BrowseDB.pm Thu Aug  3 17:44:29 2006
@@ -46,7 +46,10 @@
 		$::d_info && msg("browsedb - invalid hierarchy: $hierarchy\n");
 	}
 
+	$::d_info && msg("\n");
+	$::d_info && msg("**********************************************\n");
 	$::d_info && msg("browsedb - hierarchy: $hierarchy level: $level\n");
+	$::d_info && msg("**********************************************\n");
 
 	# code further down expects the lcfirst version of the levels
 	my @levels = map { lcfirst($_) } split(',', $validHierarchies->{lc($hierarchy)});
@@ -75,20 +78,11 @@
 	}
 
 	# Build up a list of params to include in the href links.
-	my @attrs    = ();
+	my %attrs    = ();
 
 	my $rs       = Slim::Schema->rs($levelName);
 	my $topRS    = Slim::Schema->rs($levels[0]);
 	my $title    = $params->{'browseby'} = $topRS->title;
-
-	# XXXX - sort is not currently generated or used.
-	# The orderBy is used by the artwork/album sorting feature.
-	my ($filters, $cond, $sort) = $topRS->generateConditionsFromFilters({
-		'rs'      => $rs,
-		'level'   => $level,
-		'levels'  => \@levels,
-		'params'  => $params,
-	});
 
 	# This hash is reused later, during the main item list build
 	my %form = (
@@ -108,34 +102,47 @@
 		'orderBy'      => $orderBy,
 	};
 
-	# We want to include Compilations in the pwd, so we need the artist,
-	# but not in the actual search.
-	if (defined $filters->{'contributor.id'} && 
-	    defined $filters->{'album.compilation'} && $filters->{'album.compilation'} == 1) {
-
-		delete $filters->{'contributor.id'};
-
-		push @attrs, 'album.compilation=1';
-	}
-
 	# browsetree might pass this along - we want to keep it in the attrs
 	# for the breadcrumbs so cue sheets aren't edited. See bug: 1360
 	if (defined $params->{'noEdit'}) {
 
-		push @attrs, join('=', 'noEdit', $params->{'noEdit'});
+		$attrs{'noEdit'} = $params->{'noEdit'};
 	}
 
 	# editplaylist might pass this along - we want to keep it in the attrs
 	# for the pagebar and pwd_list so that we don't go into edit mode. Bug 2870
 	if (defined $params->{'saveCurrentPlaylist'}) {
 
-		push @attrs, join('=', 'saveCurrentPlaylist', $params->{'saveCurrentPlaylist'});
+		$attrs{'saveCurrentPlaylist'} = $params->{'saveCurrentPlaylist'};
 	}
 
 	# Generate the breadcrumb list for the current level.
 	for (my $i = 0; $i < $level ; $i++) {
 
 		my $attr = $levels[$i];
+
+		# we're at the contributor level, looking for compilations
+		# and no defined artist, we need add VA for the breadcrumb
+		if ($attr eq 'contributor') {
+
+			my $vaObjId = Slim::Schema->variousArtistsObject->id;
+
+			# we're at the contributor level, looking for compilations
+			# and no defined artist, we need add VA for the breadcrumb
+			if (defined $params->{'album.compilation'} && !defined $params->{'contributor.id'}) {
+
+				$params->{'contributor.id'} = $vaObjId;
+			}
+
+			if ($params->{'contributor.id'} && $params->{'contributor.id'} == $vaObjId) {
+
+				$attrs{'album.compilation'} = 1;
+
+				$::d_info && msg("browsedb - adding VA for breadcrumb\n");
+			}
+		}
+
+		$::d_info && msg("browsedb - breadcrumb level: [$i] attr: [$attr]\n");
 
 		for my $levelKey (grep { /^$attr/ } keys %{$params}) {
 
@@ -154,6 +161,13 @@
 				$searchKey = sprintf('%s.%s', $rs->{'attrs'}{'alias'}, $2);
 			}
 
+			# Don't try and look non-id items.
+			if ($searchKey !~ /\.id$/) {
+				next;
+			}
+
+			$::d_info && msg("browsedb - breadcrumb levelKey: [$levelKey] value: [$value]\n");
+
 			my $obj = $rs->search({ $searchKey => $value })->single;
 
 			if (blessed($obj) && $obj->can('name')) {
@@ -161,7 +175,7 @@
 				$value           = $obj->name;
 			}
 
-			push @attrs, join('=', $levelKey, $params->{$levelKey});
+			$attrs{$levelKey} = $params->{$levelKey};
 
 			push @{$params->{'pwd_list'}}, {
 				 'hreftype'     => 'browseDb',
@@ -169,11 +183,25 @@
 				 'hierarchy'    => $hierarchy,
 				 'level'        => $i+1,
 				 'orderBy'      => $orderBy,
-				 'attributes'   => (scalar(@attrs) ? ('&' . join("&", @attrs)) : ''),
-			}
-		}
-	}
-	
+				 'attributes'   => _attributesToKeyValuePair(\%attrs),
+			}
+		}
+	}
+
+	# We want to include Compilations in the pwd, so we need the
+	# artist, but not in the actual search.
+	# 
+	# This block needs to be after the pwd building.
+	if ($params->{'contributor.id'}) {
+
+		if ($params->{'contributor.id'} == Slim::Schema->variousArtistsObject->id) {
+
+			delete $params->{'contributor.id'};
+
+			$attrs{'album.compilation'} = 1;
+		}
+	}
+
 	# Bug 3311, disable editing for iTunes, MoodLogic, and MusicMagic playlists
 	if (ref $params->{'playlist'}) {
 
@@ -183,20 +211,37 @@
 		}
 	}
 
-	my $otherparams = join('&',
-		'player=' . Slim::Utils::Misc::escape($player || ''),
-		"hierarchy=$hierarchy",
-		"level=$level",
-		@attrs,
-	);
+	# otherParams is used by the page bar generator.
+	my %otherParams = %attrs;
+
+	if (defined $player) {
+		$otherParams{'player'}    = Slim::Utils::Misc::escape($player);
+	}
+
+	if (defined $hierarchy) {
+		$otherParams{'hierarchy'} = $hierarchy;
+	}
+
+	if (defined $level) {
+		$otherParams{'level'}     = $level;
+	}
 
 	if (defined $orderBy) {
-		$otherparams .= '&' . "orderBy=$orderBy";
+		$otherParams{'orderBy'}  = $orderBy;
 	}
 
 	if (defined $artwork) {
-		$otherparams .= '&' . "artwork=$artwork";
-	}
+		$otherParams{'artwork'}  = $artwork;
+	}
+
+	# XXXX - sort is not currently generated or used.
+	# The orderBy is used by the artwork/album sorting feature.
+	my ($filters, $cond, $sort) = $topRS->generateConditionsFromFilters({
+		'rs'      => $rs,
+		'level'   => $level,
+		'levels'  => \@levels,
+		'params'  => $params,
+	});
 
 	my $browseRS = $topRS->descend($filters, $cond, $orderBy, @levels[0..$level])->distinct;
 	my $count    = 0;
@@ -230,7 +275,7 @@
 			'results'      => $alphaitems ? $alphaitems : $browseRS,
 			'addAlpha'     => defined $alphaitems,
 			'path'         => $params->{'path'},
-			'otherParams'  => $otherparams,
+			'otherParams'  => _attributesToKeyValuePair(\%otherParams),
 			'start'        => $params->{'start'},
 			'perPage'      => $params->{'itemsPerPage'},
 		});
@@ -288,7 +333,7 @@
 		$form{'orderBy'}      = $nextLevelRS->orderBy;
 		$form{'odd'}          = ($itemCount + 1) % 2;
 		$form{'skinOverride'} = $params->{'skinOverride'};
-		$form{'attributes'}   = (scalar(@attrs) ? ('&' . join("&", @attrs)) : '');
+		$form{'attributes'}   = _attributesToKeyValuePair(\%attrs);
 
 		# For some queries - such as New Music - we want to
 		# get the list of tracks to play from the fieldInfo
@@ -306,12 +351,15 @@
 	if ($levelName eq 'contributor' && Slim::Utils::Prefs::get('variousArtistAutoIdentification')) {
 
 		# Only show VA item if there's valid listings below the current level.
-		my %find = map { split /=/ } @attrs;
-
-		if (Slim::Schema->variousArtistsAlbumCount(\%find)) {
-
-			my $vaObj      = Slim::Schema->variousArtistsObject;
-			my @attributes = (@attrs, 'album.compilation=1', sprintf('contributor.id=%d', $vaObj->id));
+		my %attributes = %attrs;
+
+		if (Slim::Schema->variousArtistsAlbumCount(\%attrs)) {
+
+			$::d_info && msg("browsedb - VA added\n");
+
+			my $vaObj = Slim::Schema->variousArtistsObject;
+
+			$attributes{'album.compilation'} = 1;
 
 			push @{$params->{'browse_items'}}, {
 				'text'        => $vaObj->name,
@@ -321,7 +369,7 @@
 				'level'       => $level + 1,
 				'orderBy'     => $orderBy,
 				'odd'         => ($itemCount + 1) % 2,
-				'attributes'  => (scalar(@attributes) ? ('&' . join("&", @attributes, )) : ''),
+				'attributes'  => _attributesToKeyValuePair(\%attributes),
 			};
 
 			$itemCount++;
@@ -365,15 +413,12 @@
 			# If we're at the track level - only append the track
 			# id for each item - it's a unique value and doesn't
 			# need any joins.
-			if (lc($levelName) eq 'track') {
-
-				$form{'attributes'}    = sprintf('&%s.id=%d', $attrName, $itemid);
-
-			} else {
-
-				$form{'attributes'}    = (scalar(@attrs) ? ('&' . join('&', @attrs)) : '') . '&' .
-					sprintf('%s.id=%d', $attrName, $itemid);
-			}
+			if (lc($levelName) ne 'track') {
+
+				$form{'attributes'} = _attributesToKeyValuePair(\%attrs);
+			}
+
+			$form{'attributes'} .= sprintf('&%s.id=%d', $attrName, $itemid);
 
 			$item->displayAsHTML(\%form, $descend, $orderBy);
 
@@ -486,6 +531,21 @@
 	return browsedb($client, $params);
 }
 
+# Turn an array of attributes into a URI string
+# 
+# Use URI::Query instead?
+sub _attributesToKeyValuePair {
+	my $attrs = shift;
+	my @pairs = ();
+
+	for my $key (sort keys %{$attrs}) {
+
+		push @pairs, sprintf('%s=%s', $key, $attrs->{$key});
+	}
+
+	return sprintf('&%s', join('&', @pairs));
+}
+
 1;
 
 __END__

Modified: trunk/server/Slim/Web/Pages/LiveSearch.pm
URL: http://svn.slimdevices.com/trunk/server/Slim/Web/Pages/LiveSearch.pm?rev=8784&r1=8783&r2=8784&view=diff
==============================================================================
--- trunk/server/Slim/Web/Pages/LiveSearch.pm (original)
+++ trunk/server/Slim/Web/Pages/LiveSearch.pm Thu Aug  3 17:44:29 2006
@@ -168,7 +168,7 @@
 
 	} elsif ($type eq 'contributor') {
 
-		$url = "browsedb.html?hierarchy=contributor,album,track\&amp;level=1\&amp;contributor.id=$id";
+		$url = "browsedb.html?hierarchy=contributor,album,track\&amp;level=1\&amp;contributor.id=$id&contributor.role=ALL";
 	}
 
 	push @xml,"<div class=\"$rowType\">\n<div class=\"browsedbListItem\">

Modified: trunk/server/Slim/Web/Pages/Search.pm
URL: http://svn.slimdevices.com/trunk/server/Slim/Web/Pages/Search.pm?rev=8784&r1=8783&r2=8784&view=diff
==============================================================================
--- trunk/server/Slim/Web/Pages/Search.pm (original)
+++ trunk/server/Slim/Web/Pages/Search.pm Thu Aug  3 17:44:29 2006
@@ -356,7 +356,8 @@
 
 		if ($type eq 'contributor') {
 
-			$form{'hierarchy'} = 'contributor,album,track';
+			$form{'attributes'} .= '&contributor.role=ALL';
+			$form{'hierarchy'}  = 'contributor,album,track';
 
 		} elsif ($type eq 'album') {
 



More information about the checkins mailing list