From 43a38d4c6535d2c439660f302e570a5047828b48 Mon Sep 17 00:00:00 2001 From: Thibaud Guillot Date: Mon, 27 Nov 2023 16:27:42 +0100 Subject: [PATCH] Bug 8137: Add 'global' has_priority rules for checkouts I tried to review the desired behavior through this bug. The goal would be to be able to have a general 'override' rule. Currently, if you set a max_checkouts_allowed of 5 (regardless of itemtype) in one library, for example, and do the same in another, it adds up to 10. By giving checkout rules priority, you "overwrite" other local rules per branchcode. Test plan: 1) Set for library "A" a max_checkouts_allowed to 3 for any type of user and item type. 2) On main rules set the same rule to 2 3) Do a few checkouts with a user belonging to library A, normally you'll be able to do 3. 4) Apply this patch and run updatedatabase.pl and also rebuild your schema by updating dbix class files 5) See now the Has priority checkboxes on "Default checkout, hold and return policy" and "Default checkout, hold policy by patron category" 6) You can set here you new rule for "Total current checkouts allowed" to 2 7) Repeat step 3 and normally you won't be able to make the third --- C4/Circulation.pm | 58 +++++++++++++++++-- Koha/CirculationRules.pm | 33 ++++++++--- admin/smart-rules.pl | 11 ++++ .../Bug_8137-add-column-has_priority.pl | 13 +++++ .../prog/en/modules/admin/smart-rules.tt | 24 +++++++- 5 files changed, 125 insertions(+), 14 deletions(-) create mode 100644 installer/data/mysql/atomicupdate/Bug_8137-add-column-has_priority.pl diff --git a/C4/Circulation.pm b/C4/Circulation.pm index ac28b2e74b..318439bae4 100644 --- a/C4/Circulation.pm +++ b/C4/Circulation.pm @@ -461,6 +461,39 @@ sub TooMany { } ); + my $default_priority_no_branch_no_cat_patron_maxissueqty = Koha::CirculationRules->get_effective_rule( + { + rule_name => 'patron_maxissueqty', + has_priority => 1, + } + ); + + my $default_priority_no_branch_patron_maxissueqty = Koha::CirculationRules->get_effective_rule( + { + categorycode => $cat_borrower, + rule_name => 'patron_maxissueqty', + has_priority => 1, + } + ); + + my $default_priority_patron_maxissueqty = $default_priority_no_branch_patron_maxissueqty->rule_value // $default_priority_no_branch_no_cat_patron_maxissueqty->rule_value; + + my $default_priority_no_branch_no_cat_patron_maxonsiteissueqty = Koha::CirculationRules->get_effective_rule( + { + rule_name => 'patron_maxonsiteissueqty', + has_priority => 1, + } + ); + my $default_priority_no_branch_patron_maxonsiteissueqty = Koha::CirculationRules->get_effective_rule( + { + categorycode => $cat_borrower, + rule_name => 'patron_maxonsiteissueqty', + has_priority => 1, + } + ); + + my $default_priority_patron_maxonsiteissueqty = $default_priority_no_branch_patron_maxonsiteissueqty->rule_value // $default_priority_no_branch_no_cat_patron_maxonsiteissueqty->rule_value; + # if a rule is found and has a loan limit set, count # how many loans the patron already has that meet that # rule @@ -534,12 +567,15 @@ sub TooMany { my $checkout_count = $sum_checkouts->{total} || 0; my $onsite_checkout_count = $sum_checkouts->{onsite_checkouts} || 0; + my $max_checkouts_allowed = $default_priority_patron_maxissueqty ? $default_priority_patron_maxissueqty : $maxissueqty_rule->rule_value; + my $max_onsite_checkouts_allowed = $default_priority_patron_maxonsiteissueqty ? $default_priority_patron_maxonsiteissueqty : $maxonsiteissueqty_rule->rule_value; + my $checkout_rules = { checkout_count => $checkout_count, onsite_checkout_count => $onsite_checkout_count, onsite_checkout => $onsite_checkout, - max_checkouts_allowed => $maxissueqty_rule ? $maxissueqty_rule->rule_value : undef, - max_onsite_checkouts_allowed => $maxonsiteissueqty_rule ? $maxonsiteissueqty_rule->rule_value : undef, + max_checkouts_allowed => $max_checkouts_allowed, + max_onsite_checkouts_allowed => $max_onsite_checkouts_allowed, switch_onsite_checkout => $switch_onsite_checkout, }; # If parent rules exists @@ -579,8 +615,8 @@ sub TooMany { my $checkout_count = $checkouts->count; my $onsite_checkout_count = $checkouts->search({ onsite_checkout => 1 })->count; - my $max_checkouts_allowed = $branch_borrower_circ_rule->{patron_maxissueqty}; - my $max_onsite_checkouts_allowed = $branch_borrower_circ_rule->{patron_maxonsiteissueqty} || undef; + my $max_checkouts_allowed = $default_priority_patron_maxissueqty ? $default_priority_patron_maxissueqty : $branch_borrower_circ_rule->{patron_maxissueqty}; + my $max_onsite_checkouts_allowed = $default_priority_patron_maxonsiteissueqty ? $default_priority_patron_maxonsiteissueqty : $branch_borrower_circ_rule->{patron_maxonsiteissueqty}; my $qty_over = _check_max_qty( { @@ -603,6 +639,20 @@ sub TooMany { return; } +sub _get_has_priority_circulation_rule { + my ($categorycode, $branchcode, $rule_name) = @_; + + return Koha::CirculationRules->get_effective_rule( + { + categorycode => $categorycode, + itemtype => undef, + branchcode => $branchcode, + rule_name => $rule_name, + has_priority => 1, + } + ); +} + sub _check_max_qty { my $params = shift; my $checkout_count = $params->{checkout_count}; diff --git a/Koha/CirculationRules.pm b/Koha/CirculationRules.pm index 1d07476875..a4bb0766a2 100644 --- a/Koha/CirculationRules.pm +++ b/Koha/CirculationRules.pm @@ -216,6 +216,9 @@ our $RULE_KINDS = { holds_pickup_period => { scope => [ 'branchcode', 'categorycode', 'itemtype' ], }, + has_priority => { + scope => [ 'branchcode', 'categorycode' ], + }, # Not included (deprecated?): # * accountsent # * reservecharge @@ -248,11 +251,13 @@ sub get_effective_rule { $params->{categorycode} //= undef; $params->{branchcode} //= undef; $params->{itemtype} //= undef; + $params->{has_priority} //= undef; my $rule_name = $params->{rule_name}; my $categorycode = $params->{categorycode}; my $itemtype = $params->{itemtype}; my $branchcode = $params->{branchcode}; + my $has_priority = $params->{has_priority}; Koha::Exceptions::MissingParameter->throw( "Required parameter 'rule_name' missing") @@ -271,6 +276,7 @@ sub get_effective_rule { $search_params->{categorycode} = defined $categorycode ? [ $categorycode, undef ] : undef; $search_params->{itemtype} = defined $itemtype ? [ $itemtype, undef ] : undef; $search_params->{branchcode} = defined $branchcode ? [ $branchcode, undef ] : undef; + $search_params->{has_priority} = defined $has_priority ? [ $has_priority, undef ] : undef; my $rule = $self->search( $search_params, @@ -308,10 +314,10 @@ sub get_effective_rule_value { my $categorycode = $params->{categorycode}; my $itemtype = $params->{itemtype}; my $branchcode = $params->{branchcode}; - + my $has_priority = $params->{has_priority}; my $memory_cache = Koha::Cache::Memory::Lite->get_instance; my $cache_key = sprintf "CircRules:%s:%s:%s:%s", $rule_name // q{}, - $categorycode // q{}, $branchcode // q{}, $itemtype // q{}; + $categorycode // q{}, $branchcode // q{}, $itemtype // q{} ,$has_priority // q{}; my $cached = $memory_cache->get_from_cache($cache_key); return $cached if $cached; @@ -334,6 +340,7 @@ sub get_effective_rules { my $categorycode = $params->{categorycode}; my $itemtype = $params->{itemtype}; my $branchcode = $params->{branchcode}; + my $has_priority = $params->{has_priority}; my $r; foreach my $rule (@$rules) { @@ -343,9 +350,10 @@ sub get_effective_rules { categorycode => $categorycode, itemtype => $itemtype, branchcode => $branchcode, + has_priority => $has_priority, } ); - + $r->{$rule} = $effective_rule if defined $effective_rule; } @@ -357,7 +365,7 @@ sub get_effective_rules { =cut sub set_rule { - my ( $self, $params ) = @_; + my ( $self, $params ) = @_; for my $mandatory_parameter (qw( rule_name rule_value ) ) { Koha::Exceptions::MissingParameter->throw( @@ -386,6 +394,8 @@ sub set_rule { my $itemtype = $params->{itemtype}; my $rule_name = $params->{rule_name}; my $rule_value = $params->{rule_value}; + my $has_priority = $params->{has_priority}; + my $can_be_blank = defined $kind_info->{can_be_blank} ? $kind_info->{can_be_blank} : 1; $rule_value = undef if defined $rule_value && $rule_value eq "" && !$can_be_blank; my $is_monetary = defined $kind_info->{is_monetary} ? $kind_info->{is_monetary} : 0; @@ -407,6 +417,7 @@ sub set_rule { if ($rule) { if ( defined $rule_value ) { $rule->rule_value($rule_value); + $rule->has_priority($has_priority); $rule->update(); } else { @@ -422,6 +433,7 @@ sub set_rule { itemtype => $itemtype, rule_name => $rule_name, rule_value => $rule_value, + has_priority => $has_priority, } ); $rule->store(); @@ -442,12 +454,17 @@ sub set_rule { sub set_rules { my ( $self, $params ) = @_; - + my %set_params; - $set_params{branchcode} = $params->{branchcode} if exists $params->{branchcode}; - $set_params{categorycode} = $params->{categorycode} if exists $params->{categorycode}; + $set_params{branchcode} = $params->{branchcode} + if exists $params->{branchcode}; + $set_params{categorycode} = $params->{categorycode} + if exists $params->{categorycode}; $set_params{itemtype} = $params->{itemtype} if exists $params->{itemtype}; - my $rules = $params->{rules}; + $set_params{has_priority} = $params->{has_priority} + if exists $params->{has_priority}; + my $rules = $params->{rules}; + my $rule_objects = []; while ( my ( $rule_name, $rule_value ) = each %$rules ) { diff --git a/admin/smart-rules.pl b/admin/smart-rules.pl index b1c7868184..9f4c896511 100755 --- a/admin/smart-rules.pl +++ b/admin/smart-rules.pl @@ -362,12 +362,14 @@ elsif ($op eq "set-branch-defaults") { my $hold_fulfillment_policy = $input->param('hold_fulfillment_policy'); my $returnbranch = $input->param('returnbranch'); my $max_holds = strip_non_numeric( scalar $input->param('max_holds') ); + my $has_priority = $input->param('has_priority') ? 1 : 0; if ($branch eq "*") { Koha::CirculationRules->set_rules( { itemtype => undef, branchcode => undef, + has_priority => $has_priority, rules => { holdallowed => $holdallowed, hold_fulfillment_policy => $hold_fulfillment_policy, @@ -379,6 +381,7 @@ elsif ($op eq "set-branch-defaults") { { categorycode => undef, branchcode => undef, + has_priority => $has_priority, rules => { patron_maxissueqty => $patron_maxissueqty, patron_maxonsiteissueqty => $patron_maxonsiteissueqty, @@ -390,6 +393,7 @@ elsif ($op eq "set-branch-defaults") { { itemtype => undef, branchcode => $branch, + has_priority => $has_priority, rules => { holdallowed => $holdallowed, hold_fulfillment_policy => $hold_fulfillment_policy, @@ -401,6 +405,7 @@ elsif ($op eq "set-branch-defaults") { { categorycode => undef, branchcode => $branch, + has_priority => $has_priority, rules => { patron_maxissueqty => $patron_maxissueqty, patron_maxonsiteissueqty => $patron_maxonsiteissueqty, @@ -424,6 +429,7 @@ elsif ($op eq "add-branch-cat") { $patron_maxonsiteissueqty = strip_non_numeric($patron_maxonsiteissueqty); my $max_holds = $input->param('max_holds'); $max_holds = strip_non_numeric($max_holds); + my $has_priority = $input->param('has_priority') ? 1 : 0; if ($branch eq "*") { if ($categorycode eq "*") { @@ -431,6 +437,7 @@ elsif ($op eq "add-branch-cat") { { categorycode => undef, branchcode => undef, + has_priority => $has_priority, rules => { max_holds => $max_holds, patron_maxissueqty => $patron_maxissueqty, @@ -439,10 +446,12 @@ elsif ($op eq "add-branch-cat") { } ); } else { + Koha::CirculationRules->set_rules( { categorycode => $categorycode, branchcode => undef, + has_priority => $has_priority, rules => { max_holds => $max_holds, patron_maxissueqty => $patron_maxissueqty, @@ -456,6 +465,7 @@ elsif ($op eq "add-branch-cat") { { categorycode => undef, branchcode => $branch, + has_priority => $has_priority, rules => { max_holds => $max_holds, patron_maxissueqty => $patron_maxissueqty, @@ -468,6 +478,7 @@ elsif ($op eq "add-branch-cat") { { categorycode => $categorycode, branchcode => $branch, + has_priority => $has_priority, rules => { max_holds => $max_holds, patron_maxissueqty => $patron_maxissueqty, diff --git a/installer/data/mysql/atomicupdate/Bug_8137-add-column-has_priority.pl b/installer/data/mysql/atomicupdate/Bug_8137-add-column-has_priority.pl new file mode 100644 index 0000000000..6f421ae5ae --- /dev/null +++ b/installer/data/mysql/atomicupdate/Bug_8137-add-column-has_priority.pl @@ -0,0 +1,13 @@ +use Modern::Perl; + +return { + bug_number => "BUG_8137", + description => "Add column has_priority", + up => sub { + my ($args) = @_; + my ( $dbh, $out ) = @$args{qw(dbh out)}; + + $dbh->do('ALTER TABLE circulation_rules ADD IF NOT EXISTS has_priority INT(1) DEFAULT 0'); + say $out "Added column 'has_priority'"; + }, +}; 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 00de0896bc..4ceb5227d9 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 @@ -634,6 +634,7 @@ Hold policy Hold pickup library match Return policy + Has priority Actions [% SET patron_maxissueqty = CirculationRules.Search( current_branch, undef, undef, 'patron_maxissueqty', { want_rule => 1 } ) %] @@ -643,6 +644,7 @@ [% SET hold_fulfillment_policy = CirculationRules.Search( current_branch, undef, undef, 'hold_fulfillment_policy', { want_rule => 1 }) %] [% SET returnbranch = CirculationRules.Search( current_branch, undef, undef, 'returnbranch', { want_rule => 1 }) %] [% SET default_checkout_hold_and_return_policy = ( patron_maxissueqty || patron_maxonsiteissueqty || rule_value || holdallowed || hold_fulfillment_policy || returnbranch ) %] + [% SET has_priority = patron_maxissueqty.has_priority || patron_maxonsiteissueqty.has_priority %] [% IF ( default_checkout_hold_and_return_policy ) %] @@ -796,6 +798,9 @@ + + + [% IF ( default_checkout_hold_and_return_policy ) %] @@ -825,6 +830,7 @@ Total current checkouts allowed Total current on-site checkouts allowed Total holds allowed + Has priority   [% FOREACH c IN categorycodes %] @@ -832,7 +838,11 @@ [% SET patron_maxissueqty = CirculationRules.Search( branchcode, c, undef, 'patron_maxissueqty' ) %] [% SET patron_maxonsiteissueqty = CirculationRules.Search( branchcode, c, undef, 'patron_maxonsiteissueqty' ) %] [% SET max_holds = CirculationRules.Search( branchcode, c, undef, 'max_holds' ) %] - + [% SET nobranch_patron_maxissueqty = CirculationRules.Search( undef, c, undef, 'patron_maxissueqty', {want_rule => 1} ) %] + [% SET nobranch_patron_maxonsiteissueqty = CirculationRules.Search( undef, c, undef, 'patron_maxonsiteissueqty', {want_rule => 1} ) %] + [% SET nobranch_max_holds = CirculationRules.Search( undef, c, undef, 'max_holds', {want_rule => 1} ) %] + [% SET has_priority = nobranch_patron_maxissueqty.has_priority || nobranch_patron_maxonsiteissueqty.has_priority %] + [% IF ( patron_maxissueqty.defined && patron_maxissueqty != '' ) || ( patron_maxonsiteissueqty.defined && patron_maxonsiteissueqty != '' ) || ( max_holds.defined && max_holds != '' ) %] @@ -863,7 +873,9 @@ Unlimited [% END %] - + + + Delete @@ -881,6 +893,9 @@ + + + @@ -1582,6 +1597,11 @@ } return true; }); + if($("#branch").val() != '*') { + $(".hasPriority").each(function() { + $(this).remove(); + }); + } }); [% END %] -- 2.30.2