From 514ad9cb75c296e9323d0f6123accaf4d4551bc5 Mon Sep 17 00:00:00 2001
From: Josef Moravec <josef.moravec@gmail.com>
Date: Wed, 17 May 2017 11:36:05 +0000
Subject: [PATCH] Bug 18606: Get rid of AddItemToCollection

---
 C4/RotatingCollections.pm        | 50 ----------------------------------------
 Koha/RotatingCollection.pm       | 33 ++++++++++++++++++++++++++
 rotating_collections/addItems.pl | 28 +++++++++++-----------
 3 files changed, 47 insertions(+), 64 deletions(-)

diff --git a/C4/RotatingCollections.pm b/C4/RotatingCollections.pm
index 521a4e9..b5aebd8 100644
--- a/C4/RotatingCollections.pm
+++ b/C4/RotatingCollections.pm
@@ -48,61 +48,11 @@ BEGIN {
     require Exporter;
     @ISA    = qw( Exporter );
     @EXPORT = qw(
-      AddItemToCollection
       RemoveItemFromCollection
       TransferCollection
     );
 }
 
-=head2 AddItemToCollection
-
- ( $success, $errorcode, $errormessage ) = AddItemToCollection( $colId, $itemnumber );
-
-Adds an item to a rotating collection.
-
- Input:
-   $colId: Collection to add the item to.
-   $itemnumber: Item to be added to the 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 AddItemToCollection {
-    my ( $colId, $itemnumber ) = @_;
-
-    ## Check for all necessary parameters
-    if ( !$colId ) {
-        return ( 0, 1, "NO_ID" );
-    }
-    if ( !$itemnumber ) {
-        return ( 0, 2, "NO_ITEM" );
-    }
-
-    if ( isItemInThisCollection( $itemnumber, $colId ) ) {
-        return ( 0, 2, "IN_COLLECTION" );
-    }
-    elsif ( isItemInAnyCollection($itemnumber) ) {
-        return ( 0, 3, "IN_COLLECTION_OTHER" );
-    }
-
-    my $dbh = C4::Context->dbh;
-
-    my $sth;
-    $sth = $dbh->prepare("
-        INSERT INTO collections_tracking (
-            colId,
-            itemnumber
-        ) VALUES ( ?, ? )
-    ");
-    $sth->execute( $colId, $itemnumber ) or return ( 0, 3, $sth->errstr() );
-
-    return 1;
-
-}
-
 =head2  RemoveItemFromCollection
 
  ( $success, $errorcode, $errormessage ) = RemoveItemFromCollection( $colId, $itemnumber );
diff --git a/Koha/RotatingCollection.pm b/Koha/RotatingCollection.pm
index ba90192..4c8f48b 100644
--- a/Koha/RotatingCollection.pm
+++ b/Koha/RotatingCollection.pm
@@ -22,7 +22,9 @@ use Modern::Perl;
 use Carp;
 
 use Koha::Database;
+use Koha::Exceptions;
 use Koha::Items;
+use Koha::RotatingCollection::Trackings;
 
 use base qw(Koha::Object);
 
@@ -54,6 +56,37 @@ sub items {
     return $items;
 }
 
+=head3 add_item
+
+$collection->add_item( $itemnumber );
+
+throws
+    Koha::Exceptions::MissingParameter
+    Koha::Exceptions::DuplicateObject
+    Koha::Exceptions::ObjectNotFound
+
+=cut
+
+sub add_item {
+    my ( $self, $itemnumber ) = @_;
+
+    Koha::Exceptions::MissingParameter->throw if not defined $itemnumber;
+
+    Koha::Exceptions::DuplicateObject->throw
+        if Koha::RotatingCollection::Trackings->search( { itemnumber => $itemnumber } )->count;
+
+    Koha::Exceptions::ObjectNotFound->throw if not Koha::Items->find( $itemnumber );
+
+    my $col_tracking = Koha::RotatingCollection::Tracking->new(
+        {
+            colId => $self->colId,
+            itemnumber => $itemnumber,
+        }
+    )->store;
+
+    return $col_tracking;
+}
+
 =head3 type
 
 =cut
diff --git a/rotating_collections/addItems.pl b/rotating_collections/addItems.pl
index 8ef4645..f07f64a 100755
--- a/rotating_collections/addItems.pl
+++ b/rotating_collections/addItems.pl
@@ -41,9 +41,13 @@ my ( $template, $loggedinuser, $cookie ) = get_template_and_user(
     }
 );
 
+my @errors;
+my @messages;
+my $colId = $query->param('colId');
+my $collection = Koha::RotatingCollections->find($colId);
+
 if ( $query->param('action') eq 'addItem' ) {
     ## Add the given item to the collection
-    my $colId      = $query->param('colId');
     my $barcode    = $query->param('barcode');
     my $removeItem = $query->param('removeItem');
     my $itemnumber = GetItemnumberFromBarcode($barcode);
@@ -53,20 +57,17 @@ if ( $query->param('action') eq 'addItem' ) {
     $template->param( barcode => $barcode );
 
     if ( !$removeItem ) {
-        ( $success, $errorCode, $errorMessage ) =
-          AddItemToCollection( $colId, $itemnumber );
+        my $added = eval { $collection->add_item( $itemnumber ) };
+
+        if ( $@ or not $added ) {
+            push @errors, { code => 'error_adding_item' };
+        } else {
+            push @messages, { code => 'success_adding_item' };
+        }
 
         $template->param(
             previousActionAdd => 1,
         );
-
-        if ($success) {
-            $template->param( addSuccess => 1 );
-        }
-        else {
-            $template->param( addFailure     => 1 );
-            $template->param( failureMessage => $errorMessage );
-        }
     }
     else {
         ## Remove the given item from the collection
@@ -89,11 +90,10 @@ if ( $query->param('action') eq 'addItem' ) {
     }
 }
 
-my $colId = $query->param('colId');
-my $collection = Koha::RotatingCollections->find($colId);
-
 $template->param(
     collection => $collection,
+    messages   => \@messages,
+    errors     => \@errors,
 );
 
 output_html_with_http_headers $query, $cookie, $template->output;
-- 
2.7.4