Attachment 121722 Details for Bug 28520 – Bug 28520: Allow creating a transfer back automatically if a hold is canceled during transit

[patch] Bug 28520: Allow creating a transfer back automatically if a hold is canceled during transit

Bug-28520-Allow-creating-a-transfer-back-automatic.patch (text/plain), 4.50 KB, created by Martin Renvoize (ashimema) on 2021-06-08 12:05:49 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Martin Renvoize (ashimema)

Created: 2021-06-08 12:05:49 UTC

Size: 4.50 KB

patch

obsolete

>From 2ac04d4131ac0f955fb1be3e5a11ecbe11288c42 Mon Sep 17 00:00:00 2001
>From: =?UTF-8?q?Joonas=20Kylm=C3=A4l=C3=A4?= <joonas.kylmala@helsinki.fi>
>Date: Tue, 8 Jun 2021 11:14:20 +0300
>Subject: [PATCH] Bug 28520: Allow creating a transfer back automatically if a
> hold is canceled during transit
>
>This fixes regression caused by "Bug 12362: Cancel transfer with hold
>cancelation" where cancelled hold's transfer didn't show up in
>intranet and opac because it create a new transfer that was not yet
>put in in-transit state. The original idea of bug 12362 was to be able
>to trigger transfer back home if a hold was cancelled (a regression
>caused by bug 26078). However, we can do it more simply by setting the
>$validTransfer variable true in the item check-in code when we are
>dealing with Reserve transfers. More down in the AddReturn() code
>there is also a check "and !$resfound" to make sure we only try to
>trigger the transfer back home automatically if there is no hold
>waiting at the current location the item arrived in.
>
>It should be noted however that now we only display generic message
>for the automatic transfer reason. Bug 12362 made the return display
>as the reason "Transfer was cancelled whilst in transit". However,
>since this fixes the original regressions caused by bug 26078 and
>restores similar behaviour to that I think giving a more descriptive
>message for example regarding a hold being cancelled can be considered
>a further enhancement.
>
>To test:
> 1) Apply patch
> 1) Have biblio with item in branch A
> 2) Create a new hold with a pickup library to branch B
> 3) Check-in the item at branch A and confirm the hold and transfer
> 4) Cancel the hold
> 5) Check-in the hold at branch B and notice it prompt to return it to
> branch A
>
>Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
>---
> C4/Circulation.pm                   |  4 ++++
> t/db_dependent/Circulation/Branch.t | 10 ++++++++--
> 2 files changed, 12 insertions(+), 2 deletions(-)
>
>diff --git a/C4/Circulation.pm b/C4/Circulation.pm
>index f0e39b7583..a027407888 100644
>--- a/C4/Circulation.pm
>+++ b/C4/Circulation.pm
>@@ -2171,6 +2171,8 @@ sub AddReturn {
>             if ( $transfer->tobranch eq $branch ) {
>                 $transfer->receive;
>                 $messages->{'TransferArrived'} = $transfer->frombranch;
>+                # validTransfer=1 allows us returning the item back if the reserve is cancelled
>+                $validTransfer = 1 if $transfer->reason eq 'Reserve';
>             }
>             else {
>                 $messages->{'WrongTransfer'}     = $transfer->tobranch;
>@@ -2181,6 +2183,8 @@ sub AddReturn {
>             if ( $transfer->tobranch eq $branch ) {
>                 $transfer->receive;
>                 $messages->{'TransferArrived'} = $transfer->frombranch;
>+                # validTransfer=1 allows us returning the item back if the reserve is cancelled
>+                $validTransfer = 1 if $transfer->reason eq 'Reserve';
>             }
>             else {
>                 $messages->{'WasTransfered'}   = $transfer->tobranch;
>diff --git a/t/db_dependent/Circulation/Branch.t b/t/db_dependent/Circulation/Branch.t
>index 1a66ad1efb..041c000449 100755
>--- a/t/db_dependent/Circulation/Branch.t
>+++ b/t/db_dependent/Circulation/Branch.t
>@@ -25,7 +25,7 @@ use Koha::CirculationRules;
> 
> use Koha::Patrons;
> 
>-use Test::More tests => 15;
>+use Test::More tests => 16;
> use t::lib::Mocks;
> use t::lib::TestBuilder;
> 
>@@ -307,7 +307,13 @@ is( $messages->{NeedsTransfer}, $samplebranch1->{branchcode}, "AddReturn respect
> ModItemTransfer($item_id2, $samplebranch2->{branchcode}, $samplebranch1->{branchcode}, "ReturnToHolding");
> # Fulfill it
> ($doreturn, $messages, $iteminformation, $borrower) = AddReturn('barcode_2',$samplebranch1->{branchcode});
>-is( $messages->{NeedsTransfer}, undef, "AddReturn does not generate a new transfer for return policy when resolving an existing transfer" );
>+is( $messages->{NeedsTransfer}, undef, "AddReturn does not generate a new transfer for return policy when resolving an existing non-Reserve transfer" );
>+
>+# Generate a hold caused transfer which doesn't have a hold i.e. is the hold is cancelled
>+ModItemTransfer($item_id2, $samplebranch2->{branchcode}, $samplebranch1->{branchcode}, "Reserve");
>+# Fulfill it
>+($doreturn, $messages, $iteminformation, $borrower) = AddReturn('barcode_2',$samplebranch1->{branchcode});
>+is( $messages->{NeedsTransfer}, $samplebranch2->{branchcode}, "AddReturn generates a new transfer for hold transfer if the hold was cancelled" );
> 
> # item3 should not trigger transfer - floating collection
> $query =
>-- 
>2.20.1

Actions: View | Diff | Splinter Review

Attachments on bug 28520: 121686 | 121689 | 121690 | 121691 | 121693 | 121708 | 121709 | 121710 | 121711 | 121712 | 121718 | 121719 | 121720 | 121721 | 121722 | 121843 | 121844 | 121845 | 121846 | 121847 | 121951 | 121952 | 121953 | 121954 | 121955 | 121985 | 122005 | 122006 | 122007