Attachment 117064 Details for Bug 25690 – Bug 25690: Remove duplicated logic in SIP2 checkouts

[patch] Bug 25690: Remove duplicated logic in SIP2 checkouts

Bug-25690-Remove-duplicated-logic-in-SIP2-checkout.patch (text/plain), 4.41 KB, created by Martin Renvoize (ashimema) on 2021-02-19 12:09:13 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Martin Renvoize (ashimema)

Created: 2021-02-19 12:09:13 UTC

Size: 4.41 KB

patch

obsolete

>From fa015a1f5846601c4e948be33b700b64edebc492 Mon Sep 17 00:00:00 2001
>From: =?UTF-8?q?Joonas=20Kylm=C3=A4l=C3=A4?= <joonas.kylmala@helsinki.fi>
>Date: Wed, 17 Feb 2021 14:17:36 +0200
>Subject: [PATCH] Bug 25690: Remove duplicated logic in SIP2 checkouts
>
>Because AllowItemsOnHoldCheckoutSIP only affects the checkoutability
>of non-attached, i.e. RESERVED holds in SIP2 we can therefore use the
>common code from CanBookBeIssued and ignore only the RESERVED confirmation
>message case in SIP2 checkout code.
>
>This slightly changes the checkout error message given for "In
>processing" holds that someone other than the holdee tries to
>checkout. Otherwise there is no logic changes. The message that this
>changes is "Item is on hold for another patron." vs. now "Item cannot
>be issued: $confirmation". It is easier to create follow-up patch to
>properly add INPROCESSING confirmation to CanBookBeIssued and then
>show correct message based on the CanBookBeIssued return value.
>
>To test:
> 1) Apply all patches from bug 25690 to get latest Transaction.t version
> 2) prove t/db_dependent/SIP/Transaction.t => passes
>
>Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
>---
> C4/SIP/ILS/Transaction/Checkout.pm | 30 ++++++++++--------------------
> 1 file changed, 10 insertions(+), 20 deletions(-)
>
>diff --git a/C4/SIP/ILS/Transaction/Checkout.pm b/C4/SIP/ILS/Transaction/Checkout.pm
>index 57a3c97307..9a14d5ea18 100644
>--- a/C4/SIP/ILS/Transaction/Checkout.pm
>+++ b/C4/SIP/ILS/Transaction/Checkout.pm
>@@ -53,9 +53,7 @@ sub do_checkout {
>     my $patron         = Koha::Patrons->find($self->{patron}->{borrowernumber});
>     my $overridden_duedate; # usually passed as undef to AddIssue
>     $debug and warn "do_checkout borrower: . " . $patron->borrowernumber;
>-    my ($issuingimpossible, $needsconfirmation) = _can_we_issue($patron, $barcode,
>-        C4::Context->preference("AllowItemsOnHoldCheckoutSIP")
>-    );
>+    my ($issuingimpossible, $needsconfirmation) = _can_we_issue($patron, $barcode, 0);
> 
>     my $noerror=1;  # If set to zero we block the issue
>     if (keys %{$issuingimpossible}) {
>@@ -69,14 +67,15 @@ sub do_checkout {
>         foreach my $confirmation (keys %{$needsconfirmation}) {
>             if ($confirmation eq 'RENEW_ISSUE'){
>                 $self->screen_msg("Item already checked out to you: renewing item.");
>-            } elsif ($confirmation eq 'RESERVED' or $confirmation eq 'RESERVE_WAITING' or $confirmation eq 'TRANSFERRED') {
>-                my $x = $self->{item}->available($patron->borrowernumber);
>-                if ($x) {
>-                    $self->screen_msg("Item was reserved for you.");
>-                } else {
>-                    $self->screen_msg("Item is reserved for another patron upon return.");
>-                    $noerror = 0;
>-                }
>+            } elsif ($confirmation eq 'RESERVED' and !C4::Context->preference("AllowItemsOnHoldCheckoutSIP")) {
>+                $self->screen_msg("Item is reserved for another patron upon return.");
>+                $noerror = 0;
>+            } elsif ($confirmation eq 'RESERVED' and C4::Context->preference("AllowItemsOnHoldCheckoutSIP")) {
>+                next;
>+            } elsif ($confirmation eq 'RESERVE_WAITING' or $confirmation eq 'TRANSFERRED') {
>+               $debug and warn "Item is on hold for another patron.";
>+               $self->screen_msg("Item is on hold for another patron.");
>+               $noerror = 0;
>             } elsif ($confirmation eq 'ISSUED_TO_ANOTHER') {
>                 $self->screen_msg("Item already checked out to another patron.  Please return item for check-in.");
>                 $noerror = 0;
>@@ -110,15 +109,6 @@ sub do_checkout {
>         }
>     }
>     my $itemnumber = $self->{item}->{itemnumber};
>-    foreach (@$shelf) {
>-        $debug and warn sprintf( "shelf has (%s for %s). this is (%is, %s)", $_->{itemnumber}, $_->{borrowernumber}, $itemnumber, $patron->borrowernumber );
>-        ($_->{itemnumber} eq $itemnumber) or next;    # skip it if not this item
>-        ($_->{borrowernumber} == $patron->borrowernumber) and last;
>-            # if item was waiting for this patron, we're done.  AddIssue takes care of the "W" hold.
>-        $debug and warn "Item is on hold for another patron.";
>-        $self->screen_msg("Item is on hold for another patron.");
>-        $noerror = 0;
>-    }
>     my ($fee, undef) = GetIssuingCharges($itemnumber, $patron->borrowernumber);
>     if ( $fee > 0 ) {
>         $self->{sip_fee_type} = '06';
>-- 
>2.20.1

Actions: View | Diff | Splinter Review

Attachments on bug 25690: 116948 | 116949 | 116950 | 116951 | 117000 | 117001 | 117002 | 117003 | 117004 | 117062 | 117063 | 117064 | 117065 | 117066 | 117067 | 117374 | 117507 | 117508 | 117509 | 117510 | 117511 | 117512 | 117513 | 117519 | 117520 | 117521 | 117522 | 117523 | 117524 | 117525 | 117674