Bug 13418: Clean C4::VirtualShelves::Page a bit

I tried to reuse this package for a new development, I did not manage.
It's a really hard to read and to understand.

Here my first try, this patch only remove the call to _shelf_count in
the GetShelves subroutine.
Someone might want to get this value for another reason and from
somewhere else.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>

diff --git a/C4/VirtualShelves.pm b/C4/VirtualShelves.pm
index 70dc965..24ee6dd 100644 (file)
--- a/C4/VirtualShelves.pm
+++ b/C4/VirtualShelves.pm
@@ -71,7 +71,7 @@ bibs to and from virtual shelves.
 
 =head2 GetShelves
 
-  ($shelflist, $totshelves) = &GetShelves($category, $row_count, $offset, $owner);
+  $shelflist = &GetShelves($category, $row_count, $offset, $owner);
   ($shelfnumber, $shelfhash) = each %{$shelflist};
 
 Returns the number of shelves specified by C<$row_count> and C<$offset> as well as the total
@@ -90,18 +90,14 @@ and the values (C<$shelfhash>, above) are themselves references-to-hash, with th
 
 A string. The name of the shelf.
 
-=item C<$shelfhash-E<gt>{count}>
-
-The number of virtuals on that virtualshelves.
-
 =back
 
 =cut
 
 sub GetShelves {
     my ($category, $row_count, $offset, $owner) = @_;
-    my @params;
-    my $total = _shelf_count($owner, $category);
+    $offset ||= 0;
+    my @params = ( $offset, $row_count );
     my $dbh = C4::Context->dbh;
     my $query = qq{
         SELECT vs.shelfnumber, vs.shelfname,vs.owner,
@@ -115,11 +111,10 @@ sub GetShelves {
             LEFT JOIN virtualshelfshares sh ON sh.shelfnumber=vs.shelfnumber
             AND sh.borrowernumber=?
         WHERE category=1 AND (vs.owner=? OR sh.borrowernumber=?) };
-        @params= ($owner, $owner, $owner, $offset||0, $row_count);
+        push @params, ($owner) x 3;
     }
     else {
         $query.= 'WHERE category=2 ';
-        @params= ($offset||0, $row_count);
     }
     $query.= qq{
         GROUP BY vs.shelfnumber
@@ -139,7 +134,7 @@ sub GetShelves {
         $shelflist{$shelfnumber}->{'surname'}   = $surname;
         $shelflist{$shelfnumber}->{'firstname'} = $firstname;
     }
-    return ( \%shelflist, $total );
+    return \%shelflist;
 }
 
 =head2 GetAllShelves
@@ -751,9 +746,8 @@ WHERE borrowernumber=? AND shelfnumber=?
     return 1;
 }
 
-# internal subs
 
-sub _shelf_count {
+sub GetShelfCount {
     my ($owner, $category) = @_;
     my @params;
     # Find out how many shelves total meet the submitted criteria...
@@ -777,6 +771,7 @@ sub _shelf_count {
     return $total;
 }
 
+# internal subs
 sub _CheckShelfName {
     my ($name, $cat, $owner, $number)= @_;
 

diff --git a/C4/VirtualShelves/Page.pm b/C4/VirtualShelves/Page.pm
index 052839d..1e3e351 100644 (file)
--- a/C4/VirtualShelves/Page.pm
+++ b/C4/VirtualShelves/Page.pm
@@ -90,7 +90,8 @@ sub shelfpage {
     $shelvesoffset = ( $shelfoff - 1 ) * $shelflimit;    # Sets the offset to begin retrieving shelves at (offset)
                                                 # getting the Shelves list
     my $category = ( ( $displaymode eq 'privateshelves' ) ? 1 : 2 );
-    my ( $shelflist, $totshelves ) = GetShelves( $category, $shelveslimit, $shelvesoffset, $loggedinuser );
+    my $shelflist = GetShelves( $category, $shelveslimit, $shelvesoffset, $loggedinuser );
+    my $totshelves = C4::VirtualShelves::GetShelfCount( $loggedinuser, $category );
 
     #Get a list of private shelves for possible deletion. Only do this when we've defaulted to public shelves
     my ( $privshelflist, $privtotshelves );
@@ -429,7 +430,8 @@ sub shelfpage {
     my $numberCanManage = 0;
 
     # rebuild shelflist in case a shelf has been added
-    ( $shelflist, $totshelves ) = GetShelves( $category, $shelveslimit, $shelvesoffset, $loggedinuser ) unless $delflag;
+    $shelflist = GetShelves( $category, $shelveslimit, $shelvesoffset, $loggedinuser ) unless $delflag;
+    $totshelves = C4::VirtualShelves::GetShelfCount( $loggedinuser, $category ) unless $delflag;
     foreach my $element ( sort { lc( $shelflist->{$a}->{'shelfname'} ) cmp lc( $shelflist->{$b}->{'shelfname'} ) } keys %$shelflist ) {
         my %line;
         $shelflist->{$element}->{shelf} = $element;