Bugzilla – Attachment 189682 Details for
Bug 41267
It should be possible to prevent some itemtypes from filling other biblio level holds
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 41267: Add fill_other_biblio_holds_policy circulation rule
Bug-41267-Add-fillotherbiblioholdspolicy-circulati.patch (text/plain), 17.17 KB, created by
Nick Clemens (kidclamp)
on 2025-11-18 19:18:25 UTC
(
hide
)
Description:
Bug 41267: Add fill_other_biblio_holds_policy circulation rule
Filename:
MIME Type:
Creator:
Nick Clemens (kidclamp)
Created:
2025-11-18 19:18:25 UTC
Size:
17.17 KB
patch
obsolete
>From 41bfb1535b4d4f1bc9ee62029c60926711d1aad2 Mon Sep 17 00:00:00 2001 >From: Nick Clemens <nick@bywatersolutions.com> >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 hold on the 'No' item type for a patron > 6 - Checkout the 'Yes' item type to the patron > 7 - Their hold should fill > 8 - Check in the items - place a hold for the 'Yes' item type > 9 - Checkout the 'No' itemtype to the patron >10 - Their hold should not be filled >11 - Set a rule for the patrons branch to No and set system preference ReservesControlBranch to 'Patrons library' >12 - repeat 5&6 >13 - The hold should not fill >14 - Change reserves control branch to items home library >15 - checkin /checkout the yes item type >16 - The hold should fill >--- > C4/Reserves.pm | 21 +++- > 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, 127 insertions(+), 50 deletions(-) > >diff --git a/C4/Reserves.pm b/C4/Reserves.pm >index c4eb3b7ff69..00a163eecda 100644 >--- a/C4/Reserves.pm >+++ b/C4/Reserves.pm >@@ -2225,9 +2225,26 @@ sub MoveReserve { > )->next(); > > if ($hold) { >+ if ( $hold->itemnumber ) { > >- # The item is reserved by the current patron >- $hold->fill( { item_id => $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; >+ } >+ >+ 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', >+ } >+ ); >+ $hold->fill( { item_id => $item->id } ) if $fill_other_biblio_holds_policy; >+ } > } > > $hold = Koha::Holds->find( $res->{reserve_id} ); >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 a1b9d79a41e..12426d44900 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 @@ > <tr> > <th>Item type</th> > <th>Hold policy</th> >+ <th>Fill other holds on record</th> > <th>Hold and booking pickup library match</th> > <th>Booking pre-processing (days)</th> > <th>Booking post-processing (days)</th> >@@ -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 %] > <tr> > <td> [% i.translated_description | html %] </td> > <td> >@@ -1339,6 +1341,13 @@ > <span>No holds allowed</span> > [% END %] > </td> >+ <td> >+ [% IF fill_other_biblio_holds_policy %] >+ <span> Yes </span> >+ [% ELSE %] >+ <span> No </span> >+ [% END %] >+ </td> > <td> > [% IF hold_fulfillment_policy == 'any' %] > <span>any library</span> >@@ -1391,6 +1400,12 @@ > <option value="not_allowed">No holds allowed</option> > </select> > </td> >+ <td> >+ <select name="fill_other_biblio_holds_policy"> >+ <option value="1" selected="selected"> Yes </option> >+ <option value="0"> No </option> >+ </select> >+ </td> > <td> > <select name="hold_fulfillment_policy"> > <option value="any"> any library </option> >diff --git a/t/db_dependent/Reserves.t b/t/db_dependent/Reserves.t >index 09dea6da2d9..d77d5a41f71 100755 >--- a/t/db_dependent/Reserves.t >+++ b/t/db_dependent/Reserves.t >@@ -1027,7 +1027,7 @@ subtest 'ChargeReserveFee tests' => sub { > > subtest 'MoveReserve additional test' => sub { > >- plan tests => 8; >+ plan tests => 10; > > # Create the items and patrons we need > my $biblio = $builder->build_sample_biblio(); >@@ -1098,6 +1098,38 @@ subtest 'MoveReserve additional test' => sub { > is( $patron_2->holds->count, 0, "The 2nd patron no longer has a hold" ); > is( $patron_2->old_holds->count(), 2, "The 2nd patron's hold was filled and moved to old holds" ); > >+ my $reserve_4 = AddReserve( >+ { >+ branchcode => $item_2->homebranch, >+ borrowernumber => $patron_2->borrowernumber, >+ biblionumber => $biblio->biblionumber, >+ priority => 1, >+ itemnumber => undef, >+ } >+ ); >+ Koha::CirculationRules->set_rule( >+ { >+ branchcode => $item_1->homebranch, >+ itemtype => $item_1->itype, >+ rule_name => 'fill_other_biblio_holds_policy', >+ rule_value => 0 >+ } >+ ); >+ Koha::CirculationRules->set_rule( >+ { >+ branchcode => $patron_2->branchcode, >+ itemtype => $item_1->itype, >+ rule_name => 'fill_other_biblio_holds_policy', >+ rule_value => 1 >+ } >+ ); >+ t::lib::Mocks::mock_preference( 'ReservesControlBranch', 'ItemHomeLibrary' ); >+ MoveReserve( $item_1, $patron_2 ); >+ is( $patron_2->holds->count, 1, "The hold is not filled because of circ rules" ); >+ t::lib::Mocks::mock_preference( 'ReservesControlBranch', 'PatronLibrary' ); >+ MoveReserve( $item_1, $patron_2 ); >+ is( $patron_2->holds->count, 0, "The hold is filled because of circ rules and affected by ReservesControlBranch" ); >+ > }; > > # FIXME: Should be in Circulation.t >-- >2.39.5
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 41267
: 189682