Bug 35100: Revert change to request_transfer
This patch reverts the change to request_transfer, opting to tackle the StockRotationAdvance requirement to stay in place in ModItemTransfer itself. We also add a FIXME to RotatingCollections.. I'll look to removing that on another bug to reduce the scope of this one. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
This commit is contained in:
parent
d64a206a25
commit
bcb4ac59d4
GPG key ID:
422B469130441A0F
5 changed files with 81 additions and 34 deletions
56
C4/Items.pm
56
C4/Items.pm
|
|
@ -54,6 +54,7 @@ use C4::Log qw( logaction );
|
|||
use List::MoreUtils qw( any );
|
||||
use DateTime::Format::MySQL;
|
||||
# debugging; so please don't remove this
|
||||
use Try::Tiny qw( catch try );
|
||||
|
||||
use Koha::AuthorisedValues;
|
||||
use Koha::DateUtils qw( dt_from_string );
|
||||
|
|
@ -359,31 +360,64 @@ The last optional parameter allows for passing skip_record_index through to the
|
|||
sub ModItemTransfer {
|
||||
my ( $itemnumber, $frombranch, $tobranch, $trigger, $params ) = @_;
|
||||
|
||||
my $dbh = C4::Context->dbh;
|
||||
my $item = Koha::Items->find( $itemnumber );
|
||||
my $dbh = C4::Context->dbh;
|
||||
my $item = Koha::Items->find($itemnumber);
|
||||
|
||||
# NOTE: This retains the existing hard coded behaviour by ignoring transfer limits
|
||||
# and always replacing any existing transfers. (In theory, calls to ModItemTransfer
|
||||
# will have been preceded by a check of branch transfer limits)
|
||||
my $to_library = Koha::Libraries->find($tobranch);
|
||||
my $transfer = $item->request_transfer(
|
||||
{
|
||||
to => $to_library,
|
||||
reason => $trigger,
|
||||
ignore_limits => 1,
|
||||
replace => 1
|
||||
my $transfer;
|
||||
try {
|
||||
$transfer = $item->request_transfer(
|
||||
{
|
||||
to => $to_library,
|
||||
reason => $trigger,
|
||||
ignore_limits => 1,
|
||||
}
|
||||
);
|
||||
} catch {
|
||||
if ( $_->isa('Koha::Exceptions::Item::Transfer::InQueue') ) {
|
||||
my $exception = $_;
|
||||
my $found_transfer = $_->transfer;
|
||||
|
||||
# If StockRotationAdvance, leave in place but ensure transit state is reset
|
||||
if ( $found_transfer->reason eq 'StockrotationAdvance' ) {
|
||||
$transfer = $item->request_transfer(
|
||||
{
|
||||
to => $to_library,
|
||||
reason => $trigger,
|
||||
ignore_limits => 1,
|
||||
enqueue => 1
|
||||
}
|
||||
);
|
||||
|
||||
# Ensure transit is reset
|
||||
$found_transfer->datesent(undef)->store;
|
||||
} else {
|
||||
$transfer = $item->request_transfer(
|
||||
{
|
||||
to => $to_library,
|
||||
reason => $trigger,
|
||||
ignore_limits => 1,
|
||||
replace => 1
|
||||
}
|
||||
);
|
||||
}
|
||||
} else {
|
||||
$_->rethrow();
|
||||
}
|
||||
);
|
||||
};
|
||||
|
||||
# Immediately set the item to in transit if it is checked in
|
||||
if ( !$item->checkout ) {
|
||||
$item->holdingbranch($frombranch)->store(
|
||||
{
|
||||
log_action => 0,
|
||||
skip_record_index => 1, # avoid indexing duplication, let ->transit handle it
|
||||
skip_record_index => 1, # avoid indexing duplication, let ->transit handle it
|
||||
}
|
||||
);
|
||||
$transfer->transit({ skip_record_index => $params->{skip_record_index} });
|
||||
$transfer->transit( { skip_record_index => $params->{skip_record_index} } );
|
||||
}
|
||||
|
||||
return $transfer;
|
||||
|
|
|
|||
|
|
@ -487,8 +487,11 @@ sub TransferCollection {
|
|||
replace => 1
|
||||
}
|
||||
); # Replace transfer
|
||||
# NOTE: If we just replaced a StockRotationAdvance,
|
||||
# it will get enqueued afresh on the next cron run
|
||||
# FIXME: If we just replaced a StockRotationAdvance,
|
||||
# it will get enqueued afresh on the next cron run.. but
|
||||
# that will also push the stage on too.. and what about if
|
||||
# we were at the first stage.. then there won't be a datearrived
|
||||
# to calculate against. See bug 35100
|
||||
}
|
||||
}
|
||||
elsif ( $_->isa('Koha::Exceptions::Item::Transfer::Limit') ) {
|
||||
|
|
|
|||
|
|
@ -776,7 +776,7 @@ sub request_transfer {
|
|||
if ( $request && !$params->{enqueue} && !$params->{replace} );
|
||||
|
||||
$request->cancel( { reason => $params->{reason}, force => 1 } )
|
||||
if ( defined($request) && $params->{replace} && ( $request->reason ne 'StockrotationAdvance' ) );
|
||||
if ( defined($request) && $params->{replace} );
|
||||
|
||||
my $transfer = Koha::Item::Transfer->new(
|
||||
{
|
||||
|
|
|
|||
|
|
@ -130,7 +130,7 @@ subtest 'General Add, Get and Del tests' => sub {
|
|||
};
|
||||
|
||||
subtest 'ModItemTransfer tests' => sub {
|
||||
plan tests => 8;
|
||||
plan tests => 14;
|
||||
|
||||
$schema->storage->txn_begin;
|
||||
|
||||
|
|
@ -189,6 +189,34 @@ subtest 'ModItemTransfer tests' => sub {
|
|||
my $transfer3 = $transfers->next;
|
||||
is($transfer3->reason, 'Manual', "Reason set via ModItemTransfer");
|
||||
|
||||
# Ensure StockrotationAdvance transfers are not cancelled
|
||||
my $stock_transfer = $transfer3->reason('StockrotationAdvance')->store();
|
||||
is(
|
||||
$stock_transfer->reason, 'StockrotationAdvance',
|
||||
'StockrotationAdvance transfer set'
|
||||
);
|
||||
ok(
|
||||
$stock_transfer->datesent,
|
||||
'StockrotationAdvance transfer is in transit'
|
||||
);
|
||||
|
||||
ModItemTransfer( $item->itemnumber, $library1->{branchcode}, $library2->{branchcode}, 'Manual' );
|
||||
$transfers = Koha::Item::Transfers->search(
|
||||
{ itemnumber => $item->itemnumber, },
|
||||
{ order_by => { '-desc' => 'branchtransfer_id' } }
|
||||
);
|
||||
|
||||
my $replaced_transfer = $transfers->next;
|
||||
is(
|
||||
$replaced_transfer->reason, 'Manual',
|
||||
'StockrotationAdvance transfer "replaced" by manual transfer at top of queue'
|
||||
);
|
||||
$stock_transfer->discard_changes;
|
||||
is( $stock_transfer->datecancelled, undef, "StockrotationAdvance transfer not cancelled" );
|
||||
is( $stock_transfer->datesent, undef, "StockrotationAdvance transfer no longer in transit" );
|
||||
$transfers = $item->get_transfers;
|
||||
is( $transfers->count, 2, "There are now 2 live transfers in the queue" );
|
||||
|
||||
$schema->storage->txn_rollback;
|
||||
};
|
||||
|
||||
|
|
|
|||
|
|
@ -846,7 +846,7 @@ subtest 'pickup_locations() tests' => sub {
|
|||
};
|
||||
|
||||
subtest 'request_transfer' => sub {
|
||||
plan tests => 17;
|
||||
plan tests => 13;
|
||||
$schema->storage->txn_begin;
|
||||
|
||||
my $library1 = $builder->build_object( { class => 'Koha::Libraries' } );
|
||||
|
|
@ -906,24 +906,6 @@ subtest 'request_transfer' => sub {
|
|||
is($transfers->count, 1, "There is only 1 live transfer in the queue");
|
||||
$replaced_transfer->datearrived(dt_from_string)->store();
|
||||
|
||||
# Replace StockrotationAdvance transfer
|
||||
my $stock_transfer = $item->request_transfer( { to => $library1, reason => 'StockrotationAdvance' } );
|
||||
is(
|
||||
ref($stock_transfer), 'Koha::Item::Transfer',
|
||||
'Koha::Item->request_transfer added StockrotationAdvance transfer'
|
||||
);
|
||||
$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'
|
||||
);
|
||||
$stock_transfer->discard_changes;
|
||||
is( $stock_transfer->datecancelled, undef, "StockrotationAdvance transfer left intact" );
|
||||
$transfers = $item->get_transfers;
|
||||
is( $transfers->count, 2, "There are now 2 live transfers in the queue" );
|
||||
$replaced_transfer->datearrived(dt_from_string)->store();
|
||||
$stock_transfer->datearrived(dt_from_string)->store();
|
||||
|
||||
# BranchTransferLimits
|
||||
t::lib::Mocks::mock_preference('UseBranchTransferLimits', 1);
|
||||
t::lib::Mocks::mock_preference('BranchTransferLimitsType', 'itemtype');
|
||||
|
|
|
|||
Loading…
Reference in a new issue