Attachment 136304 Details for Bug 30960 – Bug 30960: Don't allow user to place the hold without a pick-up place

[patch] Bug 30960: Don't allow user to place the hold without a pick-up place

Bug-30960-Dont-allow-user-to-place-the-hold-withou.patch (text/plain), 5.29 KB, created by David Nind on 2022-06-17 19:49:27 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: David Nind

Created: 2022-06-17 19:49:27 UTC

Size: 5.29 KB

patch

obsolete

>From 83b9a99898c511ccabcd390168e456f146520825 Mon Sep 17 00:00:00 2001
>From: Petro Vashchuk <stalkernoid@gmail.com>
>Date: Mon, 13 Jun 2022 15:02:28 +0300
>Subject: [PATCH] Bug 30960: Don't allow user to place the hold without a
> pick-up place
>
>It's possible to place item-level hold without selecting a pick-up
>location, which causes problems: if the item is then returned,
>Koha tells about a hold, but gives error 500.
>At customer's hold tab you see the number of holds, but cannot see the
>actual holds there. However, if you go to the title in question,
>then modify the hold so that it has a pick-up place,
>then the hold will work normally again.
>
>This patch fixes already existing but not working js error message and
>ensures that hold cannot be made while pickup location is undefined.
>
>To reproduce:
>1. Go to admin page, to the libraries configurations, and disable
>pickup location for one of them.
>2. Pick any biblio that has items that have that same library as a
>default pickup location.
>3. When placing the item-level hold, notice that the pickup location
>dropdown box is empty by default. Keep it empty, place the hold.
>4. Go to the patron's page of the patron who you placed that hold for,
>check that it doesn't show the new hold.
>5. Apply patch.
>6. Repeat steps 2 and 3, it shouldn't let you make the item-level hold
>until you select a specific pickup location.
>
>Signed-off-by: David Nind <david@davidnind.com>
>---
> .../prog/en/modules/reserve/request.tt        |  2 +-
> reserve/placerequest.pl                       | 47 ++++++++++---------
> 2 files changed, 25 insertions(+), 24 deletions(-)
>
>diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt
>index 1a062bb750..20cde5f0e3 100644
>--- a/koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt
>+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt
>@@ -1362,7 +1362,7 @@
> 
>             var msg = "";
> 
>-            if ( $("#requestany").attr("checked") !== "checked" ) {
>+            if ( ! $("#requestany").is(":checked") ) {
>                 // requestany not selected, go through the item-specific cases
>                 if ( $('input[type="radio"]:checked').length > 0 ) {
>                     // got item-specific hold requests in the form!
>diff --git a/reserve/placerequest.pl b/reserve/placerequest.pl
>index 4f255fdcc5..0fc7361b73 100755
>--- a/reserve/placerequest.pl
>+++ b/reserve/placerequest.pl
>@@ -71,34 +71,35 @@ if ( $patron ) {
>         my $can_override = C4::Context->preference('AllowHoldPolicyOverride');
>         if ( defined $checkitem && $checkitem ne '' ) {
> 
>-            my $item_pickup_location = $input->param("item_pickup_$checkitem");
>+            if ( my $item_pickup_location = $input->param("item_pickup_$checkitem") ) {
> 
>-            my $item = Koha::Items->find($checkitem);
>+                my $item = Koha::Items->find($checkitem);
> 
>-            if ( $item->biblionumber ne $biblionumber ) {
>-                $biblionumber = $item->biblionumber;
>-            }
>+                if ( $item->biblionumber ne $biblionumber ) {
>+                    $biblionumber = $item->biblionumber;
>+                }
> 
>-            my $can_item_be_reserved = CanItemBeReserved($patron, $item, $item_pickup_location)->{status};
>+                my $can_item_be_reserved = CanItemBeReserved($patron, $item, $item_pickup_location)->{status};
> 
>-            if ( $can_item_be_reserved eq 'OK' || ( $can_item_be_reserved ne 'itemAlreadyOnHold' && $can_override ) ) {
>-                AddReserve(
>-                    {
>-                        branchcode       => $item_pickup_location,
>-                        borrowernumber   => $patron->borrowernumber,
>-                        biblionumber     => $biblionumber,
>-                        priority         => $rank[0],
>-                        reservation_date => $startdate,
>-                        expiration_date  => $expirationdate,
>-                        notes            => $notes,
>-                        title            => $title,
>-                        itemnumber       => $checkitem,
>-                        found            => $found,
>-                        itemtype         => $itemtype,
>-                        non_priority     => $non_priority,
>-                    }
>-                );
>+                if ( $can_item_be_reserved eq 'OK' || ( $can_item_be_reserved ne 'itemAlreadyOnHold' && $can_override ) ) {
>+                    AddReserve(
>+                        {
>+                            branchcode       => $item_pickup_location,
>+                            borrowernumber   => $patron->borrowernumber,
>+                            biblionumber     => $biblionumber,
>+                            priority         => $rank[0],
>+                            reservation_date => $startdate,
>+                            expiration_date  => $expirationdate,
>+                            notes            => $notes,
>+                            title            => $title,
>+                            itemnumber       => $checkitem,
>+                            found            => $found,
>+                            itemtype         => $itemtype,
>+                            non_priority     => $non_priority,
>+                        }
>+                    );
> 
>+                }
>             }
> 
>         } elsif (@biblionumbers > 1) {
>-- 
>2.30.2

Actions: View | Diff | Splinter Review

Attachments on bug 30960: 136066 | 136257 | 136304 | 136325 | 136326