From 79a4ef1b078d5c60c578f6966d609b4f95477c0a Mon Sep 17 00:00:00 2001 From: Thibaud Guillot Date: Fri, 23 Feb 2024 10:14:40 +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 Sponsored-by: BibLibre --- C4/Circulation.pm | 69 +++++++++++++++++-- Koha/CirculationRules.pm | 27 ++++++-- admin/smart-rules.pl | 10 +++ .../Bug_8137-add-column-has_priority.pl | 13 ++++ .../prog/en/modules/admin/smart-rules.tt | 21 +++++- 5 files changed, 130 insertions(+), 10 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 d8b75fe8585..97cc3756f65 100644 --- a/C4/Circulation.pm +++ b/C4/Circulation.pm @@ -461,6 +461,50 @@ 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; + if ($default_priority_no_branch_patron_maxissueqty) { + $default_priority_patron_maxissueqty = $default_priority_no_branch_patron_maxissueqty->rule_value; + } elsif ($default_priority_no_branch_no_cat_patron_maxissueqty) { + $default_priority_patron_maxissueqty = $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; + if ($default_priority_no_branch_patron_maxonsiteissueqty) { + $default_priority_patron_maxonsiteissueqty = $default_priority_no_branch_patron_maxonsiteissueqty->rule_value; + } elsif ($default_priority_no_branch_no_cat_patron_maxonsiteissueqty) { + $default_priority_patron_maxonsiteissueqty = $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 +578,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, circulation_rule => $maxissueqty_rule, onsite_circulation_rule => $maxonsiteissueqty_rule, @@ -581,8 +628,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( { @@ -606,6 +653,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 1d074768754..48e8b70b197 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,6 +350,7 @@ sub get_effective_rules { categorycode => $categorycode, itemtype => $itemtype, branchcode => $branchcode, + has_priority => $has_priority, } ); @@ -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(); @@ -444,10 +456,15 @@ 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 efe1b91dfb5..c864231089f 100755 --- a/admin/smart-rules.pl +++ b/admin/smart-rules.pl @@ -365,12 +365,14 @@ elsif ($op eq "cud-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, @@ -382,6 +384,7 @@ elsif ($op eq "cud-set-branch-defaults") { { categorycode => undef, branchcode => undef, + has_priority => $has_priority, rules => { patron_maxissueqty => $patron_maxissueqty, patron_maxonsiteissueqty => $patron_maxonsiteissueqty, @@ -393,6 +396,7 @@ elsif ($op eq "cud-set-branch-defaults") { { itemtype => undef, branchcode => $branch, + has_priority => $has_priority, rules => { holdallowed => $holdallowed, hold_fulfillment_policy => $hold_fulfillment_policy, @@ -404,6 +408,7 @@ elsif ($op eq "cud-set-branch-defaults") { { categorycode => undef, branchcode => $branch, + has_priority => $has_priority, rules => { patron_maxissueqty => $patron_maxissueqty, patron_maxonsiteissueqty => $patron_maxonsiteissueqty, @@ -427,6 +432,7 @@ elsif ($op eq "cud-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 "*") { @@ -434,6 +440,7 @@ elsif ($op eq "cud-add-branch-cat") { { categorycode => undef, branchcode => undef, + has_priority => $has_priority, rules => { max_holds => $max_holds, patron_maxissueqty => $patron_maxissueqty, @@ -446,6 +453,7 @@ elsif ($op eq "cud-add-branch-cat") { { categorycode => $categorycode, branchcode => undef, + has_priority => $has_priority, rules => { max_holds => $max_holds, patron_maxissueqty => $patron_maxissueqty, @@ -459,6 +467,7 @@ elsif ($op eq "cud-add-branch-cat") { { categorycode => undef, branchcode => $branch, + has_priority => $has_priority, rules => { max_holds => $max_holds, patron_maxissueqty => $patron_maxissueqty, @@ -471,6 +480,7 @@ elsif ($op eq "cud-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 00000000000..05a6e898862 --- /dev/null +++ b/installer/data/mysql/atomicupdate/Bug_8137-add-column-has_priority.pl @@ -0,0 +1,13 @@ +use Modern::Perl; + +return { + bug_number => "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 26d44e54c61..8caf8698cfc 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 @@ -628,6 +628,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 } ) %] @@ -637,6 +638,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 ) %] @@ -790,6 +792,9 @@ + + + [% IF ( default_checkout_hold_and_return_policy ) %] @@ -820,6 +825,7 @@ Total current checkouts allowed Total current on-site checkouts allowed Total holds allowed + Has priority   [% FOREACH c IN categorycodes %] @@ -828,6 +834,9 @@ [% SET patron_maxissueqty = all_rules.$c.$i.patron_maxissueqty %] [% SET patron_maxonsiteissueqty = all_rules.$c.$i.patron_maxonsiteissueqty %] [% SET max_holds = all_rules.$c.$i.max_holds %] + [% SET nobranch_patron_maxissueqty = all_rules.$c.$i.max_holds.patron_maxissueqty %] + [% SET nobranch_patron_maxonsiteissueqty = all_rules.$c.$i.max_holds.patron_maxonsiteissueqty %] + [% 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 != '' ) %] @@ -859,7 +868,9 @@ Unlimited [% END %] - + + + Delete @@ -877,6 +888,9 @@ + + + @@ -1669,6 +1683,11 @@ return f.submit(); }); + if($("#branch").val() != '*') { + $(".hasPriority").each(function() { + $(this).remove(); + }); + } }); [% END %] -- 2.30.2