Bug 27064: (QA follow-up) Pass 'replace' through to Koha::Item::Transfer->cancel

The `replace` option found in Koha::Item->request_transfer should be
passed through to Koha::Item::Transfer->cancel and prevent any reverse
transfers from being queued in such cases.

This prevents modItemTransfer from adding superflous reverse transfers
whenever it is used.

Test plan
1/ Run t/db_dependent/Koha/Item.t to prove the new tests pass

Signed-off-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
This commit is contained in:
Martin Renvoize 2021-05-06 16:31:39 +01:00 committed by Kyle M Hall
parent 8b095aaf37
commit 6c1b7df3f6
3 changed files with 27 additions and 5 deletions

View file

@ -467,7 +467,7 @@ sub request_transfer {
} }
)->store(); )->store();
$request->cancel( { reason => $params->{reason}, force => 1 } ) $request->cancel( { reason => $params->{reason}, force => 1, replace => 1 } )
if ( defined($request) && $params->{replace} ); if ( defined($request) && $params->{replace} );
return $transfer; return $transfer;

View file

@ -159,9 +159,10 @@ sub cancel {
->store; ->store;
# Set up return transfer if transfer was force cancelled whilst in transit # Set up return transfer if transfer was force cancelled whilst in transit
# and we were not notified that the transfer is being replaced.
# NOTE: We don't catch here, as we're happy to fail if there are already # NOTE: We don't catch here, as we're happy to fail if there are already
# other transfers in the queue. # other transfers in the queue.
if ($in_transit) { if ($in_transit && !$params->{replace}) {
try { try {
$self->item->request_transfer( $self->item->request_transfer(
{ to => $self->from_library, reason => 'TransferCancellation' } ); { to => $self->from_library, reason => 'TransferCancellation' } );

View file

@ -459,7 +459,7 @@ subtest 'pickup_locations' => sub {
}; };
subtest 'request_transfer' => sub { subtest 'request_transfer' => sub {
plan tests => 7; plan tests => 13;
$schema->storage->txn_begin; $schema->storage->txn_begin;
my $library1 = $builder->build_object( { class => 'Koha::Libraries' } ); my $library1 = $builder->build_object( { class => 'Koha::Libraries' } );
@ -485,6 +485,7 @@ subtest 'request_transfer' => sub {
is( ref($transfer), 'Koha::Item::Transfer', is( ref($transfer), 'Koha::Item::Transfer',
'Koha::Item->request_transfer should return a Koha::Item::Transfer object' 'Koha::Item->request_transfer should return a Koha::Item::Transfer object'
); );
my $original_transfer = $transfer->get_from_storage;
# Transfer already in progress # Transfer already in progress
throws_ok { $item->request_transfer( { to => $library2, reason => 'Manual' } ) } throws_ok { $item->request_transfer( { to => $library2, reason => 'Manual' } ) }
@ -496,7 +497,27 @@ subtest 'request_transfer' => sub {
'Koha::Item::Transfer', 'Koha::Item::Transfer',
'The exception contains the found Koha::Item::Transfer' ); 'The exception contains the found Koha::Item::Transfer' );
$transfer->datearrived(dt_from_string)->store(); # Queue transfer
my $queued_transfer = $item->request_transfer(
{ to => $library2, reason => 'Manual', enqueue => 1 } );
is( ref($queued_transfer), 'Koha::Item::Transfer',
'Koha::Item->request_transfer allowed when enqueue is set' );
my $transfers = $item->get_transfers;
is($transfers->count, 2, "There are now 2 live transfers in the queue");
$transfer = $transfer->get_from_storage;
is_deeply($transfer->unblessed, $original_transfer->unblessed, "Original transfer unchanged");
$queued_transfer->datearrived(dt_from_string)->store();
# Replace transfer
my $replaced_transfer = $item->request_transfer(
{ to => $library2, reason => 'Manual', replace => 1 } );
is( ref($replaced_transfer), 'Koha::Item::Transfer',
'Koha::Item->request_transfer allowed when replace is set' );
$original_transfer->discard_changes;
ok($original_transfer->datecancelled, "Original transfer cancelled");
$transfers = $item->get_transfers;
is($transfers->count, 1, "There is only 1 live transfer in the queue");
$replaced_transfer->datearrived(dt_from_string)->store();
# BranchTransferLimits # BranchTransferLimits
t::lib::Mocks::mock_preference('UseBranchTransferLimits', 1); t::lib::Mocks::mock_preference('UseBranchTransferLimits', 1);
@ -513,7 +534,7 @@ subtest 'request_transfer' => sub {
my $forced_transfer = $item->request_transfer( { to => $library1, reason => 'Manual', ignore_limits => 1 } ); my $forced_transfer = $item->request_transfer( { to => $library1, reason => 'Manual', ignore_limits => 1 } );
is( ref($forced_transfer), 'Koha::Item::Transfer', is( ref($forced_transfer), 'Koha::Item::Transfer',
'Koha::Item->request_transfer allowed when forced' 'Koha::Item->request_transfer allowed when ignore_limits is set'
); );
$schema->storage->txn_rollback; $schema->storage->txn_rollback;