From bda15dbf71653d80b7005060fb43d459908e1106 Mon Sep 17 00:00:00 2001
From: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Date: Tue, 16 Jun 2020 20:38:58 +0100
Subject: [PATCH] Bug 24446: Improve StockRotationItem->advance tests

This patch adds additional checks in the tests for item homebranch
updates and cyclical transfers caused by the StockRotationItem->advance
method.

It highlights a bug in the original logic whereby a cyclical rotation
would always result in a completed transfer but the physical item would
not have actually been moved between branches.
---
 t/db_dependent/StockRotationItems.t | 36 +++++++++++++++++++++++++++--
 1 file changed, 34 insertions(+), 2 deletions(-)

diff --git a/t/db_dependent/StockRotationItems.t b/t/db_dependent/StockRotationItems.t
index 707842e4a5..89f3b7c1bf 100644
--- a/t/db_dependent/StockRotationItems.t
+++ b/t/db_dependent/StockRotationItems.t
@@ -206,7 +206,7 @@ subtest "Tests for needs_advancing." => sub {
 };
 
 subtest "Tests for advance." => sub {
-    plan tests => 15;
+    plan tests => 23;
     $schema->storage->txn_begin;
 
     my $sritem = $builder->build({
@@ -259,22 +259,54 @@ subtest "Tests for advance." => sub {
     $dbitem = Koha::StockRotationItems->find($sritem->{itemnumber_id});
     ## Test results
     is($dbitem->stage->stage_id, $dbstage2->stage_id, "Stage updated.");
+    is(
+        $dbitem->itemnumber->homebranch,
+        $dbstage2->branchcode_id,
+        "Item homebranch updated"
+    );
     my $intransfer = $dbitem->itemnumber->get_transfer;
     is($intransfer->frombranch, $dbstage->branchcode_id, "Origin correct.");
     is($intransfer->tobranch, $dbstage2->branchcode_id, "Target Correct.");
 
+    # Arrive at new branch
+    $intransfer->datearrived(DateTime->now)->store;
+    $dbitem->itemnumber->holdingbranch($srstage->{branchcode_id})->store;
+
+    # Test a cyclical advance
+    ok($dbitem->advance, "Cyclical advancement done.");
+    ## Refetch dbitem
+    $dbitem = Koha::StockRotationItems->find($sritem->{itemnumber_id});
+    ## Test results
+    is($dbitem->stage->stage_id, $dbstage->stage_id, "Stage updated.");
+    is(
+        $dbitem->itemnumber->homebranch,
+        $dbstage->branchcode_id,
+        "Item homebranch updated"
+    );
+    $intransfer = $dbitem->itemnumber->get_transfer;
+    is($intransfer->frombranch, $dbstage2->branchcode_id, "Origin correct.");
+    is($intransfer->tobranch, $dbstage->branchcode_id, "Target correct.");
+
+    # Arrive at new branch
+    $intransfer->datearrived(DateTime->now)->store;
+    $dbitem->itemnumber->holdingbranch($srstage->{branchcode_id})->store;
+
     $dbstage->rota->cyclical(0)->store;         # Set Rota to non-cyclical.
 
+    # Advance again, to end of rota.
+    ok($dbitem->advance, "Non-cyclical advance to last stage.");
+
     # Arrive at new branch
     $intransfer->datearrived(DateTime->now)->store;
     $dbitem->itemnumber->holdingbranch($srstage->{branchcode_id})->store;
-    $dbitem->itemnumber->homebranch($srstage->{branchcode_id})->store;
 
     # Advance again, Remove from rota.
     ok($dbitem->advance, "Non-cyclical advance.");
     ## Refetch dbitem
     $dbitem = Koha::StockRotationItems->find($sritem->{itemnumber_id});
     is($dbitem, undef, "StockRotationItem has been removed.");
+    my $item = Koha::Items->find($sritem->{itemnumber_id});
+    is($item->homebranch, $srstage->{branchcode_id}, "Item homebranch remains");
 
     $schema->storage->txn_rollback;
 };
-- 
2.20.1