From a2ef73fe957e2f17b001ace61c0ff7c553d4dbdd Mon Sep 17 00:00:00 2001 From: Martin Renvoize Date: Wed, 15 Apr 2026 17:59:43 +0100 Subject: [PATCH] Bug 41410: Add local holds priority exclusivity period When LocalHoldsPriority is enabled, the holds queue picks a local-group item for eligible holds. Until now there was nothing to reserve that priority at checkin time: if a non-local item was returned first, it would happily fill the hold and the local-priority decision was lost. This patch adds a time-bounded exclusivity window so that, for a configurable number of days after a hold is placed, a hold whose queue target is a local-group item will not be filled by a non-local item. Changes: - C4/HoldsQueue.pm: in MapItemsToHoldRequests, flag %item_map entries that matched on local library (GiveLibrary/GiveLibraryAndGroup) or local hold group (GiveLibraryGroup/GiveLibraryAndGroup) with local_holdgroup_match => 1, and propagate that into both the tmp_holdsqueue and hold_fill_targets INSERTs. - C4/Reserves.pm: in the CheckReserves loop, when LocalHoldsPriorityExclusivityPeriod is set, the hold is still within that window, and the item being evaluated is not itself a local match, skip the reserve if its hold_fill_targets row has local_holdgroup_match = 1. Missing hold_fill_targets row means "no information" -- fall through so holds are never trapped between queue runs. - circulation.pref: expose the new LocalHoldsPriorityExclusivityPeriod preference next to the existing LocalHoldsPriority* entries. The feature is a no-op unless both LocalHoldsPriority is set to something other than "Don't give" AND LocalHoldsPriorityExclusivityPeriod is > 0. Test plan: 1) Apply the patches and run the atomicupdate. 2) Set up a library group flagged as a local hold group containing two libraries (call them A and B). Add a third library C that is NOT in the group. 3) Add one item to each library (itemA, itemB, itemC) on the same biblio. 4) Administration -> System preferences -> Circulation: - Set LocalHoldsPriority to "Give library group". - Set LocalHoldsPriorityPatronControl to "home library". - Set LocalHoldsPriorityItemControl to "home library". - Set LocalHoldsPriorityExclusivityPeriod to 7. 5) As a patron whose home library is A, place a title-level hold with pickup at A. 6) Run the holds queue (misc/cronjobs/holds/build_holds_queue.pl). -> The queue should target itemA (or itemB) for this hold. -> Confirm in the database: SELECT reserve_id, itemnumber, local_holdgroup_match FROM hold_fill_targets; The targeted row must have local_holdgroup_match = 1. -> Holds to pull / holds queue report should also show the flag (tmp_holdsqueue.local_holdgroup_match = 1). 7) At library C, check in itemC (the non-local item). -> The hold must NOT be captured -- itemC simply goes back on the shelf. 8) At library A, check in itemA (the local item). -> The hold IS captured for the patron. 9) Cancel and re-place the hold, run the queue again. Manually age the hold past the window: UPDATE reserves SET reservedate = DATE_SUB(CURRENT_DATE, INTERVAL 10 DAY) WHERE reserve_id = ; Now check in itemC again. -> The hold IS captured (exclusivity window has expired). 10) Re-place the hold but do NOT run the queue. With no hold_fill_targets row for the reserve, check in itemC. -> The hold IS captured (we never trap holds between queue runs). 11) Flip LocalHoldsPriorityExclusivityPeriod back to 0 and repeat step 7. -> The hold IS captured; the feature is fully disabled. 12) Run: prove t/db_dependent/Holds/LocalHoldsPriority.t \ t/db_dependent/HoldsQueue.t \ t/db_dependent/Reserves.t \ t/db_dependent/Koha/Hold.t -> All tests pass. Sponsored-by: Main Library Alliance Signed-off-by: Trevor Diamond Signed-off-by: Andrew Fuerste Henry Signed-off-by: Pedro Amorim --- C4/HoldsQueue.pm | 44 ++++++++++--------- C4/Reserves.pm | 25 +++++++++++ .../admin/preferences/circulation.pref | 6 +++ 3 files changed, 55 insertions(+), 20 deletions(-) diff --git a/C4/HoldsQueue.pm b/C4/HoldsQueue.pm index 33b6eb6873..8a23493142 100644 --- a/C4/HoldsQueue.pm +++ b/C4/HoldsQueue.pm @@ -798,10 +798,11 @@ sub MapItemsToHoldRequests { holdingbranch => $item->{holdingbranch}, pickup_branch => $request->{branchcode} || $request->{borrowerbranch}, - reserve_id => $request->{reserve_id}, - item_level => $request->{item_level_hold}, - reservedate => $request->{reservedate}, - reservenotes => $request->{reservenotes}, + reserve_id => $request->{reserve_id}, + item_level => $request->{item_level_hold}, + reservedate => $request->{reservedate}, + reservenotes => $request->{reservenotes}, + local_holdgroup_match => 1, }; $allocated_items{ $item->{itemnumber} }++; $request->{allocated} = 1; @@ -848,10 +849,11 @@ sub MapItemsToHoldRequests { holdingbranch => $item->{holdingbranch}, pickup_branch => $request->{branchcode} || $request->{borrowerbranch}, - reserve_id => $request->{reserve_id}, - item_level => $request->{item_level_hold}, - reservedate => $request->{reservedate}, - reservenotes => $request->{reservenotes}, + reserve_id => $request->{reserve_id}, + item_level => $request->{item_level_hold}, + reservedate => $request->{reservedate}, + reservenotes => $request->{reservenotes}, + local_holdgroup_match => 1, }; $allocated_items{ $item->{itemnumber} }++; $request->{allocated} = 1; @@ -1086,19 +1088,20 @@ sub CreatePicklistFromItemMap { my $sth_load = $dbh->prepare( " INSERT INTO tmp_holdsqueue (biblionumber,itemnumber,barcode,surname,firstname,phone,borrowernumber, cardnumber,reservedate,title, itemcallnumber, - holdingbranch,pickbranch,notes, hold_group_id, item_level_request) + holdingbranch,pickbranch,notes, hold_group_id, item_level_request, local_holdgroup_match) VALUES (?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?) " ); foreach my $itemnumber ( sort keys %$item_map ) { - my $mapped_item = $item_map->{$itemnumber}; - my $biblionumber = $mapped_item->{biblionumber}; - my $borrowernumber = $mapped_item->{borrowernumber}; - my $pickbranch = $mapped_item->{pickup_branch}; - my $holdingbranch = $mapped_item->{holdingbranch}; - my $reservedate = $mapped_item->{reservedate}; - my $reservenotes = $mapped_item->{reservenotes}; - my $item_level = $mapped_item->{item_level}; + my $mapped_item = $item_map->{$itemnumber}; + my $biblionumber = $mapped_item->{biblionumber}; + my $borrowernumber = $mapped_item->{borrowernumber}; + my $pickbranch = $mapped_item->{pickup_branch}; + my $holdingbranch = $mapped_item->{holdingbranch}; + my $reservedate = $mapped_item->{reservedate}; + my $reservenotes = $mapped_item->{reservenotes}; + my $item_level = $mapped_item->{item_level}; + my $local_holdgroup_match = $mapped_item->{local_holdgroup_match} ? 1 : 0; my $item = Koha::Items->find($itemnumber); my $barcode = $item->barcode; @@ -1123,7 +1126,7 @@ sub CreatePicklistFromItemMap { $sth_load->execute( $biblionumber, $itemnumber, $barcode, $surname, $firstname, $phone, $borrowernumber, $cardnumber, $reservedate, $title, $itemcallnumber, - $holdingbranch, $pickbranch, $reservenotes, $hold_group_id, $item_level + $holdingbranch, $pickbranch, $reservenotes, $hold_group_id, $item_level, $local_holdgroup_match ); } } @@ -1139,7 +1142,7 @@ sub AddToHoldTargetMap { my $schema = Koha::Database->new->schema; my $insert_sql = q( - INSERT INTO hold_fill_targets (borrowernumber, biblionumber, itemnumber, source_branchcode, item_level_request, reserve_id) VALUES (?, ?, ?, ?, ?, ?) + INSERT INTO hold_fill_targets (borrowernumber, biblionumber, itemnumber, source_branchcode, item_level_request, reserve_id, local_holdgroup_match) VALUES (?, ?, ?, ?, ?, ?, ?) ); my $sth_insert = $dbh->prepare($insert_sql); @@ -1150,7 +1153,8 @@ sub AddToHoldTargetMap { $dbh->do( 'DELETE FROM hold_fill_targets WHERE itemnumber = ?', {}, $itemnumber ); $sth_insert->execute( $mapped_item->{borrowernumber}, $mapped_item->{biblionumber}, $itemnumber, - $mapped_item->{holdingbranch}, $mapped_item->{item_level}, $mapped_item->{reserve_id} + $mapped_item->{holdingbranch}, $mapped_item->{item_level}, $mapped_item->{reserve_id}, + $mapped_item->{local_holdgroup_match} ? 1 : 0, ); } ); diff --git a/C4/Reserves.pm b/C4/Reserves.pm index 0bb4e60c56..1abef7ac71 100644 --- a/C4/Reserves.pm +++ b/C4/Reserves.pm @@ -895,6 +895,31 @@ sub CheckReserves { } } + # Local holds priority exclusivity period: when the queue has + # targeted a local-group item for this hold and we're still + # within the exclusivity window, skip non-local items so the + # targeted local item can fulfil it. If the queue has not yet + # run for this reserve (no hold_fill_targets row), we fall + # through and let the hold be filled normally. + my $excl_period = C4::Context->preference('LocalHoldsPriorityExclusivityPeriod'); + if ( $excl_period + && !$local_hold_match + && !$local_hold_group_match + && $res->{reserve_id} ) + { + my $reservedate_dt = eval { dt_from_string( $res->{reservedate} ) }; + if ($reservedate_dt) { + my $age_days = dt_from_string->delta_days($reservedate_dt)->in_units('days'); + if ( $age_days < $excl_period ) { + my $target = + Koha::Database->new->schema->resultset('HoldFillTarget') + ->search( { reserve_id => $res->{reserve_id} }, { rows => 1 } ) + ->next; + next if $target && $target->local_holdgroup_match; + } + } + } + # See if this item is more important than what we've got so far if ( ( $res->{'priority'} && $res->{'priority'} < $priority ) || $local_hold_match diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref index 71721ab086..a27cb4b6b8 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref @@ -1103,6 +1103,12 @@ Circulation: homebranch: "home library" holdingbranch: "holding library" -
Note: If a group option is selected, LocalHoldsPriorityPatronControl and LocalHoldsPriorityItemControl will use the library hold group of the selected libraries. + - + - For + - pref: LocalHoldsPriorityExclusivityPeriod + class: integer + - days after a hold is placed, only items flagged by the holds queue as a local hold group match may be used to fill the hold. Set to 0 to disable. + -
Note: Only takes effect when LocalHoldsPriority is set to something other than 'Don't give'. - - pref: OPACHoldsIfAvailableAtPickup choices: