Bug 26990 - No feedback if holds override is disabled and hold fails
Summary: No feedback if holds override is disabled and hold fails
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Hold requests (show other bugs)
Version: master
Hardware: All All
: P5 - low critical (vote)
Assignee: Jonathan Druart
QA Contact: Testopia
URL:
Keywords:
Depends on: 22284
Blocks:
  Show dependency treegraph
 
Reported: 2020-11-10 14:21 UTC by Nick Clemens
Modified: 2021-06-14 21:29 UTC (History)
6 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Text to go in the release notes:
Version(s) released in:
20.11.00, 20.05.06


Attachments
Bug 26990: (bug 22284 follow-up) Prevent hold to be placed if cannot be transferred (1.89 KB, patch)
2020-11-11 14:00 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 26990: (bug 22284 follow-up) Prevent hold to be placed if cannot be transferred (1.94 KB, patch)
2020-11-11 18:36 UTC, David Nind
Details | Diff | Splinter Review
Bug 26990: (bug 22284 follow-up) Prevent hold to be placed if cannot be transferred (2.01 KB, patch)
2020-11-12 13:09 UTC, Kyle M Hall
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Nick Clemens 2020-11-10 14:21:50 UTC
placerequest.pl checks CanItemBeReserved and correctly fails a hold, however, an item that is not holdable because it cannot be transferred is selectable while placing holds and will fail silently as placerequest.pl doesn't set any params for feedback if a hold fails, it expects the hold screen to block any unwanted holds

To recreate:
1 - Set preferences:
     UseBranchTransferLimits: enforce
     BranchTransferLimitsType: itype
     AllowHoldPolicyOverride: Don't allow
2 - Set Centerville 'Book' items to not be allowed to transfer to any other library
3 - Make sure Hold policy is set to 'any library'
4 - Find a record with a Centerville item and other items
5 - Attempt to place an item level hold on the Centerville item
6 - You are taken to the holds list, but your hold is not placed
7 - Nothing indicates why hold has failed
Comment 1 Jonathan Druart 2020-11-11 13:37:41 UTC
Ensure that the pickup library is different than CPL
Comment 2 Jonathan Druart 2020-11-11 13:52:55 UTC Comment hidden (obsolete)
Comment 3 Jonathan Druart 2020-11-11 13:54:37 UTC
Caused by:
  commit 546a3b6d4d6333fe938572f0247dfcbe4a4d35a8
  Bug 22284: New message, new column and filter pickup locations in reserve/request.tt
Comment 4 Jonathan Druart 2020-11-11 13:54:53 UTC
-                my $can_item_be_reserved = CanItemBeReserved( $patron->borrowernumber, $itemnumber, $pickup )->{status};
-                $item->{not_holdable} = $can_item_be_reserved unless $can_item_be_reserved eq 'OK';
+                my $can_item_be_reserved = CanItemBeReserved( $patron->borrowernumber, $itemnumber )->{status};

The $pickup parameter disappears.
Comment 5 Jonathan Druart 2020-11-11 14:00:24 UTC
Created attachment 113497 [details] [review]
Bug 26990: (bug 22284 follow-up) Prevent hold to be placed if cannot be transferred

There is a missing parameter to CanItemBeReserved, we need to know if
the hold can be placed at a given pickup library.

Test plan:
1 - Set preferences:
     UseBranchTransferLimits: enforce
     BranchTransferLimitsType: itype
     AllowHoldPolicyOverride: Don't allow
2 - Set Centerville 'Book' items to not be allowed to transfer to any other library
3 - Make sure Hold policy is set to 'any library'
4 - Find a record with a Centerville item and other items
5 - Attempt to place an item level hold on the Centerville item (with a
pickup library different than Centerville)
=> Without this patch you are taken to the holds list, but your hold is not placed
Nothing indicates why hold has failed
=> With this patch you cannot select the item from Centerville
"Cannot be transferred to pickup library"
Comment 6 David Nind 2020-11-11 18:36:23 UTC
Created attachment 113532 [details] [review]
Bug 26990: (bug 22284 follow-up) Prevent hold to be placed if cannot be transferred

There is a missing parameter to CanItemBeReserved, we need to know if
the hold can be placed at a given pickup library.

Test plan:
1 - Set preferences:
     UseBranchTransferLimits: enforce
     BranchTransferLimitsType: itype
     AllowHoldPolicyOverride: Don't allow
2 - Set Centerville 'Book' items to not be allowed to transfer to any other library
3 - Make sure Hold policy is set to 'any library'
4 - Find a record with a Centerville item and other items
5 - Attempt to place an item level hold on the Centerville item (with a
pickup library different than Centerville)
=> Without this patch you are taken to the holds list, but your hold is not placed
Nothing indicates why hold has failed
=> With this patch you cannot select the item from Centerville
"Cannot be transferred to pickup library"

Signed-off-by: David Nind <david@davidnind.com>
Comment 7 David Nind 2020-11-11 18:43:28 UTC
My notes from testing 

Step 2:
1. Go to Administration > Patrons and circulation > Library transfer limits.
2. Make sure Centerville is selected as the library.
3. For BK click on clear all then save.
4. Note that CPL still stays selected.

Step 4, before step 4 change the library in the staff interface to a library other than Centerville.
Comment 8 Kyle M Hall 2020-11-12 13:09:43 UTC
Created attachment 113554 [details] [review]
Bug 26990: (bug 22284 follow-up) Prevent hold to be placed if cannot be transferred

There is a missing parameter to CanItemBeReserved, we need to know if
the hold can be placed at a given pickup library.

Test plan:
1 - Set preferences:
     UseBranchTransferLimits: enforce
     BranchTransferLimitsType: itype
     AllowHoldPolicyOverride: Don't allow
2 - Set Centerville 'Book' items to not be allowed to transfer to any other library
3 - Make sure Hold policy is set to 'any library'
4 - Find a record with a Centerville item and other items
5 - Attempt to place an item level hold on the Centerville item (with a
pickup library different than Centerville)
=> Without this patch you are taken to the holds list, but your hold is not placed
Nothing indicates why hold has failed
=> With this patch you cannot select the item from Centerville
"Cannot be transferred to pickup library"

Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 9 Jonathan Druart 2020-11-12 16:29:02 UTC
Pushed to master for 20.11, thanks to everybody involved!
Comment 10 Lucas Gass 2020-11-16 22:46:56 UTC
backported to 20.05.x for 20.05.06
Comment 11 Aleisha Amohia 2020-11-17 05:43:31 UTC
missing dependencies, not backported to 19.11.x