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!
Created attachment 18137 [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
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.
Created attachment 18138 [details] [review] Bug 10243 - Holds queue is making transfer requests that contradict the branch holds policy - Alternate solution 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
Created attachment 18160 [details] [review] [Signed off] 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>
I, Heather Braum, from NEKLS sign off on this patch. I have tested it on our production server and it works.
*** Bug 9906 has been marked as a duplicate of this bug. ***
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>
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.
Created attachment 21121 [details] [review] Bug 10243 - Holds queue is making transfer requests that contradict the branch holds policy - Unit Tests
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.
Pushed to master. Thanks, Kyle!
This patch has been pushed to 3.12.x, will be in 3.12.8. Thanks Kyle!