Bug 10243

Summary: Holds queue is making transfer requests that contradict the branch holds policy
Product: Koha Reporter: Kyle M Hall <kyle.m.hall>
Component: Hold requestsAssignee: Bugs List <koha-bugs>
Status: CLOSED FIXED QA Contact:
Severity: major    
Priority: P5 - low CC: gmcharlt, gwilliams, melia, tomascohen
Version: Main   
Hardware: All   
OS: All   
GIT URL: Change sponsored?: ---
Patch complexity: --- Documentation contact:
Documentation submission: Text to go in the release notes:
Version(s) released in:
Circulation function:
Attachments: Bug 10243 - Holds queue is making transfer requests that contradict the branch holds policy
Bug 10243 - Holds queue is making transfer requests that contradict the branch holds policy - Alternate solution
[Signed off] Bug 10243 - Holds queue is making transfer requests that contradict the branch holds policy
Bug 10243 - Holds queue is making transfer requests that contradict the branch holds policy
Bug 10243 - Holds queue is making transfer requests that contradict the branch holds policy - Unit Tests
Bug 10243 - Holds queue is making transfer requests that contradict the branch holds policy - QA Followup

Description Kyle M Hall 2013-05-14 19:53:58 UTC
For some reason MapItemsToHoldRequests will, as a last ditch effort, grab what seems to be an arbitrary available item to fill a hold request, even if it will violate the circulation rules for holds.

In other words, even if an item matches a "Holds policy by item type" that says "From home library", the a request to transfer that item to another library will be added to the holds queue!
Comment 1 Kyle M Hall (khall) 2013-05-14 20:02:40 UTC Comment hidden (obsolete)
Comment 2 Kyle M Hall 2013-05-14 20:05:36 UTC
This patch is tested and working, but I wonder if we should simply delete lines 470-471 instead of using the code I wrote. I cannot understand the justification for what 470-471 do, and there are no comments about why it's there. 

I'll post an alterative that just deletes those lines, either one will resolve this bug.
Comment 3 Kyle M Hall (khall) 2013-05-14 20:09:38 UTC Comment hidden (obsolete)
Comment 4 Melia Meggs 2013-05-15 17:19:16 UTC Comment hidden (obsolete)
Comment 5 HB-NEKLS 2013-05-15 17:45:55 UTC
I, Heather Braum, from NEKLS sign off on this patch. I have tested it on our production server and it works.
Comment 6 Melia Meggs 2013-05-16 16:53:46 UTC
*** Bug 9906 has been marked as a duplicate of this bug. ***
Comment 7 Chris Cormack 2013-06-18 08:48:47 UTC
Created attachment 19097 [details] [review]
Bug 10243 - Holds queue is making transfer requests that contradict the branch holds policy

For some reason MapItemsToHoldRequests will, as a last ditch effort,
grab what seems to be an arbitrary available item to fill a hold request,
even if it will violate the circulation rules for holds.

In other words, even if an item matches a "Holds policy by item type"
that says "From home library", a request to transfer that item to
another library will be added to the holds queue!

Test Plan:
1) Create a record with a an item at BranchA of item type BOOK
2) Set the holds policy such that itemtype BOOK for BranchA is set
   to "From home library"
3) Place a bib-level hold request for a patron with a pickup at BranchB
4) Run build_holds_queue.pl
5) You should now see a request for that item to be transfered to
   BranchB, even though the rules should not allow this.
6) Apply this patch
7) Run build_holds_queue.pl again
8) View the holds queue again, that request should no longer exist

Signed-off-by: Heather Braum <hbraum@nekls.org>
Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz>
Comment 8 Galen Charlton 2013-06-18 18:45:06 UTC
Would you mind adding test cases for this situation to t/db_dependent/HoldsQueue.t?  Since HoldsQueue.t is pretty well fleshed out already, it should be easier to add regression tests for this than might be the case with other modules.
Comment 9 Kyle M Hall (khall) 2013-09-16 15:31:40 UTC
Created attachment 21121 [details] [review]
Bug 10243 - Holds queue is making transfer requests that contradict the branch holds policy - Unit Tests
Comment 10 Kyle M Hall (khall) 2013-10-09 18:03:59 UTC
Created attachment 21927 [details] [review]
Bug 10243 - Holds queue is making transfer requests that contradict the branch holds policy - QA Followup

Using the TransportCostMatrix can cause the same issue. Removing the
last ditch use of the first item causes the the subroutine to continue
with the traditional matching, which will respect the hold policies.
Comment 11 Galen Charlton 2013-10-30 03:23:41 UTC
Pushed to master.  Thanks, Kyle!
Comment 12 Tomás Cohen Arazi (tcohen) 2013-12-05 18:34:26 UTC
This patch has been pushed to 3.12.x, will be in 3.12.8.

Thanks Kyle!