From 7373437a14d7f9ac73afdd0e647d7f3daf9c429f Mon Sep 17 00:00:00 2001
From: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Date: Wed, 15 Jul 2015 17:16:34 +0100
Subject: [PATCH] Bug 14544: Get rid of DelShelf

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>
---
 C4/VirtualShelves.pm            | 24 +++---------------------
 C4/VirtualShelves/Page.pm       |  2 +-
 t/db_dependent/VirtualShelves.t |  6 +++---
 t/db_dependent/Virtualshelves.t |  7 ++++++-
 4 files changed, 13 insertions(+), 26 deletions(-)

diff --git a/C4/VirtualShelves.pm b/C4/VirtualShelves.pm
index 0f5e9000bc..e7201f0134 100644
--- a/C4/VirtualShelves.pm
+++ b/C4/VirtualShelves.pm
@@ -44,7 +44,7 @@ BEGIN {
             &AddToShelf
             &ModShelf
             &ShelfPossibleAction
-            &DelFromShelf &DelShelf
+            &DelFromShelf
             &GetBibliosShelves
             &AddShare &AcceptShare &RemoveShare &IsSharedList
     );
@@ -508,24 +508,6 @@ sub DelFromShelf {
     return $t;
 }
 
-=head2 DelShelf
-
-  $Number = DelShelf($shelfnumber);
-
-This function deletes the shelf number, and all of it's content.
-Authorization to do so MUST have been checked before calling, while using
-ShelfPossibleAction with manage parameter.
-
-=cut
-
-sub DelShelf {
-    my ($shelfnumber)= @_;
-    return unless $shelfnumber && $shelfnumber =~ /^\d+$/;
-    my $dbh = C4::Context->dbh;
-    my $sth = $dbh->prepare("DELETE FROM virtualshelves WHERE shelfnumber=?");
-    return $sth->execute($shelfnumber);
-}
-
 =head2 GetBibliosShelves
 
 This finds all the public lists that this bib record is in.
@@ -596,8 +578,8 @@ sub HandleDelBorrower {
     #Instead of deleting we could also disown lists (based on a pref).
     #In that way we could save shared and public lists.
     #The current table constraints support that idea now.
-    #This pref should then govern the results of other routines such as
-    #DelShelf too.
+    #This pref should then govern the results of other routines/methods such as
+    #Koha::Virtualshelf->new->delete too.
 }
 
 =head2 AddShare
diff --git a/C4/VirtualShelves/Page.pm b/C4/VirtualShelves/Page.pm
index 9e3b39d0c1..8d7bb5f94f 100644
--- a/C4/VirtualShelves/Page.pm
+++ b/C4/VirtualShelves/Page.pm
@@ -418,7 +418,7 @@ sub shelfpage {
                     $name = $shelflist->{$number}->{'shelfname'};
                     delete $shelflist->{$number};
                 }
-                unless ( DelShelf($number) ) {
+                unless( Koha::Virtualshelves->find($number)->delete ) {
                     push( @paramsloop, { delete_fail => $name } );
                     last;
                 }
diff --git a/t/db_dependent/VirtualShelves.t b/t/db_dependent/VirtualShelves.t
index 07933c59ab..f4dc31d380 100755
--- a/t/db_dependent/VirtualShelves.t
+++ b/t/db_dependent/VirtualShelves.t
@@ -173,15 +173,15 @@ for my $i (0..9) {
     is(IsSharedList($sh),$n? 1: '', "Checked IsSharedList for shelf $sh");
 }
 
-#----------------TEST DelShelf & DelFromShelf functions------------------------#
+#----------------TEST Koha::Virtualshelf->delete & DelFromShelf functions------------------------#
 
 for my $i (0..9){
     my $shelfnumber = $shelves[$i]->{number};
     if($shelfnumber<0) {
-        ok(1, 'Skip DelShelf for shelf -1');
+        ok(1, 'Skip Koha::Virtualshelf->delete for shelf -1');
         next;
     }
-    my $status = DelShelf($shelfnumber);
+    my $status = Koha::Virtualshelves->find($shelfnumber)->delete;
     is($status, 1, "deleted shelf $shelfnumber and its contents");
 }
 
diff --git a/t/db_dependent/Virtualshelves.t b/t/db_dependent/Virtualshelves.t
index bfd2dc0074..407011fb96 100644
--- a/t/db_dependent/Virtualshelves.t
+++ b/t/db_dependent/Virtualshelves.t
@@ -18,7 +18,7 @@ $dbh->do(q|DELETE FROM virtualshelves|);
 my $builder = t::lib::TestBuilder->new;
 
 subtest 'CRUD' => sub {
-    plan tests => 11;
+    plan tests => 13;
     my $patron = $builder->build({
         source => 'Borrower',
     });
@@ -72,4 +72,9 @@ subtest 'CRUD' => sub {
     )->store;
     $number_of_shelves = Koha::Virtualshelves->search->count;
     is( $number_of_shelves, 2, 'Another patron should be able to create a shelf with an existing shelfname');
+
+    my $is_deleted = Koha::Virtualshelves->find( $shelf->shelfnumber )->delete;
+    is( $is_deleted, 1, 'The shelf has been deleted correctly' );
+    $number_of_shelves = Koha::Virtualshelves->search->count;
+    is( $number_of_shelves, 1, 'To be sure the shelf has been deleted' );
 };