From 62df4804a382074bf9ed208d110c2c92e6e1b22f Mon Sep 17 00:00:00 2001 From: Martin Renvoize Date: Fri, 18 Dec 2020 16:36:20 +0000 Subject: [PATCH] Bug 24446: Improve StockRotationItem->advance tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. Signed-off-by: Kathleen Milne Signed-off-by: Joonas Kylmälä --- t/db_dependent/StockRotationItems.t | 67 +++++++++++++++++++++-------- 1 file changed, 50 insertions(+), 17 deletions(-) diff --git a/t/db_dependent/StockRotationItems.t b/t/db_dependent/StockRotationItems.t index 49da2cc304..a1917d5f54 100755 --- a/t/db_dependent/StockRotationItems.t +++ b/t/db_dependent/StockRotationItems.t @@ -22,6 +22,7 @@ use Modern::Perl; use DateTime; use DateTime::Duration; use Koha::Database; +use Koha::DateUtils; use Koha::Item::Transfer; use t::lib::TestBuilder; use Test::Warn; @@ -196,7 +197,7 @@ subtest "Tests for needs_advancing." => sub { 'itemnumber' => $dbitem->itemnumber_id, 'frombranch' => $dbitem->stage->branchcode_id, 'tobranch' => $dbitem->stage->branchcode_id, - 'datesent' => DateTime->now, + 'datesent' => dt_from_string(), 'datearrived' => undef, 'reason' => "StockrotationAdvance", })->store; @@ -209,7 +210,7 @@ subtest "Tests for needs_advancing." => sub { $dbitem->fresh(0)->store; # Test item will not be advanced if it has not spent enough time. - $dbtransfer->datearrived(DateTime->now)->store; + $dbtransfer->datearrived(dt_from_string())->store; is($dbitem->needs_advancing, 0, "Not ready to advance: Not spent enough time."); # Test item will be advanced if it has not spent enough time, but is fresh. $dbitem->fresh(1)->store; @@ -218,10 +219,10 @@ subtest "Tests for needs_advancing." => sub { # Test item will be advanced if it has spent enough time. $dbtransfer->datesent( # Item was sent 100 days ago... - DateTime->now - DateTime::Duration->new( days => 100 ) + dt_from_string() - DateTime::Duration->new( days => 100 ) )->store; $dbtransfer->datearrived( # And arrived 75 days ago. - DateTime->now - DateTime::Duration->new( days => 75 ) + dt_from_string() - DateTime::Duration->new( days => 75 ) )->store; is($dbitem->needs_advancing, 1, "Ready to be advanced."); $dbtransfer->delete; @@ -231,7 +232,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( @@ -289,22 +290,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(dt_from_string())->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(dt_from_string())->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; + $intransfer->datearrived(dt_from_string())->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; }; @@ -356,8 +389,8 @@ subtest "Tests for investigate (singular)." => sub { 'itemnumber' => $dbitem->itemnumber_id, 'frombranch' => $dbitem->itemnumber->homebranch, 'tobranch' => $dbitem->itemnumber->homebranch, - 'datesent' => DateTime->now, - 'datearrived' => DateTime->now, + 'datesent' => dt_from_string(), + 'datearrived' => dt_from_string(), 'reason' => "StockrotationAdvance", })->store; is($dbitem->investigate->{reason}, 'repatriation', "older item repatriates."); @@ -377,8 +410,8 @@ subtest "Tests for investigate (singular)." => sub { 'itemnumber' => $dbitem->itemnumber_id, 'frombranch' => $dbitem->itemnumber->homebranch, 'tobranch' => $dbitem->stage->branchcode_id, - 'datesent' => DateTime->now, - 'datearrived' => DateTime->now, + 'datesent' => dt_from_string(), + 'datearrived' => dt_from_string(), 'reason' => "StockrotationAdvance", })->store; $dbitem->itemnumber->homebranch($dbitem->stage->branchcode_id)->store; @@ -404,8 +437,8 @@ subtest "Tests for investigate (singular)." => sub { 'itemnumber' => $dbitem->itemnumber_id, 'frombranch' => $dbitem->itemnumber->homebranch, 'tobranch' => $dbitem->stage->branchcode_id, - 'datesent' => DateTime->now - $sent_duration, - 'datearrived' => DateTime->now - $arrived_duration, + 'datesent' => dt_from_string() - $sent_duration, + 'datearrived' => dt_from_string() - $arrived_duration, 'reason' => "StockrotationAdvance", })->store; $dbitem->itemnumber->homebranch($dbitem->stage->branchcode_id)->store; @@ -432,8 +465,8 @@ subtest "Tests for investigate (singular)." => sub { 'itemnumber' => $dbitem->itemnumber_id, 'frombranch' => $dbitem->itemnumber->homebranch, 'tobranch' => $dbitem->stage->branchcode_id, - 'datesent' => DateTime->now - $sent_duration, - 'datearrived' => DateTime->now - $arrived_duration, + 'datesent' => dt_from_string() - $sent_duration, + 'datearrived' => dt_from_string() - $arrived_duration, 'reason' => "StockrotationAdvance", })->store; $dbitem->itemnumber->homebranch($dbitem->stage->branchcode_id)->store; @@ -460,8 +493,8 @@ subtest "Tests for investigate (singular)." => sub { 'itemnumber' => $dbitem->itemnumber_id, 'frombranch' => $dbitem->itemnumber->homebranch, 'tobranch' => $dbitem->stage->branchcode_id, - 'datesent' => DateTime->now - $sent_duration, - 'datearrived' => DateTime->now - $arrived_duration, + 'datesent' => dt_from_string() - $sent_duration, + 'datearrived' => dt_from_string() - $arrived_duration, 'reason' => "StockrotationAdvance", })->store; is($dbitem->investigate->{reason}, 'repatriation', -- 2.20.1