Bug 26618: Remove use of transferbook in RotatingCollections

This patch replaces the use of C4::Circulation::transferbook in
C4::RotatingCollections with calls to Koha::Item->request_transfer and
adds handling for the various failure cases which that can throw.

We also introduce additional feedback for the end user where it did not
exist before. Now we notify the user if some of the collection could not
be transfers or if transfers were queued rather than set to request
immediately.

Test plan
1/ Set up a rotating collection
2/ Transfer the collection
3/ Confirm the action succeeds
4/ Set up some branch transfer limits that will affect items in your
collection
5/ Transfer the collection
6/ Note that the transfer succeeds but some items are returned as
failures

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 2020-12-18 16:37:29 +00:00 committed by Jonathan Druart
parent 80e3bfd376
commit 87c3911bb8
3 changed files with 81 additions and 22 deletions

View File

@ -30,8 +30,7 @@ use C4::Reserves qw(CheckReserves);
use Koha::Database;
use DBI;
use Data::Dumper;
use Try::Tiny;
use vars qw(@ISA @EXPORT);
@ -410,8 +409,7 @@ Transfers a collection to another branch
Output:
$success: 1 if all database operations were successful, 0 otherwise
$errorCode: Code for reason of failure, good for translating errors in templates
$errorMessage: English description of error
$messages: Arrayref of messages for user feedback
=cut
@ -435,7 +433,8 @@ sub TransferCollection {
colBranchcode = ?
WHERE colId = ?"
);
$sth->execute( $colBranchcode, $colId ) or return ( 0, 4, $sth->errstr() );
$sth->execute( $colBranchcode, $colId ) or return 0;
my $to_library = Koha::Libraries->find( $colBranchcode );
$sth = $dbh->prepare(q{
SELECT items.itemnumber, items.barcode FROM collections_tracking
@ -444,21 +443,64 @@ sub TransferCollection {
WHERE issues.borrowernumber IS NULL
AND collections_tracking.colId = ?
});
$sth->execute($colId) or return ( 0, 4, $sth->errstr );
my @results;
$sth->execute($colId) or return 0;
my $messages;
while ( my $item = $sth->fetchrow_hashref ) {
my ($status) = CheckReserves( $item->{itemnumber} );
my @transfers = C4::Circulation::GetTransfers( $item->{itemnumber} );
C4::Circulation::transferbook({
from_branch => $item->holdingbranch,
to_branch => $colBranchcode,
barcode => $item->{barcode},
ignore_reserves => 1,
trigger => 'RotatingCollection'
}) unless ( $status eq 'Waiting' || $status eq 'Processing' || @transfers );
my $item_object = Koha::Items->find( $item->{itemnumber} );
try {
$item_object->request_transfer(
{
to => $to_library,
reason => 'RotatingCollection',
ignore_limits => 0
}
); # Request transfer
}
catch {
if ( $_->isa('Koha::Exceptions::Item::Transfer::Found') ) {
my $exception = $_;
my $found_transfer = $_->transfer;
if ( $found_transfer->in_transit
|| $found_transfer->reason eq 'Reserve' )
{
my $transfer = $item_object->request_transfer(
{
to => $to_library,
reason => "RotatingCollection",
ignore_limits => 0,
enqueue => 1
}
); # Queue transfer
push @{$messages},
{
type => 'enqueu',
item => $item_object,
found_transfer => $found_transfer
};
}
else {
my $transfer = $item_object->request_transfer(
{
to => $to_library,
reason => "RotatingCollection",
ignore_limits => 0,
replace => 1
}
); # Replace transfer
# NOTE: If we just replaced a StockRotationAdvance,
# it will get enqueued afresh on the next cron run
}
}
elsif ( $_->isa('Koha::Exceptions::Item::Transfer::Limit') ) {
push @{$messages}, { type => 'failure', item => $item_object };
}
else {
$_->rethrow();
}
};
}
return 1;
return (1, $messages);
}
=head2 GetCollectionItemBranches

View File

@ -19,6 +19,21 @@
<h1>Transfer collection <em>[% colTitle | html %]</em></h1>
[% IF ( messages ) %]
[% FOREACH message IN messages %]
[%- SWITCH message.type -%]
[%- CASE 'failure' %]
<div class="dialog error">
<p>Cannot transfer item [% message.item.itemnumber | html %] due to transfer limits</p>
</div>
[%- CASE 'enqueu' -%]
<div class="dialog message">
<p>Item [% message.item.itemnumber | html %] queued behind [% message.found_transfer.reason | html %] transfer to [% Branches.GetName(message.found_transfer.tobranch) | html %]</p>
</div>
[% END %]
[% END %]
[% END %]
[% IF ( transferSuccess ) %]
<div class="dialog message">
<p>Collection transferred successfully</p>

View File

@ -41,19 +41,21 @@ my ( $template, $loggedinuser, $cookie ) = get_template_and_user(
);
## Transfer collection
my ( $success, $errorCode, $errorMessage );
my ( $success, $messages );
if ($toBranch) {
( $success, $errorCode, $errorMessage ) =
( $success, $messages) =
TransferCollection( $colId, $toBranch );
if ($success) {
$template->param( transferSuccess => 1 );
$template->param(
transferSuccess => 1,
messages => $messages
);
}
else {
$template->param(
transferFailure => 1,
errorCode => $errorCode,
errorMessage => $errorMessage
messages => $messages
);
}
}