Attachment 116571 Details for Bug 26618 – Bug 26618: Remove use of transferbook in RotatingCollections

[patch] Bug 26618: Remove use of transferbook in RotatingCollections

Bug-26618-Remove-use-of-transferbook-in-RotatingCo.patch (text/plain), 7.97 KB, created by Martin Renvoize (ashimema) on 2021-02-09 11:17:35 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Martin Renvoize (ashimema)

Created: 2021-02-09 11:17:35 UTC

Size: 7.97 KB

patch

obsolete

>From 87e4cf0c7505ea460de27909d4011182f7b46250 Mon Sep 17 00:00:00 2001
>From: Martin Renvoize <martin.renvoize@ptfs-europe.com>
>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 <kathleen.milne@cne-siar.gov.uk>
>Signed-off-by: Joonas KylmÃ¤lÃ¤ <joonas.kylmala@helsinki.fi>
>---
> 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 @@
> 
>                 <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>
>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

Actions: View | Diff | Splinter Review

Attachments on bug 26618: 111305 | 111313 | 116571 | 117608 | 117609 | 117610 | 117611