Bug 14544: Get rid of GetShelf

Signed-off-by: Alex Arnaud <alex.arnaud@biblibre.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

diff --git a/C4/VirtualShelves.pm b/C4/VirtualShelves.pm
index 0b11830..72aa7f4 100644 (file)
--- a/C4/VirtualShelves.pm
+++ b/C4/VirtualShelves.pm
@@ -40,7 +40,7 @@ BEGIN {
     require Exporter;
     @ISA    = qw(Exporter);
     @EXPORT = qw(
-            &GetShelves &GetShelfContents &GetShelf
+            &GetShelves &GetShelfContents
             &AddToShelf &AddShelf
             &ModShelf
             &ShelfPossibleAction
@@ -213,28 +213,6 @@ sub GetSomeShelfNames {
     return ( { bartotal => $bar? scalar @$bar: 0, pubtotal => $pub? scalar @$pub: 0}, $pub, $bar);
 }
 
-=head2 GetShelf
-
-  (shelfnumber,shelfname,owner,category,sortfield,allow_add,allow_delete_own,allow_delete_other) = &GetShelf($shelfnumber);
-
-Returns the above-mentioned fields for passed virtual shelf number.
-
-=cut
-
-sub GetShelf {
-    my ($shelfnumber) = @_;
-    my $dbh = C4::Context->dbh;
-    my $query = qq(
-        SELECT shelfnumber, shelfname, owner, category, sortfield,
-            allow_add, allow_delete_own, allow_delete_other
-        FROM   virtualshelves
-        WHERE  shelfnumber=?
-    );
-    my $sth = $dbh->prepare($query);
-    $sth->execute($shelfnumber);
-    return $sth->fetchrow;
-}
-
 =head2 GetShelfContents
 
   $biblist = &GetShelfContents($shelfnumber);

diff --git a/C4/VirtualShelves/Page.pm b/C4/VirtualShelves/Page.pm
index f4f09d7..7ed7f0e 100644 (file)
--- a/C4/VirtualShelves/Page.pm
+++ b/C4/VirtualShelves/Page.pm
@@ -40,6 +40,8 @@ use C4::Tags qw(get_tags);
 use C4::Csv;
 use C4::XSLT;
 
+use Koha::Virtualshelves;
+
 use constant VIRTUALSHELVES_COUNT => 20;
 
 use vars qw($debug @EXPORT @ISA $VERSION);
@@ -203,23 +205,23 @@ sub shelfpage {
             }
         #Editing a shelf
         elsif ( $op eq 'modif' ) {
-                my ( $shelfnumber2, $shelfname, $owner, $category, $sortfield, $allow_add, $allow_delete_own, $allow_delete_other) = GetShelf($shelfnumber);
-                my $member = GetMember( 'borrowernumber' => $owner );
+                my $shelf = Koha::Virtualshelves->find( $shelfnumber );
+                my $member = GetMember( 'borrowernumber' => $shelf->owner );
                 my $ownername = defined($member) ? $member->{firstname} . " " . $member->{surname} : '';
                 $edit = 1;
                 $template->param(
                     edit                => 1,
                     display             => $displaymode,
-                    shelfnumber         => $shelfnumber2,
-                    shelfname           => $shelfname,
-                    owner               => $owner,
+                    shelfnumber         => $shelf->shelfnumber,
+                    shelfname           => $shelf->shelfname,
+                    owner               => $shelf->owner,
                     ownername           => $ownername,
-                    "category$category" => 1,
-                    category            => $category,
-                    sortfield           => $sortfield,
-                    allow_add           => $allow_add,
-                    allow_delete_own    => $allow_delete_own,
-                    allow_delete_other  => $allow_delete_other,
+                    "category".$shelf->category => 1,
+                    category            => $shelf->category,
+                    sortfield           => $shelf->sortfield,
+                    allow_add           => $shelf->allow_add,
+                    allow_delete_own    => $shelf->allow_delete_own,
+                    allow_delete_other  => $shelf->allow_delete_other,
                 );
             }
             last SWITCH;
@@ -227,9 +229,7 @@ sub shelfpage {
 
         #View a shelf
         if ( $shelfnumber = $query->param('viewshelf') ) {
-            # explicitly fetch this shelf
-            my ($shelfnumber2,$shelfname,$owner,$category,$sorton) = GetShelf($shelfnumber);
-
+            my $shelf = Koha::Virtualshelves->find( $shelfnumber );
             $template->param(
                 'DisplayMultiPlaceHold' => C4::Context->preference('DisplayMultiPlaceHold'),
             );
@@ -243,7 +243,7 @@ sub shelfpage {
             if ( ShelfPossibleAction( $loggedinuser, $shelfnumber, 'view' ) ) {
                 my $items;
                 my $tag_quantity;
-                my $sortfield = ( $sorton ? $sorton : 'title' );
+                my $sortfield = ( $shelf->sortfield ? $shelf->sortfield : 'title' );
                 $sortfield = $query->param('sort') || $sortfield; ## Passed in sorting overrides default sorting
                 my $direction = $query->param('direction') || 'asc';
                 $template->param(
@@ -313,13 +313,13 @@ sub shelfpage {
                         addpubshelvesloop => $pubshelves,
                     );
                 }
-                push @paramsloop, { display => 'privateshelves' } if $category == 1;
+                push @paramsloop, { display => 'privateshelves' } if $shelf->category == 1;
                 $showadd = 1;
                 my $i = 0;
                 my $manageshelf = ShelfPossibleAction( $loggedinuser, $shelfnumber, 'manage' );
                 my $can_delete_shelf = ShelfPossibleAction( $loggedinuser, $shelfnumber, 'delete_shelf' );
                 $template->param(
-                    shelfname           => $shelfname,
+                    shelfname           => $shelf->shelfname,
                     shelfnumber         => $shelfnumber,
                     viewshelf           => $shelfnumber,
                     sortfield           => $sortfield,
@@ -327,10 +327,10 @@ sub shelfpage {
                     allowremovingitems  => ShelfPossibleAction( $loggedinuser, $shelfnumber, 'delete'),
                     allowaddingitem     => ShelfPossibleAction( $loggedinuser, $shelfnumber, 'add'),
                     allowdeletingshelf  => $can_delete_shelf,
-                    "category$category" => 1,
-                    category            => $category,
+                    "category".$shelf->category => 1,
+                    category            => $shelf->category,
                     itemsloop           => $items,
-                    showprivateshelves  => $category==1,
+                    showprivateshelves  => $shelf->category==1,
                 );
             } else {
                 push @paramsloop, { nopermission => $shelfnumber };

diff --git a/Koha/Virtualshelf.pm b/Koha/Virtualshelf.pm
index 98877a1..51e8793 100644 (file)
--- a/Koha/Virtualshelf.pm
+++ b/Koha/Virtualshelf.pm
@@ -38,7 +38,7 @@ Koha::Virtualshelf - Koha Virtualshelf Object class
 =cut
 
 sub type {
-    return 'Virtualshelf';
+    return 'Virtualshelve';
 }
 
 1;

diff --git a/Koha/Virtualshelves.pm b/Koha/Virtualshelves.pm
index 5cffdfe..5627afa 100644 (file)
--- a/Koha/Virtualshelves.pm
+++ b/Koha/Virtualshelves.pm
@@ -40,7 +40,7 @@ Koha::Virtualshelf - Koha Virtualshelf Object class
 =cut
 
 sub type {
-    return 'Virtualshelf';
+    return 'Virtualshelve';
 }
 
 sub object_class {

diff --git a/opac/opac-addbybiblionumber.pl b/opac/opac-addbybiblionumber.pl
index 6855aa2..67f6d00 100755 (executable)
--- a/opac/opac-addbybiblionumber.pl
+++ b/opac/opac-addbybiblionumber.pl
@@ -31,6 +31,8 @@ use C4::VirtualShelves qw/:DEFAULT GetAllShelves/;
 use C4::Output;
 use C4::Auth;
 
+use Koha::Virtualshelves;
+
 our $query             = new CGI;
 our @biblionumber      = $query->param('biblionumber');
 our $selectedshelf     = $query->param('selectedshelf');
@@ -115,11 +117,12 @@ sub HandleShelfNumber {
 sub HandleSelectedShelf {
     if($authorized= ShelfPossibleAction( $loggedinuser, $selectedshelf, 'add')){
         #adding to specific shelf
-        my ($singleshelf, $singleshelfname)= GetShelf($query->param('selectedshelf'));
+        my $shelfnumber = $query->param('selectedshelf');
+        my $shelf = Koha::Virtualshelves->find( $shelfnumber );
         $template->param(
-        singleshelf               => 1,
-        shelfnumber               => $singleshelf,
-        shelfname                 => $singleshelfname,
+            singleshelf               => 1,
+            shelfnumber               => $shelf->shelfnumber,
+            shelfname                 => $shelf->shelfname,
         );
     }
 }

diff --git a/opac/opac-downloadshelf.pl b/opac/opac-downloadshelf.pl
index 4a62325..15a85dc 100755 (executable)
--- a/opac/opac-downloadshelf.pl
+++ b/opac/opac-downloadshelf.pl
@@ -31,6 +31,9 @@ use C4::VirtualShelves;
 use C4::Record;
 use C4::Ris;
 use C4::Csv;
+
+use Koha::Virtualshelves;
+
 use utf8;
 my $query = new CGI;
 
@@ -104,8 +107,7 @@ if ( ShelfPossibleAction( (defined($borrowernumber) ? $borrowernumber : -1), $sh
 
     } else {
 
-        # get details of the list
-        my ($shelfnumber,$shelfname,$owner,$category,$sorton) = GetShelf($shelfid);
+        my $shelf = Koha::Virtualshelves->find( $shelfid );
 
         # if modal context is passed set a variable so that page markup can be different
         if($context eq "modal"){
@@ -116,10 +118,10 @@ if ( ShelfPossibleAction( (defined($borrowernumber) ? $borrowernumber : -1), $sh
         $template->param(csv_profiles => GetCsvProfilesLoop('marc'));
         $template->param(
             showprivateshelves  => $showprivateshelves,
-            shelfid             => $shelfid,
-            shelfname           => $shelfname,
-            shelfnumber         => $shelfnumber,
-            viewshelf           => $shelfnumber
+            shelfid             => $shelf->shelfnumber,
+            shelfnumber         => $shelf->shelfnumber,
+            viewshelf           => $shelf->shelfnumber,
+            shelfname           => $shelf->shelfname,
         );
         output_html_with_http_headers $query, $cookie, $template->output;
     }

diff --git a/opac/opac-sendshelf.pl b/opac/opac-sendshelf.pl
index 8512c65..07353ac 100755 (executable)
--- a/opac/opac-sendshelf.pl
+++ b/opac/opac-sendshelf.pl
@@ -34,6 +34,7 @@ use C4::Output;
 use C4::VirtualShelves;
 use C4::Members;
 use Koha::Email;
+use Koha::Virtualshelves;
 
 my $query = new CGI;
 
@@ -72,7 +73,7 @@ if ( $email ) {
         }
     );
 
-    my @shelf               = GetShelf($shelfid);
+    my $shelf = Koha::Virtualshelves->find( $shelfid );
     my ($items, $totitems)  = GetShelfContents($shelfid);
     my $marcflavour         = C4::Context->preference('marcflavour');
     my $iso2709;
@@ -110,7 +111,7 @@ if ( $email ) {
     $template2->param(
         BIBLIO_RESULTS => \@results,
         comment        => $comment,
-        shelfname      => $shelf[1],
+        shelfname      => $shelf->shelfname,
         firstname      => $user->{firstname},
         surname        => $user->{surname},
     );

diff --git a/opac/opac-shareshelf.pl b/opac/opac-shareshelf.pl
index f967d71..a27aa03 100755 (executable)
--- a/opac/opac-shareshelf.pl
+++ b/opac/opac-shareshelf.pl
@@ -34,6 +34,8 @@ use C4::Members ();
 use C4::Output;
 use C4::VirtualShelves;
 
+use Koha::Virtualshelves;
+
 #-------------------------------------------------------------------------------
 
 my $pvar = _init( {} );
@@ -67,11 +69,12 @@ sub _init {
     }
 
     #get some list details
-    my @temp;
-    @temp = GetShelf( $param->{shelfnumber} ) if !$param->{errcode};
-    $param->{shelfname} = @temp ? $temp[1] : '';
-    $param->{owner}     = @temp ? $temp[2] : -1;
-    $param->{category}  = @temp ? $temp[3] : -1;
+    my $shelf;
+    my $shelfnumber = $param->{shelfnumber};
+    $shelf = Koha::Virtualshelves->find( $shelfnumber ) unless $param->{errcode};
+    $param->{shelfname} = $shelf ? $shelf->shelfname : q||;
+    $param->{owner}     = $shelf ? $shelf->owner : -1;
+    $param->{category}  = $shelf ? $shelf->category : -1;
 
     load_template($param);
     return $param;

diff --git a/t/db_dependent/VirtualShelves.t b/t/db_dependent/VirtualShelves.t
index 70dbc62..e9af166 100755 (executable)
--- a/t/db_dependent/VirtualShelves.t
+++ b/t/db_dependent/VirtualShelves.t
@@ -12,6 +12,8 @@ use C4::Biblio qw( AddBiblio DelBiblio );
 use C4::Context;
 use C4::Members qw( AddMember );
 
+use Koha::Virtualshelves;
+
 
 my $dbh = C4::Context->dbh;
 $dbh->{RaiseError} = 1;
@@ -82,10 +84,10 @@ for my $i (0..9){
         ok(1, 'skip duplicate test for earlier name clash');
         next;
     }
-    my @shlf=GetShelf($shelves[$i]->{number}); #number, name, owner, catg, ...
+    my $shelf = Koha::Virtualshelves->find( $shelves[$i]->{number} );
 
     # A shelf name is not per se unique!
-    if( $shlf[3]==2 ) { #public list: try to create with same name
+    if( $shelf->category == 2 ) { #public list: try to create with same name
         my $badNumShelf= AddShelf( {
             shelfname=> $shelves[$i]->{name},
             category => 2
@@ -95,12 +97,12 @@ for my $i (0..9){
         DelShelf($badNumShelf) if $badNumShelf>-1; #delete if went wrong..
     }
     else { #private list, try to add another one for SAME user (owner)
-        my $badNumShelf= defined($shlf[2])? AddShelf(
+        my $badNumShelf= defined($shelf->owner)? AddShelf(
             {
                 shelfname=> $shelves[$i]->{name},
                 category => 1,
             },
-            $shlf[2]): -1;
+            $shelf->owner): -1;
         is($badNumShelf, -1, 'do not create private lists with duplicate name for same user');
         DelShelf($badNumShelf) if $badNumShelf>-1; #delete if went wrong..
     }
@@ -145,9 +147,8 @@ for my $i (0..9){
     $used{$key}++;
 }
 
-#-----------------------TEST ModShelf & GetShelf functions------------------------#
+#-----------------------TEST ModShelf & Koha::Virtualshelves->find functions/methods------------------------#
 # usage : ModShelf($shelfnumber, $shelfname, $owner, $category )
-# usage : (shelfnumber,shelfname,owner,category) = GetShelf($shelfnumber);
 
 for my $i (0..9){
     my $rand = int(rand(9));
@@ -167,10 +168,10 @@ for my $i (0..9){
     if(C4::VirtualShelves::_CheckShelfName($newname,$shelf->{category},
             $shelves[$rand]->{owner}, $numA)) {
         ModShelf($numA,$shelf);
-        my ($numB,$nameB,$ownerB,$categoryB) = GetShelf($numA);
-        is($numA, $numB, 'modified shelf');
-        is($shelf->{shelfname}, $nameB,     '... and name change took');
-        is($shelf->{category}, $categoryB, '... and category change took');
+        my $shelf_b = Koha::Virtualshelves->find( $numA );
+        is($numA, $shelf_b->shelfnumber, 'modified shelf');
+        is($shelf->{shelfname}, $shelf_b->shelfname,     '... and name change took');
+        is($shelf->{category}, $shelf_b->category, '... and category change took');
     }
     else {
         ok(1, "No ModShelf for $newname") for 1..3;

diff --git a/virtualshelves/addbybiblionumber.pl b/virtualshelves/addbybiblionumber.pl
index e7b459c..0586b6c 100755 (executable)
--- a/virtualshelves/addbybiblionumber.pl
+++ b/virtualshelves/addbybiblionumber.pl
@@ -67,6 +67,7 @@ use C4::Output;
 use C4::VirtualShelves qw/:DEFAULT GetAllShelves/;
 use C4::Auth;
 
+use Koha::Virtualshelves;
 
 our $query           = new CGI;
 our @biblionumber    = HandleBiblioPars();
@@ -161,11 +162,11 @@ sub HandleShelfNumber {
 sub HandleSelectedShelf {
     if($authorized= ShelfPossibleAction( $loggedinuser, $shelfnumber, 'add')){
         #confirm adding to specific shelf
-        my ($singleshelf, $singleshelfname)= GetShelf($shelfnumber);
+        my $shelf = Koha::Virtualshelves->find( $shelfnumber );
         $template->param(
         singleshelf               => 1,
-        shelfnumber               => $singleshelf,
-        shelfname                 => $singleshelfname,
+        shelfnumber               => $shelf->shelfnumber,
+        shelfname                 => $shelf->shelfname,
         );
     }
     else {

diff --git a/virtualshelves/sendshelf.pl b/virtualshelves/sendshelf.pl
index 2fa77b2..7f1ddbf 100755 (executable)
--- a/virtualshelves/sendshelf.pl
+++ b/virtualshelves/sendshelf.pl
@@ -33,6 +33,7 @@ use C4::Items;
 use C4::Output;
 use C4::VirtualShelves;
 use Koha::Email;
+use Koha::Virtualshelves;
 
 my $query = new CGI;
 
@@ -70,8 +71,8 @@ if ($email) {
         }
     );
 
-    my @shelf = GetShelf($shelfid);
-    my ( $items, $totitems ) = GetShelfContents($shelfid);
+    my $shelf = Koha::Virtualshelves->find( $shelfid );
+    my ( $items, $totitems ) = GetShelfContents($shelf->shelfnumber);
     my $marcflavour = C4::Context->preference('marcflavour');
     my $iso2709;
     my @results;
@@ -104,7 +105,7 @@ if ($email) {
     $template2->param(
         BIBLIO_RESULTS => \@results,
         comment        => $comment,
-        shelfname      => $shelf[1],
+        shelfname      => $shelf->shelfname,
     );
 
     # Getting template result