From 1d76e00dcb63ece1912b1436f22db526ed7a96d9 Mon Sep 17 00:00:00 2001
From: Josef Moravec <josef.moravec@gmail.com>
Date: Wed, 17 May 2017 11:49:02 +0000
Subject: [PATCH] Bug 18606: Get rid of RemoveItemFromCollection

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
---
 C4/RotatingCollections.pm        | 42 ----------------------------------------
 Koha/RotatingCollection.pm       | 28 +++++++++++++++++++++++++++
 rotating_collections/addItems.pl | 28 +++++++++------------------
 3 files changed, 37 insertions(+), 61 deletions(-)

diff --git a/C4/RotatingCollections.pm b/C4/RotatingCollections.pm
index b5aebd8..466eca5 100644
--- a/C4/RotatingCollections.pm
+++ b/C4/RotatingCollections.pm
@@ -48,52 +48,10 @@ BEGIN {
     require Exporter;
     @ISA    = qw( Exporter );
     @EXPORT = qw(
-      RemoveItemFromCollection
       TransferCollection
     );
 }
 
-=head2  RemoveItemFromCollection
-
- ( $success, $errorcode, $errormessage ) = RemoveItemFromCollection( $colId, $itemnumber );
-
-Removes an item to a collection
-
- Input:
-   $colId: Collection to add the item to.
-   $itemnumber: Item to be removed from collection
-
- Output:
-   $success: 1 if all database operations were successful, 0 otherwise
-   $errorCode: Code for reason of failure, good for translating errors in templates
-   $errorMessage: English description of error
-
-=cut
-
-sub RemoveItemFromCollection {
-    my ( $colId, $itemnumber ) = @_;
-
-    ## Check for all necessary parameters
-    if ( !$itemnumber ) {
-        return ( 0, 2, "NO_ITEM" );
-    }
-
-    if ( !isItemInThisCollection( $itemnumber, $colId ) ) {
-        return ( 0, 2, "NOT_IN_COLLECTION" );
-    }
-
-    my $dbh = C4::Context->dbh;
-
-    my $sth;
-    $sth = $dbh->prepare(
-        "DELETE FROM collections_tracking
-                        WHERE itemnumber = ?"
-    );
-    $sth->execute($itemnumber) or return ( 0, 3, $sth->errstr() );
-
-    return 1;
-}
-
 =head2 TransferCollection
 
  ( $success, $errorcode, $errormessage ) = TransferCollection( $colId, $colBranchcode );
diff --git a/Koha/RotatingCollection.pm b/Koha/RotatingCollection.pm
index dbc97ba..c00f80e 100644
--- a/Koha/RotatingCollection.pm
+++ b/Koha/RotatingCollection.pm
@@ -87,6 +87,34 @@ sub add_item {
     return $col_tracking;
 }
 
+=head3 remove_item
+
+$collection->remove_item( $item_object )
+
+throws
+    Koha::Exceptions::MissingParameter
+    Koha::Exceptions::ObjectNotFound
+
+=cut
+
+sub remove_item {
+    my ( $self, $item ) = @_;
+
+    Koha::Exceptions::MissingParameter->throw if not defined $item;
+
+    Koha::Exceptions::ObjectNotFound->throw if ref($item) ne 'Koha::Item';
+
+    my $collection_tracking = Koha::RotatingCollection::Trackings->find(
+        {
+            itemnumber => $item->itemnumber,
+            colId      => $self->colId,
+        } );
+
+    Koha::Exceptions::ObjectNotFound->throw if not defined $collection_tracking;
+
+    return $collection_tracking->delete;
+}
+
 =head3 type
 
 =cut
diff --git a/rotating_collections/addItems.pl b/rotating_collections/addItems.pl
index c0ef9a7..edcbf0a 100755
--- a/rotating_collections/addItems.pl
+++ b/rotating_collections/addItems.pl
@@ -65,29 +65,19 @@ if ( $query->param('action') eq 'addItem' ) {
         } else {
             push @messages, { code => 'success_adding_item' };
         }
-
-        $template->param(
-            previousActionAdd => 1,
-        );
-    }
-    else {
+    } else {
         ## Remove the given item from the collection
-        ( $success, $errorCode, $errorMessage ) =
-          RemoveItemFromCollection( $colId, $itemnumber );
+        my $deleted = eval { $collection->remove_item( $item ) };
 
-        $template->param(
-            previousActionRemove => 1,
-            removeChecked        => 1,
-        );
-
-        if ($success) {
-            $template->param( removeSuccess => 1 );
-        }
-        else {
-            $template->param( removeFailure  => 1 );
-            $template->param( failureMessage => $errorMessage );
+        if ( $@ or not $deleted ) {
+            push @errors, { code => 'error_removing_item' };
+        } else {
+            push @messages, { code => 'success_removing_item' };
         }
 
+        $template->param(
+            removeChecked => 1,
+        );
     }
 }
 
-- 
2.10.2