It would be helpful if build_holds_queue.pl either 1) checked items' holding libraries in addition to home when items float, or 2) used the Transport Cost Matrix instead when systems have it enabled. Using home library only/first results in higher 'costs' and in holds being filled more slowly than necessary, and can be confusing for libraries. Home/holding: If I understand correctly, build_holds_queue.pl looks for items whose home library matches the hold's pickup library (plus accounting for any preferences set in StaticHoldsQueueWeight/HoldsQueueSkipClosed/RandomizeHoldsQueueWeight). For example, in a system where items float, a hold to be picked up at West library has the following items: Item 1: Home library West, holding library East Item 2: Home library West, holding library West Item 3: Home library North, holding library West If build_holds_queue.pl sees Item 1 first, it will be placed on East's queue to send to West even though Item 2 is already at West and could fill the hold more efficiently. Then since the Transport Cost Matrix is only consulted if no available items have a home library matching the hold's pickup library, holds are filled in a more 'expensive' way than libraries may expect. Using the Transport Cost Matrix directly would address this. For example, in a system where items float and a hold is to be picked up at South: Item 1: Home library East, holding library South Item 2: Home library West, holding library West Item 3: Home library South, holding library West build_holds_queue.pl will see that Item 3 has a home library matching the pickup library and will place it on West's queue to send to South. If build_holds_queue.pl consulted the Transport Cost Matrix, Item 1 would be placed on South's queue with no transfer necessary.
Created attachment 142082 [details] [review] Bug 31557: Add ability for holds queue builder to prioritize either matching a patron's home library to the item's home or holding library Right now the holds queue builder starts filling bib-level holds with items whose patron's home library matches the item's home library. It would be good and reasonable to have the option to prioritize item's whose patron's home library matches the item's holding library to minimize transfers.
I updated the assignee...
Kyle, please let me know if this proposed test plan misses something: To test: 1: have 2 branches: A, B 2: have a bib with two items: Item 1 with homebranch=A, holdingbranch=B; item 2 with homebranch=B, holdingbranch=A 3: have a patron with homebranch=A 4: place a bib-level hold on your bib for your patron, for pickup at A 5: run build_holds_queue.pl 6: consult the queue, confirm it's targeted Item 1, which will require a transfer to fill 7: apply patch, restart services 8: confirm the new HoldsQueuePrioritizeBranch syspref is set to homebranch 9: run build_holds_queue again 10: confirm nothing changed, still targeting Item 1 11: switch HoldsQueuePrioritizeBranch to holdingbranch 12: run build_holds_queue 13: confirm the queue is now targeting Item 2, which will not require a transfer I didn't involve the real time holds queue, just for simplicity.
Created attachment 143136 [details] [review] Bug 31557: Add ability for holds queue builder to prioritize either matching a patron's home library to the item's home or holding library Right now the holds queue builder starts filling bib-level holds with items whose patron's home library matches the item's home library. It would be good and reasonable to have the option to prioritize item's whose patron's home library matches the item's holding library to minimize transfers. Signed-off-by: Andrew Fuerste-Henry <andrewfh@dubcolib.org>
Created attachment 147284 [details] [review] Bug 31557: Add ability for holds queue builder to prioritize either matching a patron's home library to the item's home or holding library Right now the holds queue builder starts filling bib-level holds with items whose patron's home library matches the item's home library. It would be good and reasonable to have the option to prioritize item's whose patron's home library matches the item's holding library to minimize transfers. Signed-off-by: Andrew Fuerste-Henry <andrewfh@dubcolib.org> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 147285 [details] [review] Bug 31557: (QA follow-up) Fix number of tests Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 151135 [details] [review] Bug 31557: (QA follow-up) Make tests more robust Tests had a few problems: - They weren't wrapped in a transaction explicitly - They called Koha::Biblios->delete! - They failed if run a couple times This patch: - Wraps things in a transaction - Removes not-required things ($target_rs variable not used) - Preserves the tests logic, but filters the resultset on the biblionumber instead of deleting all the database which can fail depending on FK constraints. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Pushed to master for 23.05. Nice work everyone, thanks!
Enhancement - not backporting to 22.11.x
I know I'm late, but shouldn't this look at pickup library rather than patron's home library? If the purpose was to limit transfers, pickup library should be prioritized. If patron has home library = A and the hold is pickup library = B, this will make the item with library A fill the hold, even if there is an item at library B. I'm wondering if this is by design or not. If not, I can open a new bug report. Just let me know. Caroline
I agree with Caroline. Shouldn't we compare $priority_branch to $pickup_branch? Also, do we really need an additional system preference? We already have LocalHoldsPriorityItemControl.
So, this is slightly confusing to explain. We group all the possible items by their holding branch ( $items_by_branch variable ) 554 push @{ $items_by_branch{ $item->{holdingbranch} } }, $item Then we get all the items that are held at the hold's pickup location ( here we fall back to the borrowers branch if there is no hold pickup location, but a hold must have a pickuplocation, so it's moot) 568 my $pickup_branch = $request->{branchcode} || $request->{borrowerbranch} 571 my $holding_branch_items = $items_by_branch{$pickup_branch}; And set the priority value: 572 my $priority_branch = C4::Context->preference('HoldsQueuePrioritizeBranch') // 'homebranch'; Then we will assign the hold to the first item where: $request->{borrowerbranch} eq $item->{$priority_branch} and the hold is allowed So we are already in the subset of items where the holding branch matches the pickup branch, and then we give priority to patrons from the holding branch, or patrons from the owning branch.
(In reply to Nick Clemens from comment #12) > So, this is slightly confusing to explain. > > > We group all the possible items by their holding branch ( $items_by_branch > variable ) > 554 push @{ $items_by_branch{ $item->{holdingbranch} } }, $item > > Then we get all the items that are held at the hold's pickup location ( here > we fall back to the borrowers branch if there is no hold pickup location, > but a hold must have a pickuplocation, so it's moot) > 568 my $pickup_branch = $request->{branchcode} || > $request->{borrowerbranch} > 571 my $holding_branch_items = $items_by_branch{$pickup_branch}; > > And set the priority value: > 572 my $priority_branch = > C4::Context->preference('HoldsQueuePrioritizeBranch') // 'homebranch'; > > Then we will assign the hold to the first item where: > $request->{borrowerbranch} eq $item->{$priority_branch} > and the hold is allowed > > So we are already in the subset of items where the holding branch matches > the pickup branch, and then we give priority to patrons from the holding > branch, or patrons from the owning branch. I would be wonderful if we had flowcharts for these types of things. Many of us are visual learners. Flowcharts, man! :)