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. Signed-off-by: Kathleen Milne <kathleen.milne@cne-siar.gov.uk> Signed-off-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
This commit is contained in:
Martin Renvoize
Jonathan Druart
parent
99e3bb18b0
commit
cf8ce2aa13
|
|
@ -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.");
|
||||
|
||||
$dbstage->rota->cyclical(0)->store; # Set Rota to non-cyclical.
|
||||
# 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(DateTime->now)->store;
|
||||
$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(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',
|
||||
|
|
|
|||
Loading…
Reference in New Issue