From 565a2146d8b8e289dba0f6a11c608c4c5ec82a99 Mon Sep 17 00:00:00 2001 From: Nick Clemens Date: Tue, 18 Nov 2025 19:07:19 +0000 Subject: [PATCH] Bug 41267: Add fill_other_biblio_holds_policy circulation rule This patch adds a new rule for itemtypes to allow/prevent filling other holds on a bibliographic record The rule will obey ReservesControlBranch for determining the rules To test: 1 - Apply patches 2 - Setup/choose 2 item types in Administration->Item types 3 - In Administration -> Circulation and fine rules scroll to the bottom 4 - Set one item type to "Fill other holds on record" = 'Yes' and the other to 'No' 5 - Place a next available hold on the record 6 - Checkout the 'Yes' item type to the patron 7 - Their hold should fill 8 - Check in the items 9 - Place a next available hold on the record 10 - Checkout the 'No' itemtype to the patron 11 - Their hold should not be filled 12 - Check in the item and confirm the hold 13 - Checkout the itme to the patron, the hold fills NOTE: While seemingly counter intuitive, the rule controls whether a hold NOT tied to this item will fill Confirming the hold adds an itemnumber to the hold. In most cases, these items will not be holdable, so won't trigger at checkin 14 - Set a rule for the patrons branch to No and set system preference ReservesControlBranch to 'Patrons library' 15 - repeat 5&6 16 - The hold should not fill 17 - Change reserves control branch to items home library 18 - checkin /checkout the yes item type 19 - The hold should fill --- C4/Reserves.pm | 35 ++++-- Koha/CirculationRules.pm | 4 + admin/smart-rules.pl | 101 ++++++++++-------- .../prog/en/modules/admin/smart-rules.tt | 17 ++- t/db_dependent/Reserves.t | 34 +++++- 5 files changed, 136 insertions(+), 55 deletions(-) diff --git a/C4/Reserves.pm b/C4/Reserves.pm index 624cf08444d..7ef2f40918f 100644 --- a/C4/Reserves.pm +++ b/C4/Reserves.pm @@ -2198,13 +2198,13 @@ sub MoveReserve { my $lookahead = C4::Context->preference('ConfirmFutureHolds'); #number of days to look for future holds my ( $restype, $res, undef ) = CheckReserves( $item, $lookahead ); + my $hold; if ( $res && $res->{borrowernumber} == $patron->borrowernumber ) { - my $hold = Koha::Holds->find( $res->{reserve_id} ); - $hold->fill( { item_id => $item->id } ); + $hold = Koha::Holds->find( $res->{reserve_id} ); } else { - # The item is reserved by someone else. - # Find this item in the reserves + # The next reserve is for someone else, + # Check if this patron has a reserve on this biblio/item my $lookahead_date = output_pref( { @@ -2212,7 +2212,7 @@ sub MoveReserve { dateformat => 'iso', dateonly => 1 } ); - my $hold = $patron->holds->search( + $hold = $patron->holds->search( { biblionumber => $item->biblionumber, reservedate => { '<=' => $lookahead_date }, @@ -2220,17 +2220,38 @@ sub MoveReserve { }, { order_by => 'priority' } )->next(); + } - if ($hold) { + if ($hold) { + if ( $hold->itemnumber && $hold->itemnumber == $item->id ) { # The item is reserved by the current patron $hold->fill( { item_id => $item->id } ); + } else { + my $controlbranch = $patron->branchcode; + if ( C4::Context->preference('ReservesControlBranch') eq 'ItemHomeLibrary' ) { + $controlbranch = $item->homebranch; + } + + # If no rule is set, the default behavior is to fill the hold + my $fill_other_biblio_holds_policy = Koha::CirculationRules->get_effective_rule_value( + { + categorycode => undef, + itemtype => $item->itype, + branchcode => $controlbranch, + rule_name => 'fill_other_biblio_holds_policy', + } + ) // 1; + + $hold->fill( { item_id => $item->id } ) if $fill_other_biblio_holds_policy; } + } + if ( $cancelreserve && $res ) { $hold = Koha::Holds->find( $res->{reserve_id} ); if ( $cancelreserve eq 'revert' ) { $hold->revert_found(); - } elsif ( $cancelreserve eq 'cancel' || $cancelreserve ) { # cancel reserves on this item + } else { # cancel reserves on this item $hold->cancel; } } diff --git a/Koha/CirculationRules.pm b/Koha/CirculationRules.pm index b0b2bd00c2b..b5fbc3c1ab4 100644 --- a/Koha/CirculationRules.pm +++ b/Koha/CirculationRules.pm @@ -74,6 +74,10 @@ our $RULE_KINDS = { scope => [ 'branchcode', 'itemtype' ], can_be_blank => 0, }, + fill_other_biblio_holds_policy => { + scope => [ 'branchcode', 'itemtype' ], + can_be_blank => 0, + }, returnbranch => { scope => [ 'branchcode', 'itemtype' ], can_be_blank => 0, diff --git a/admin/smart-rules.pl b/admin/smart-rules.pl index 7f5448f511c..3e2488644f9 100755 --- a/admin/smart-rules.pl +++ b/admin/smart-rules.pl @@ -212,11 +212,12 @@ if ( $op eq 'cud-delete' ) { branchcode => undef, itemtype => undef, rules => { - holdallowed => undef, - hold_fulfillment_policy => undef, - bookings_lead_period => undef, - bookings_trail_period => undef, - returnbranch => undef, + holdallowed => undef, + hold_fulfillment_policy => undef, + fill_other_biblio_holds_policy => undef, + bookings_lead_period => undef, + bookings_trail_period => undef, + returnbranch => undef, } } ); @@ -226,11 +227,12 @@ if ( $op eq 'cud-delete' ) { branchcode => undef, itemtype => $itemtype, rules => { - holdallowed => undef, - hold_fulfillment_policy => undef, - bookings_lead_period => undef, - bookings_trail_period => undef, - returnbranch => undef, + holdallowed => undef, + hold_fulfillment_policy => undef, + fill_other_biblio_holds_policy => undef, + bookings_lead_period => undef, + bookings_trail_period => undef, + returnbranch => undef, } } ); @@ -241,11 +243,12 @@ if ( $op eq 'cud-delete' ) { branchcode => $branch, itemtype => undef, rules => { - holdallowed => undef, - hold_fulfillment_policy => undef, - bookings_lead_period => undef, - bookings_trail_period => undef, - returnbranch => undef, + holdallowed => undef, + hold_fulfillment_policy => undef, + fill_other_biblio_holds_policy => undef, + bookings_lead_period => undef, + bookings_trail_period => undef, + returnbranch => undef, } } ); @@ -255,11 +258,12 @@ if ( $op eq 'cud-delete' ) { branchcode => $branch, itemtype => $itemtype, rules => { - holdallowed => undef, - hold_fulfillment_policy => undef, - bookings_lead_period => undef, - bookings_trail_period => undef, - returnbranch => undef, + holdallowed => undef, + hold_fulfillment_policy => undef, + fill_other_biblio_holds_policy => undef, + bookings_lead_period => undef, + bookings_trail_period => undef, + returnbranch => undef, } } ); @@ -609,12 +613,13 @@ elsif ( $op eq 'cud-add' ) { } ); } elsif ( $op eq "cud-add-branch-item" ) { - my $itemtype = $input->param('itemtype'); - my $holdallowed = $input->param('holdallowed'); - my $hold_fulfillment_policy = $input->param('hold_fulfillment_policy'); - my $bookings_lead_period = $input->param('bookings_lead_period'); - my $bookings_trail_period = $input->param('bookings_trail_period'); - my $returnbranch = $input->param('returnbranch'); + my $itemtype = $input->param('itemtype'); + my $holdallowed = $input->param('holdallowed'); + my $hold_fulfillment_policy = $input->param('hold_fulfillment_policy'); + my $fill_other_biblio_holds_policy = $input->param('fill_other_biblio_holds_policy'); + my $bookings_lead_period = $input->param('bookings_lead_period'); + my $bookings_trail_period = $input->param('bookings_trail_period'); + my $returnbranch = $input->param('returnbranch'); if ( $branch eq "*" ) { if ( $itemtype eq "*" ) { @@ -623,11 +628,12 @@ elsif ( $op eq 'cud-add' ) { itemtype => undef, branchcode => undef, rules => { - holdallowed => $holdallowed, - hold_fulfillment_policy => $hold_fulfillment_policy, - bookings_lead_period => $bookings_lead_period, - bookings_trail_period => $bookings_trail_period, - returnbranch => $returnbranch, + holdallowed => $holdallowed, + hold_fulfillment_policy => $hold_fulfillment_policy, + fill_other_biblio_holds_policy => $fill_other_biblio_holds_policy, + bookings_lead_period => $bookings_lead_period, + bookings_trail_period => $bookings_trail_period, + returnbranch => $returnbranch, } } ); @@ -637,11 +643,12 @@ elsif ( $op eq 'cud-add' ) { itemtype => $itemtype, branchcode => undef, rules => { - holdallowed => $holdallowed, - hold_fulfillment_policy => $hold_fulfillment_policy, - bookings_lead_period => $bookings_lead_period, - bookings_trail_period => $bookings_trail_period, - returnbranch => $returnbranch, + holdallowed => $holdallowed, + hold_fulfillment_policy => $hold_fulfillment_policy, + fill_other_biblio_holds_policy => $fill_other_biblio_holds_policy, + bookings_lead_period => $bookings_lead_period, + bookings_trail_period => $bookings_trail_period, + returnbranch => $returnbranch, } } ); @@ -652,11 +659,12 @@ elsif ( $op eq 'cud-add' ) { itemtype => undef, branchcode => $branch, rules => { - holdallowed => $holdallowed, - hold_fulfillment_policy => $hold_fulfillment_policy, - bookings_lead_period => $bookings_lead_period, - bookings_trail_period => $bookings_trail_period, - returnbranch => $returnbranch, + holdallowed => $holdallowed, + hold_fulfillment_policy => $hold_fulfillment_policy, + fill_other_biblio_holds_policy => $fill_other_biblio_holds_policy, + bookings_lead_period => $bookings_lead_period, + bookings_trail_period => $bookings_trail_period, + returnbranch => $returnbranch, } } ); @@ -666,11 +674,12 @@ elsif ( $op eq 'cud-add' ) { itemtype => $itemtype, branchcode => $branch, rules => { - holdallowed => $holdallowed, - hold_fulfillment_policy => $hold_fulfillment_policy, - bookings_lead_period => $bookings_lead_period, - bookings_trail_period => $bookings_trail_period, - returnbranch => $returnbranch, + holdallowed => $holdallowed, + fill_other_biblio_holds_policy => $fill_other_biblio_holds_policy, + hold_fulfillment_policy => $hold_fulfillment_policy, + bookings_lead_period => $bookings_lead_period, + bookings_trail_period => $bookings_trail_period, + returnbranch => $returnbranch, } } ); diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/smart-rules.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/smart-rules.tt index bd0526017da..7df73fa6b27 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/smart-rules.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/smart-rules.tt @@ -1310,6 +1310,7 @@ Item type Hold policy + Fill other holds on record Hold and booking pickup library match Booking pre-processing (days) Booking post-processing (days) @@ -1320,12 +1321,13 @@ [% SET it = i.itemtype %] [% SET c = undef %] [% SET holdallowed = all_rules.$c.$it.holdallowed %] + [% SET fill_other_biblio_holds_policy = all_rules.$c.$it.fill_other_biblio_holds_policy %] [% SET hold_fulfillment_policy = all_rules.$c.$it.hold_fulfillment_policy %] [% SET bookings_lead_period = all_rules.$c.$it.bookings_lead_period %] [% SET bookings_trail_period = all_rules.$c.$it.bookings_trail_period %] [% SET returnbranch = all_rules.$c.$it.returnbranch %] - [% IF holdallowed || hold_fulfillment_policy || returnbranch || bookings_lead_period || bookings_trail_period %] + [% IF holdallowed || hold_fulfillment_policy || fill_other_biblio_holds_policy || returnbranch || bookings_lead_period || bookings_trail_period %] [% i.translated_description | html %] @@ -1339,6 +1341,13 @@ No holds allowed [% END %] + + [% IF fill_other_biblio_holds_policy %] + Yes + [% ELSE %] + No + [% END %] + [% IF hold_fulfillment_policy == 'any' %] any library @@ -1391,6 +1400,12 @@ + + +