From 0c5dc436ce94f08bf09c90ac552db1a848d1321a Mon Sep 17 00:00:00 2001 From: Martin Renvoize 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 | 67 +++++++++++++++++++++-------- 1 file changed, 50 insertions(+), 17 deletions(-) diff --git a/t/db_dependent/StockRotationItems.t b/t/db_dependent/StockRotationItems.t index 707842e4a5..1829a9b61f 100644 --- 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; @@ -173,7 +174,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; @@ -186,7 +187,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; @@ -195,10 +196,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."); @@ -206,7 +207,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 +260,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; }; @@ -305,8 +338,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."); @@ -321,8 +354,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; @@ -340,8 +373,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; @@ -360,8 +393,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; @@ -380,8 +413,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