From 87e4cf0c7505ea460de27909d4011182f7b46250 Mon Sep 17 00:00:00 2001 From: Martin Renvoize Date: Fri, 18 Dec 2020 16:37:29 +0000 Subject: [PATCH] Bug 26618: Remove use of transferbook in RotatingCollections MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Signed-off-by: Joonas Kylmälä --- C4/RotatingCollections.pm | 76 ++++++++++++++----- .../transferCollection.tt | 15 ++++ rotating_collections/transferCollection.pl | 12 +-- 3 files changed, 81 insertions(+), 22 deletions(-) diff --git a/C4/RotatingCollections.pm b/C4/RotatingCollections.pm index b1fae8219a..6613602ab0 100644 --- a/C4/RotatingCollections.pm +++ b/C4/RotatingCollections.pm @@ -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 diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/rotating_collections/transferCollection.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/rotating_collections/transferCollection.tt index af41ccbc85..66a5817aec 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/rotating_collections/transferCollection.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/rotating_collections/transferCollection.tt @@ -19,6 +19,21 @@

Transfer collection [% colTitle | html %]

+ [% IF ( messages ) %] + [% FOREACH message IN messages %] + [%- SWITCH message.type -%] + [%- CASE 'failure' %] +
+

Cannot transfer item [% message.item.itemnumber | html %] due to transfer limits

+
+ [%- CASE 'enqueu' -%] +
+

Item [% message.item.itemnumber | html %] queued behind [% message.found_transfer.reason | html %] transfer to [% Branches.GetName(message.found_transfer.tobranch) | html %]

+
+ [% END %] + [% END %] + [% END %] + [% IF ( transferSuccess ) %]

Collection transferred successfully

diff --git a/rotating_collections/transferCollection.pl b/rotating_collections/transferCollection.pl index 5cfc62699a..6c1bcde1ff 100755 --- a/rotating_collections/transferCollection.pl +++ b/rotating_collections/transferCollection.pl @@ -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 ); } } -- 2.20.1