Browse Source

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 <frederic.durand@univ-lyon2.fr>
Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz>
3.4.x
Colin Campbell 13 years ago
committed by Chris Cormack
parent
commit
0cf2eccfe9
  1. 44
      C4/Auth.pm
  2. 44
      C4/VirtualShelves.pm
  3. 12
      C4/VirtualShelves/Page.pm
  4. 15
      catalogue/search.pl
  5. 2
      koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/results.tmpl
  6. 4
      virtualshelves/addbybiblionumber.pl

44
C4/Auth.pm

@ -164,19 +164,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);
@ -297,11 +297,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});
}
}
@ -852,12 +852,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 {
@ -877,9 +877,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...

44
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);

12
C4/VirtualShelves/Page.pm

@ -393,18 +393,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;

15
catalogue/search.pl

@ -673,17 +673,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);
}

2
koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/results.tmpl

@ -65,8 +65,8 @@ $(".addtocart").show();
param1 += "<option id=\"s<!-- TMPL_VAR NAME="shelfnumber" -->\" value=\"addtolist\"><!-- TMPL_VAR NAME="shelfname" ESCAPE="html"--><\/option>";<!-- /TMPL_LOOP -->
param1 += "<\/optgroup>";<!-- /TMPL_IF -->
<!-- TMPL_IF NAME="addpubshelves" -->param1 += "<optgroup label=\""+_("Public Lists:")+"\">"<!-- TMPL_LOOP NAME="addpubshelvesloop" -->+"<option id=\"s<!-- TMPL_VAR NAME="shelfnumber" -->\" value=\"addtolist\"><!-- TMPL_VAR NAME="shelfname" ESCAPE="html"--><\/option>"<!-- /TMPL_LOOP -->
<!-- /TMPL_IF -->
param1 +="<\/optgroup><option value=\"newlist\">"+_("[ New List ]")+"<\/option>"
<!-- /TMPL_IF -->
<!-- /TMPL_IF -->
param1 += "<\/select> <input id=\"cartsubmit\" type=\"submit\" class=\"submit\" value=\""+_("Save")+"\" />";
$('#sortsubmit').hide();

4
virtualshelves/addbybiblionumber.pl

@ -160,13 +160,13 @@ else { # this shelf doesn't already exist.
#grab each type of shelf, open (type 3) should not be limited by user.
foreach my $shelftype (1,2,3) {
my ($shelflist) = GetRecentShelves($shelftype, $limit, $shelftype == 3 ? undef : $loggedinuser);
for my $shelf (@{ $shelflist->[0] }) {
for my $shelf (@{ $shelflist }) {
push(@shelvesloop, $shelf->{shelfnumber});
$shelvesloop{$shelf->{shelfnumber}} = $shelf->{shelfname};
}
}
if(@shelvesloop gt 0){
if( @shelvesloop ){
my $CGIvirtualshelves = CGI::scrolling_list
(
-name => 'shelfnumber',

Loading…
Cancel
Save