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
Summary: Add ability for holds queue builder to prioritize either matching a patron's ...
Status: Needs documenting
Alias: None
Product: Koha
Classification: Unclassified
Component: Circulation (show other bugs)
Version: Main
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Kyle M Hall
QA Contact: Marcel de Rooy
URL:
Keywords: rel_23_05_candidate
Depends on:
Blocks: 32089
  Show dependency treegraph
 
Reported: 2022-09-13 20:22 UTC by Sara Brown
Modified: 2023-12-08 20:03 UTC (History)
16 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
23.05.00


Attachments
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 (9.37 KB, patch)
2022-10-18 16:49 UTC, Kyle M Hall
Details | Diff | Splinter 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 (9.43 KB, patch)
2022-11-03 15:28 UTC, Andrew Fuerste-Henry
Details | Diff | Splinter 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 (9.36 KB, patch)
2023-02-24 09:28 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 31557: (QA follow-up) Fix number of tests (760 bytes, patch)
2023-02-24 09:28 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 31557: (QA follow-up) Make tests more robust (3.63 KB, patch)
2023-05-12 14:16 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Sara Brown 2022-09-13 20:22:54 UTC
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.
Comment 1 Kyle M Hall 2022-10-18 16:49:55 UTC
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.
Comment 2 David Nind 2022-11-01 01:37:46 UTC
I updated the assignee...
Comment 3 Andrew Fuerste-Henry 2022-11-03 14:39:28 UTC
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.
Comment 4 Andrew Fuerste-Henry 2022-11-03 15:28:57 UTC
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>
Comment 5 Marcel de Rooy 2023-02-24 09:28:46 UTC
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>
Comment 6 Marcel de Rooy 2023-02-24 09:28:49 UTC
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>
Comment 7 Tomás Cohen Arazi 2023-05-12 14:16:25 UTC
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>
Comment 8 Tomás Cohen Arazi 2023-05-12 14:20:44 UTC
Pushed to master for 23.05.

Nice work everyone, thanks!
Comment 9 Matt Blenkinsop 2023-05-15 10:46:12 UTC
Enhancement - not backporting to 22.11.x
Comment 10 Caroline Cyr La Rose 2023-05-16 15:42:46 UTC
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
Comment 11 Andreas Jonsson 2023-06-29 09:34:39 UTC
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.
Comment 12 Nick Clemens 2023-06-29 13:30:19 UTC
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.
Comment 13 Christopher Brannon 2023-11-07 22:12:13 UTC
(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! :)