Bug 31557

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! :)