This patch fixes a problem where the holds queue generator
was making requests where the pickup library is the
same as the item's library but not the patron's branch,
even if there is a "Default holds policy by item type" rule that states
this item can only fill holds for patrons of the same library as the
item.
Test Plan:
1) Create a test record with 2 items with different itemtypes
2) Set the Default holds policy by item type for the first
item to "From any library"
3) Set the Default holds policy by item type for the second
item to "From home library"
4) Place a record level hold for a patron from another library,
but for pickup at the same library as the item is from
5) Rebuild the holds queue
6) View the holds queue, note the item is listed, though this
patron cannot place a hold on this item
7) Apply this patch
8) Repeat step 5, note the hold is no longer in the queue
Signed-off-by: Liz Rea <liz@catalyst.net.nz>
automated tests pass, functional tests pass. Bug replicated, eradicated by patch.
Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
I finally managed to reproduce this, patch works as described.
Passes tests and QA script, provided tests fail without patch, but
succeed with the patch.
Signed-off-by: Galen Charlton <gmc@esilibrary.com>
(cherry picked from commit 7966037747)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
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.
Signed-off-by: Galen Charlton <gmc@esilibrary.com>
(cherry picked from commit 06ea76c3c2)
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
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>
Signed-off-by: Galen Charlton <gmc@esilibrary.com>
(cherry picked from commit 78d27ad539)
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
For some reason, C4::HoldsQueue::MapItemsToHoldRequests used the system
preference AutomaticItemReturn to decide if an attempt to fill local
holds with local items. No explanation of this behavior is provided.
This patch removes this behavior, and also adjusts the calculation
of the lead-cost library to always return the pickup library if it
is on the list of libraries that could fill the hold -- on the
basis that if the item is already at the pickup library, its
transport cost is inherently zero.
Signed-off-by: Srdjan <srdjan@catalyst.net.nz>
Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Passes QA script and adds unit tests.
Tested with some examples and those worked correctly.
Signed-off-by: Galen Charlton <gmc@esilibrary.com>
(cherry picked from commit d46657f8af)
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
Small merge problem in the tests.
If a record has only one item, and that item has one item-level hold on
it, that hold will not show in the holds queue.
Test Plan:
1) Create 1 record with 1 item at BranchA
2) Create an item-level hold for that item, for pickup at BranchA by a
patron of BranchA
3) Run build_holds_queue.pl
4) View the holds queue for BranchA
5) Note the hold is not in there
6) Apply this patch
7) Re-run build_holds_queue.pl
8) View the holds queue again
9) Not that the hold is now there
Signed-off-by: George Williams <georgew@latahlibrary.org>
Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz>
Signed-off-by: Galen Charlton <gmc@esilibrary.com>
(cherry picked from commit acd4b21c91)
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
This bug was reintroduced by the patch for bu 5911: Transport Cost Matrix
Test Plan:
1) Place a hold on a record
2) Run build_holds_queue.pl
3) Verify the hold is showing in the holds queue
4) Suspend the hold
5) Re-run build_holds_queue.pl
6) Note the hold is still in the holds queue
7) Apply patch
8) Re-run build_holds_queue.pl
9) Note the hold is no longer in the holds queue
Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz>
Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Passes test plan and QA script.
Signed-off-by: Jared Camins-Esakov <jcamins@cpbibliography.com>
(cherry picked from commit d098a7c298)
Signed-off-by: Jared Camins-Esakov <jcamins@cpbibliography.com>
Test Plan:
1) Enable AutomaticItemReturn
2) Place a reserve on a record where the holding and home branches differ
for the available items
3) Rebuild the holds queue
4) Check the holds queue, verify the item is listed in the items to pull for the item's home branch
5) Disable AutomaticItemReturn
6) Rebuild the holds queue
7) Verify the item is listed in the items to pull for the item's holding branch
8) Enable AutomaticItemReturn
9) Apply patch
10) Rebuild the holds queue
11) Verify the item is listed in the items to pull for the item's holding branch
Signed-off-by: Liz Rea <liz@catalyst.net.nz>
Tested per plan, and the patch seems sane. Functionality of the hold queue is restored to previous behaviour.
Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Amended test plan to make clear it has to be a record level
hold for the test plan to work.
All tests and QA script pass.
Signed-off-by: Jared Camins-Esakov <jcamins@cpbibliography.com>
This counter-patch moves all database handling code into subroutines
in C4::HoldsQueue. This fixes the test, and is required for persistent
environments like Plack.
Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz>
Create transport_cost table, added UseTransportCostMatrix syspref.
transport_cost table contains branch to branch transfer
costs. These are used for filling inter-branch hold transfers.
Moved GetHoldsQueueItems() from .pl to HoldsQueue.pm
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Paul Poulain <paul.poulain@biblibre.com>
Kyle M Hall
Jared Camins-Esakov
Srdjan