From 554f7f522158d65314a2da5f0266b132b8fab858 Mon Sep 17 00:00:00 2001 From: Colin Campbell Date: Thu, 7 Apr 2011 16:25:27 +0200 Subject: [PATCH] Bug 5529 Absence or Presence of lists not being reliably returned C4::VirtualShelves::GetRecentShelves contained some rather confused code The contents of the requested list are returned in an arrayref which was in its turn being wrapped needlessly in an array As a result the returned array always consisted of a single element irrespective of the number of lists. Made the routine return the arrayref, which can now be tested directly Unfortunately rather than fixing this we had previously coded around it assuming it to be a "design" decision. Have amended other calls of the subroutine resulting in some hopefully less obscure code Fixed logic error in the results template which displayed new list within a test for the presence of lists Removed the offset parameter from the sql in the routine as it was hardcoded to 0 i.e. the default value Signed-off-by: fdurand Signed-off-by: Chris Cormack (cherry picked from commit 0cf2eccfe926f77753a2b948e2babf077e2975d3) Signed-off-by: Chris Nighswonger --- C4/Auth.pm | 44 +++++++++---------- C4/VirtualShelves.pm | 44 +++++++++++-------- C4/VirtualShelves/Page.pm | 12 ++--- catalogue/search.pl | 15 +++---- .../prog/en/modules/catalogue/results.tmpl | 2 +- virtualshelves/addbybiblionumber.pl | 4 +- 6 files changed, 62 insertions(+), 59 deletions(-) diff --git a/C4/Auth.pm b/C4/Auth.pm index aae851846b..1c4573e659 100644 --- a/C4/Auth.pm +++ b/C4/Auth.pm @@ -154,19 +154,19 @@ sub get_template_and_user { $template->param( loggedinusername => $user ); $template->param( sessionID => $sessionID ); - my ($total, $pubshelves, $barshelves) = C4::Context->get_shelves_userenv(); - if (defined($pubshelves)) { - $template->param( pubshelves => scalar (@$pubshelves), - pubshelvesloop => $pubshelves, - ); - $template->param( pubtotal => $total->{'pubtotal'}, ) if ($total->{'pubtotal'} > scalar (@$pubshelves)); - } - if (defined($barshelves)) { - $template->param( barshelves => scalar (@$barshelves), - barshelvesloop => $barshelves, - ); - $template->param( bartotal => $total->{'bartotal'}, ) if ($total->{'bartotal'} > scalar (@$barshelves)); - } + my ($total, $pubshelves, $barshelves) = C4::Context->get_shelves_userenv(); + if (defined($pubshelves)) { + $template->param( pubshelves => scalar @{$pubshelves}, + pubshelvesloop => $pubshelves, + ); + $template->param( pubtotal => $total->{'pubtotal'}, ) if ($total->{'pubtotal'} > scalar @{$pubshelves}); + } + if (defined($barshelves)) { + $template->param( barshelves => scalar @{$barshelves}, + barshelvesloop => $barshelves, + ); + $template->param( bartotal => $total->{'bartotal'}, ) if ($total->{'bartotal'} > scalar @{$barshelves}); + } $borrowernumber = getborrowernumber($user) if defined($user); @@ -287,11 +287,11 @@ sub get_template_and_user { $template->param( sessionID => $sessionID ); my ($total, $pubshelves) = C4::Context->get_shelves_userenv(); # an anonymous user has no 'barshelves'... - if (defined(($pubshelves))) { - $template->param( pubshelves => scalar (@$pubshelves), + if (defined $pubshelves) { + $template->param( pubshelves => scalar @{$pubshelves}, pubshelvesloop => $pubshelves, ); - $template->param( pubtotal => $total->{'pubtotal'}, ) if ($total->{'pubtotal'} > scalar (@$pubshelves)); + $template->param( pubtotal => $total->{'pubtotal'}, ) if ($total->{'pubtotal'} > scalar @{$pubshelves}); } } @@ -836,12 +836,12 @@ sub checkauth { $total->{'bartotal'} = $totshelves; ($pubshelves, $totshelves) = C4::VirtualShelves::GetRecentShelves(2, $row_count, undef); $total->{'pubtotal'} = $totshelves; - $session->param('barshelves', $barshelves->[0]); - $session->param('pubshelves', $pubshelves->[0]); + $session->param('barshelves', $barshelves); + $session->param('pubshelves', $pubshelves); $session->param('totshelves', $total); - C4::Context::set_shelves_userenv('bar',$barshelves->[0]); - C4::Context::set_shelves_userenv('pub',$pubshelves->[0]); + C4::Context::set_shelves_userenv('bar',$barshelves); + C4::Context::set_shelves_userenv('pub',$pubshelves); C4::Context::set_shelves_userenv('tot',$total); } else { @@ -861,9 +861,9 @@ sub checkauth { my ($total, $totshelves, $pubshelves); ($pubshelves, $totshelves) = C4::VirtualShelves::GetRecentShelves(2, $row_count, undef); $total->{'pubtotal'} = $totshelves; - $session->param('pubshelves', $pubshelves->[0]); + $session->param('pubshelves', $pubshelves); $session->param('totshelves', $total); - C4::Context::set_shelves_userenv('pub',$pubshelves->[0]); + C4::Context::set_shelves_userenv('pub',$pubshelves); C4::Context::set_shelves_userenv('tot',$total); # setting a couple of other session vars... diff --git a/C4/VirtualShelves.pm b/C4/VirtualShelves.pm index 72571490ec..5bddadbbdd 100644 --- a/C4/VirtualShelves.pm +++ b/C4/VirtualShelves.pm @@ -186,9 +186,9 @@ sub GetShelvesSummary ($$$$) { =head2 GetRecentShelves - ($shelflist) = GetRecentShelves(1, $limit, $owner) + ($shelflist, $total) = GetRecentShelves(1, $limit, $owner) -This function returns a references to an array of hashrefs containing specified shelves sorted +This function returns a reference to an array of hashrefs containing specified shelves sorted by the date the shelf was last modified in descending order limited to the number of records specified by C<$row_count>. If calling with C<$mincategory> other than 1, use undef as C<$owner>. @@ -197,19 +197,25 @@ the submitted parameters. =cut -sub GetRecentShelves ($$$) { - my ($mincategory, $row_count, $owner) = @_; - my (@shelflist); - my $total = _shelf_count($owner, $mincategory); - my @params = ($owner, $mincategory, 0, $row_count); #FIXME: offset is hardcoded here, but could be passed in for enhancements - shift @params if (not defined $owner); - my $query = "SELECT * FROM virtualshelves"; - $query .= ((defined $owner) ? " WHERE owner = ? AND category = ?" : " WHERE category >= ? "); - $query .= " ORDER BY lastmodified DESC LIMIT ?, ?"; - my $sth = $dbh->prepare($query); - $sth->execute(@params); - @shelflist = $sth->fetchall_arrayref({}); - return ( \@shelflist, $total ); +sub GetRecentShelves { + my ($mincategory, $row_count, $owner) = @_; + my $total = _shelf_count($owner, $mincategory); + my @params; + my $selection; + if (defined $owner) { + @params = ($owner, $mincategory, $row_count); + $selection = ' WHERE owner = ? AND category = ?'; + } else { + @params = ( $mincategory, $row_count); + $selection = ' WHERE category >= ? '; + } + my $query = 'SELECT * FROM virtualshelves'; + $query .= $selection; + $query .= ' ORDER BY lastmodified DESC LIMIT ?'; + my $sth = $dbh->prepare($query); + $sth->execute(@params); + my $shelflist = $sth->fetchall_arrayref({}); + return ( $shelflist, $total ); } =head2 GetAllShelves @@ -563,13 +569,13 @@ sub RefreshShelvesSummary ($$$) { $total->{'pubtotal'} = $totshelves; # Update the current session with the latest shelves... - $session->param('barshelves', $barshelves->[0]); - $session->param('pubshelves', $pubshelves->[0]); + $session->param('barshelves', $barshelves); + $session->param('pubshelves', $pubshelves); $session->param('totshelves', $total); # likewise the userenv... - C4::Context->set_shelves_userenv('bar',$barshelves->[0]); - C4::Context->set_shelves_userenv('pub',$pubshelves->[0]); + C4::Context->set_shelves_userenv('bar',$barshelves); + C4::Context->set_shelves_userenv('pub',$pubshelves); C4::Context::set_shelves_userenv('tot',$total); return ($total, $pubshelves, $barshelves); diff --git a/C4/VirtualShelves/Page.pm b/C4/VirtualShelves/Page.pm index 49d1aa8ece..3674410bc6 100644 --- a/C4/VirtualShelves/Page.pm +++ b/C4/VirtualShelves/Page.pm @@ -388,18 +388,18 @@ sub shelfpage ($$$$$) { if ( defined $barshelves ) { $template->param( - barshelves => scalar( @{ $barshelves->[0] } ), - barshelvesloop => $barshelves->[0], + barshelves => scalar( @{ $barshelves } ), + barshelvesloop => $barshelves, ); - $template->param( bartotal => $total->{'bartotal'}, ) if ( $total->{'bartotal'} > scalar( @{ $barshelves->[0] } ) ); + $template->param( bartotal => $total->{'bartotal'}, ) if ( $total->{'bartotal'} > scalar( @{ $barshelves } ) ); } if ( defined $pubshelves ) { $template->param( - pubshelves => scalar( @{ $pubshelves->[0] } ), - pubshelvesloop => $pubshelves->[0], + pubshelves => scalar( @{ $pubshelves } ), + pubshelvesloop => $pubshelves, ); - $template->param( pubtotal => $total->{'pubtotal'}, ) if ( $total->{'pubtotal'} > scalar( @{ $pubshelves->[0] } ) ); + $template->param( pubtotal => $total->{'pubtotal'}, ) if ( $total->{'pubtotal'} > scalar( @{ $pubshelves } ) ); } output_html_with_http_headers $query, $cookie, $template->output; diff --git a/catalogue/search.pl b/catalogue/search.pl index 40e5f8c3d9..3e98e05e59 100755 --- a/catalogue/search.pl +++ b/catalogue/search.pl @@ -656,17 +656,14 @@ my $row_count = 10; # FIXME:This probably should be a syspref my ($pubshelves, $total) = GetRecentShelves(2, $row_count, undef); my ($barshelves, $total) = GetRecentShelves(1, $row_count, $borrowernumber); -my @pubshelves = @{$pubshelves}; -my @barshelves = @{$barshelves}; - -if (@pubshelves) { - $template->param( addpubshelves => scalar (@pubshelves)); - $template->param( addpubshelvesloop => @pubshelves); +if (@{$pubshelves}) { + $template->param( addpubshelves => scalar @{$pubshelves}); + $template->param( addpubshelvesloop => $pubshelves); } -if (@barshelves) { - $template->param( addbarshelves => scalar (@barshelves)); - $template->param( addbarshelvesloop => @barshelves); +if (@{$barshelves}) { + $template->param( addbarshelves => scalar @{$barshelves}); + $template->param( addbarshelvesloop => $barshelves); } diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/results.tmpl b/koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/results.tmpl index 76b874100c..6336625fc4 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/results.tmpl +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/results.tmpl @@ -45,8 +45,8 @@ $(".addtocart").show(); param1 += ""+"