From 9094977c7495f2a572feffd8c2fbcc9470f22be1 Mon Sep 17 00:00:00 2001
From: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Date: Tue, 24 Mar 2020 07:46:10 +0000
Subject: [PATCH] Bug 24446: Update StockRotationItem to use daterequested
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

This patch updates the stockrotation functionality to utilise the new
branchtransfers.daterequested field to more accurately track the state
of stockrotation transfers.

We also opt to start using the new Koha::Item and Koha::Item::Transfer
methods to achieve this goal.

Test plan
0/ Setup stockrotation and configure some rotas and assign items to
them.
1/ Run the stockrotation cronscript
2/ Check the database for branchtransfers initiated by stockrotation and
confirm that that now populate the daterequested field and not the
datesent.
3/ Run the stockrotation tests and verify they pass
4/ Signoff

Signed-off-by: Kathleen Milne <kathleen.milne@cne-siar.gov.uk>
Signed-off-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi>
---
 Koha/StockRotationItem.pm | 101 +++++++++++++++++++++-----------------
 1 file changed, 55 insertions(+), 46 deletions(-)

diff --git a/Koha/StockRotationItem.pm b/Koha/StockRotationItem.pm
index 43d949acaa..4b9325790a 100644
--- a/Koha/StockRotationItem.pm
+++ b/Koha/StockRotationItem.pm
@@ -150,17 +150,20 @@ that it may return to it's stage.branch to continue its rota as normal.
 
 sub repatriate {
     my ( $self, $msg ) = @_;
+
     # Create the transfer.
-    my $transfer_stored = Koha::Item::Transfer->new({
-        'itemnumber' => $self->itemnumber_id,
-        'frombranch' => $self->itemnumber->holdingbranch,
-        'tobranch'   => $self->stage->branchcode_id,
-        'datesent'   => dt_from_string(),
-        'comments'   => $msg,
-        'reason'     => "StockrotationRepatriation"
-    })->store;
-    $self->itemnumber->homebranch($self->stage->branchcode_id)->store;
-    return $transfer_stored;
+    my $transfer = $self->itemnumber->request_transfer(
+        {
+            to      => $self->stage->branchcode,
+            reason  => "StockrotationRepatriation",
+            comment => $msg
+        }
+    );
+
+    # Ensure the homebranch is still in sync with the rota stage
+    $self->itemnumber->homebranch( $self->stage->branchcode_id )->store;
+
+    return $transfer;
 }
 
 =head3 advance
@@ -180,49 +183,55 @@ advance the item as per usual.
 =cut
 
 sub advance {
-    my ( $self ) = @_;
-    my $item = $self->itemnumber;
-    my $transfer = Koha::Item::Transfer->new({
-        'itemnumber' => $self->itemnumber_id,
-        'frombranch' => $item->holdingbranch,
-        'datesent'   => dt_from_string(),
-        'reason'     => "StockrotationAdvance"
-    });
-
+    my ($self)         = @_;
+    my $item           = $self->itemnumber;
+    my $current_branch = $item->holdingbranch;
+    my $transfer;
+
+    # Find and interpret our stage
+    my $stage = $self->stage;
+    my $new_stage;
     if ( $self->indemand && !$self->fresh ) {
-        $self->indemand(0)->store;  # De-activate indemand
-        $transfer->tobranch($self->stage->branchcode_id);
-        $transfer->datearrived(dt_from_string());
-    } else {
-        # Find and update our stage.
-        my $stage = $self->stage;
-        my $new_stage;
-        if ( $self->fresh ) {   # Just added to rota
+        $self->indemand(0)->store;                          # De-activate indemand
+        $new_stage = $stage;
+    }
+    else {
+        # New to rota?
+        if ( $self->fresh ) {
             $new_stage = $self->stage->first_sibling || $self->stage;
-            $transfer->tobranch($new_stage->branchcode_id);
-            $transfer->datearrived(dt_from_string()) # Already at first branch
-                if $item->holdingbranch eq $new_stage->branchcode_id;
-            $self->fresh(0)->store;         # Reset fresh
-        } elsif ( !$stage->last_sibling ) { # Last stage
-            if ( $stage->rota->cyclical ) { # Cyclical rota?
-                # Revert to first stage.
-                $new_stage = $stage->first_sibling || $stage;
-                $transfer->tobranch($new_stage->branchcode_id);
-                $transfer->datearrived(dt_from_string());
-            } else {
-                $self->delete;  # StockRotationItem is done.
+            $self->fresh(0)->store;                         # Reset fresh
+        }
+        # Last stage?
+        elsif ( !$stage->last_sibling ) {
+            # Cyclical rota?
+            if ( $stage->rota->cyclical ) {
+                $new_stage =
+                  $stage->first_sibling || $stage;           # Revert to first stage.
+            }
+            else {
+                $self->delete;                               # StockRotationItem is done.
                 return 1;
             }
-        } else {
-            # Just advance.
-            $new_stage = $self->stage->next_sibling;
         }
-        $self->stage_id($new_stage->stage_id)->store;        # Set new stage
-        $item->homebranch($new_stage->branchcode_id)->store; # Update homebranch
-        $transfer->tobranch($new_stage->branchcode_id);      # Send to new branch
+        else {
+            $new_stage = $self->stage->next_sibling;         # Just advance
+        }
     }
 
-    return $transfer->store;
+    # Update stage and record transfer
+    $self->stage_id( $new_stage->stage_id )->store;          # Set new stage
+    $item->homebranch( $new_stage->branchcode_id )->store;   # Update homebranch
+    $transfer = $item->request_transfer(
+        {
+            to            => $new_stage->branchcode,
+            reason        => "StockrotationAdvance",
+            ignore_limits => 1
+        }
+    );                                                       # Add transfer
+    $transfer->receive
+      if $item->holdingbranch eq $new_stage->branchcode_id;  # Already at branch
+
+    return $transfer;
 }
 
 =head3 investigate
-- 
2.20.1