From ff1d512d5a745776e48416f2aff10c72ec946b1f Mon Sep 17 00:00:00 2001
From: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Date: Tue, 6 Oct 2020 14:11:06 +0100
Subject: [PATCH] Bug 26618: Remove use of transferbook in RotatingCollections

This patch replaces the use of C4::Circulation::transferbook in
C4::RotatingCollections with calls to Koha::Item->request_transfer and
adds handling for the various failure cases which that can throw.
---
 C4/RotatingCollections.pm                     | 72 +++++++++++++++----
 .../transferCollection.tt                     | 15 ++++
 rotating_collections/transferCollection.pl    | 10 ++-
 3 files changed, 76 insertions(+), 21 deletions(-)

diff --git a/C4/RotatingCollections.pm b/C4/RotatingCollections.pm
index c41d5780e9..af808bec53 100644
--- a/C4/RotatingCollections.pm
+++ b/C4/RotatingCollections.pm
@@ -407,8 +407,7 @@ Transfers a collection to another branch
 
  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
+   $messages: Arrayref of messages for user feedback
 
 =cut
 
@@ -432,7 +431,7 @@ sub TransferCollection {
                         colBranchcode = ? 
                         WHERE colId = ?"
     );
-    $sth->execute( $colBranchcode, $colId ) or return ( 0, 4, $sth->errstr() );
+    $sth->execute( $colBranchcode, $colId ) or return 0;
 
     $sth = $dbh->prepare(q{
         SELECT items.itemnumber, items.barcode FROM collections_tracking
@@ -441,21 +440,64 @@ sub TransferCollection {
         WHERE issues.borrowernumber IS NULL
           AND collections_tracking.colId = ?
     });
-    $sth->execute($colId) or return ( 0, 4, $sth->errstr );
-    my @results;
+    $sth->execute($colId) or return 0;
+    my $messages;
     while ( my $item = $sth->fetchrow_hashref ) {
-        my ($status) = CheckReserves( $item->{itemnumber} );
-        my @transfers = C4::Circulation::GetTransfers( $item->{itemnumber} );
-        C4::Circulation::transferbook({
-            from_branch => $item->holdingbranch,
-            to_branch => $colBranchcode,
-            barcode => $item->{barcode},
-            ignore_reserves => 1,
-            trigger => 'RotatingCollection'
-        }) unless ( $status eq 'Waiting' || @transfers );
+        my $item_object = Koha::Items->find( $item->{itemnumber} );
+        try {
+            $item_object->request_transfer(
+                {
+                    to            => $colBranchcode,
+                    reason        => 'RotatingCollection',
+                    ignore_limits => 0
+                }
+            );    # Request transfer
+        }
+        catch {
+            if ( $_->isa('Koha::Exceptions::Item::Transfer::Found') ) {
+                my $exception      = $_;
+                my $found_transfer = $_->transfer;
+                if (   $found_transfer->in_transit
+                    || $found_transfer->reason eq 'Reserve' )
+                {
+                    my $transfer = $item_object->request_transfer(
+                        {
+                            to            => $colBranchcode,
+                            reason        => "RotatingCollection",
+                            ignore_limits => 0,
+                            enqueue       => 1
+                        }
+                    );    # Queue transfer
+                    push @{$messages},
+                      {
+                        type           => 'enqueu',
+                        item           => $item_object,
+                        found_transfer => $found_transfer
+                      };
+                }
+                else {
+                    my $transfer = $item->request_transfer(
+                        {
+                            to            => $colBranchcode,
+                            reason        => "RotatingCollection",
+                            ignore_limits => 0,
+                            replace       => 1
+                        }
+                    );    # Replace transfer
+                    # NOTE: If we just replaced a StockRotationAdvance, 
+                    # it will get enqueued afresh on the next cron run
+                }
+            }
+            elsif ( $_->isa('Koha::Exceptions::Item::Transfer::Limit') ) {
+                push @{$messages}, { type => 'failure', item => $item_object };
+            }
+            else {
+                $_->rethrow();
+            }
+        };
     }
 
-    return 1;
+    return (1, $messages);
 }
 
 =head2 GetCollectionItemBranches
diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/rotating_collections/transferCollection.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/rotating_collections/transferCollection.tt
index af41ccbc85..5da9e013f8 100644
--- a/koha-tmpl/intranet-tmpl/prog/en/modules/rotating_collections/transferCollection.tt
+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/rotating_collections/transferCollection.tt
@@ -19,6 +19,21 @@
 
                 <h1>Transfer collection <em>[% colTitle | html %]</em></h1>
 
+                [% IF ( messages ) %]
+                    [% FOREACH message IN messages %]
+                        [%- SWITCH message.type -%]
+                            [%- CASE 'failure' %]
+                            <div class="dialog error">
+                                <p>Cannot transfer item [% message.item.itemnumber | html %] due to transfer limits</p>            
+                            </div>
+                            [%- CASE 'enqueu' -%]
+                            <div class="dialog message">
+                                <p>Item [% message.item.itemnumber %] queued behind transfer to [% message.found_transfer.to | html %]</p>            
+                            </div>
+                       [% END %]
+                    [% END %]
+                [% END %]
+
                 [% IF ( transferSuccess ) %]
                     <div class="dialog message">
                         <p>Collection transferred successfully</p>
diff --git a/rotating_collections/transferCollection.pl b/rotating_collections/transferCollection.pl
index bdee62a83e..11859e5f9d 100755
--- a/rotating_collections/transferCollection.pl
+++ b/rotating_collections/transferCollection.pl
@@ -41,19 +41,17 @@ my ( $template, $loggedinuser, $cookie ) = get_template_and_user(
 );
 
 ## Transfer collection
-my ( $success, $errorCode, $errorMessage );
+my ( $success, $messages );
 if ($toBranch) {
-    ( $success, $errorCode, $errorMessage ) =
+    $success =
       TransferCollection( $colId, $toBranch );
 
     if ($success) {
-        $template->param( transferSuccess => 1 );
+        $template->param( transferSuccess => 1, messages => $messages );
     }
     else {
         $template->param(
-            transferFailure => 1,
-            errorCode       => $errorCode,
-            errorMessage    => $errorMessage
+            transferFailure => 1
         );
     }
 }
-- 
2.20.1