From e853cc9ac2c1cf84082815b5ccfa58e3c12b826d 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 The aim here is to avoid the accumulation of checkouts possibilities through the rules of the various branches, in order to have a global quota. Test plan: 1) - Set for library "A" a max checkouts allowed to 3 for any categorycode on "Default checkout, hold and return policy" - Set for library "B" a max checkouts allowed to 2 for any categorycode on "Default checkout, hold and return policy" 2) Currently you can accumulate on each branch to get 5 checkouts in this test. 3) Apply this patch 4) Run updatedatabase script and rebuild schema, restart_all.. 5) Now you have a "Priority checkouts rule" checkboxes on the main circulation rules table, "Default checkouts[..]" table and the same by categorycode below (only on "Standard rules for all librairies" select option). 6) If checked on save, these rules will override global max checkouts allowed even if you try to check out from different librairy. 7) According to this test plan, if you set a "global" max checkouts to 2 you can only perform 2 checkouts and not more if you try on another lib. Try it on main circulation rules table and 2 defaults... Sponsored-by: BibLibre --- C4/Circulation.pm | 89 +++++++++++++++++-- Koha/CirculationRules.pm | 27 ++++-- admin/smart-rules.pl | 54 +++++++++-- .../Bug_8137-add-column-has_priority.pl | 13 +++ .../prog/en/modules/admin/smart-rules.tt | 65 ++++++++++---- 5 files changed, 215 insertions(+), 33 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 70ccd7fb54d..6c8318ca32f 100644 --- a/C4/Circulation.pm +++ b/C4/Circulation.pm @@ -461,6 +461,62 @@ sub TooMany { } ); + my $maxissueqty_priority_rule = Koha::CirculationRules->get_effective_rule( + { + categorycode => $cat_borrower, + itemtype => $type, + rule_name => 'maxissueqty', + has_priority => 1, + } + ); + + my $maxonsiteissueqty_priority_rule = Koha::CirculationRules->get_effective_rule( + { + categorycode => $cat_borrower, + itemtype => $type, + rule_name => 'maxonsiteissueqty', + has_priority => 1, + } + ); + + # Override checkouts quantity rules if priority rules exist + $maxissueqty_rule = $maxissueqty_priority_rule ? $maxissueqty_priority_rule : $maxissueqty_rule; + $maxonsiteissueqty_rule = $maxonsiteissueqty_priority_rule ? $maxonsiteissueqty_priority_rule : $maxonsiteissueqty_rule; + + 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 // $default_priority_no_branch_no_cat_patron_maxissueqty; + + $maxissueqty_rule = $default_priority_patron_maxissueqty ? $default_priority_patron_maxissueqty : $maxissueqty_rule; + + 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 // $default_priority_no_branch_no_cat_patron_maxonsiteissueqty; + # if a rule is found and has a loan limit set, count # how many loans the patron already has that meet that # rule @@ -534,15 +590,36 @@ sub TooMany { my $checkout_count = $sum_checkouts->{total} || 0; my $onsite_checkout_count = $sum_checkouts->{onsite_checkouts} || 0; + my $max_checkouts_allowed; + my $max_onsite_checkouts_allowed; + my $circulation_rule; + my $onsite_circulation_rule; + + if ( $default_priority_patron_maxissueqty && $default_priority_patron_maxissueqty->rule_value ne "" ) { + $max_checkouts_allowed = $default_priority_patron_maxissueqty->rule_value; + $circulation_rule = $default_priority_patron_maxissueqty; + } elsif ( $maxissueqty_rule && $maxissueqty_rule->rule_value ne "" ) { + $max_checkouts_allowed = $maxissueqty_rule->rule_value; + $circulation_rule = $maxissueqty_rule; + } + + if ( $default_priority_patron_maxonsiteissueqty && $default_priority_patron_maxonsiteissueqty->rule_value ne "" ) { + $max_onsite_checkouts_allowed = $default_priority_patron_maxonsiteissueqty->rule_value; + $onsite_circulation_rule = $default_priority_patron_maxonsiteissueqty; + } elsif ( $maxonsiteissueqty_rule && $maxonsiteissueqty_rule->rule_value ne "" ) { + $max_onsite_checkouts_allowed = $maxonsiteissueqty_rule->rule_value; + $onsite_circulation_rule = $maxonsiteissueqty_rule; + } + 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, + circulation_rule => $circulation_rule, + onsite_circulation_rule => $onsite_circulation_rule, }; # If parent rules exists if ( defined($parent_maxissueqty_rule) and defined($parent_maxissueqty_rule->rule_value) ){ @@ -581,8 +658,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->rule_value ? $default_priority_patron_maxissueqty->rule_value : $branch_borrower_circ_rule->{patron_maxissueqty}; + my $max_onsite_checkouts_allowed = $default_priority_patron_maxonsiteissueqty->rule_value ? $default_priority_patron_maxonsiteissueqty->rule_value : $branch_borrower_circ_rule->{patron_maxonsiteissueqty}; my $qty_over = _check_max_qty( { diff --git a/Koha/CirculationRules.pm b/Koha/CirculationRules.pm index 1d074768754..03df0a51e73 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 : 0; 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..c5b6234231c 100755 --- a/admin/smart-rules.pl +++ b/admin/smart-rules.pl @@ -130,12 +130,21 @@ elsif ($op eq 'cud-delete-branch-cat') { branchcode => undef, categorycode => undef, rules => { - max_holds => undef, + has_priority => undef, patron_maxissueqty => undef, patron_maxonsiteissueqty => undef, } } ); + Koha::CirculationRules->set_rules( + { + branchcode => undef, + categorycode => undef, + rules => { + max_holds => undef, + } + } + ); Koha::CirculationRules->set_rules( { branchcode => undef, @@ -153,12 +162,21 @@ elsif ($op eq 'cud-delete-branch-cat') { categorycode => $categorycode, branchcode => undef, rules => { - max_holds => undef, + has_priority => undef, patron_maxissueqty => undef, patron_maxonsiteissueqty => undef, } } ); + Koha::CirculationRules->set_rules( + { + categorycode => $categorycode, + branchcode => undef, + rules => { + max_holds => undef, + } + } + ); } } elsif ($categorycode eq "*") { Koha::CirculationRules->set_rules( @@ -302,10 +320,14 @@ elsif ( $op eq 'cud-add' ) { my $recall_overdue_fine = $input->param('recall_overdue_fine'); my $recall_shelf_time = $input->param('recall_shelf_time'); my $holds_pickup_period = strip_non_numeric( scalar $input->param('holds_pickup_period') ); + my $has_priority = $input->param('has_priority_rule') ? 1 : 0; - my $rules = { + my $checkout_rules = { maxissueqty => $maxissueqty, maxonsiteissueqty => $maxonsiteissueqty, + }; + + my $rules = { rentaldiscount => $rentaldiscount, fine => $fine, finedays => $finedays, @@ -355,6 +377,16 @@ elsif ( $op eq 'cud-add' ) { } ); + Koha::CirculationRules->set_rules( + { + categorycode => $bor eq '*' ? undef : $bor, + itemtype => $itemtype eq '*' ? undef : $itemtype, + branchcode => $br eq '*' ? undef : $br, + rules => $checkout_rules, + has_priority => $has_priority, + } + ); + } elsif ($op eq "cud-set-branch-defaults") { my $categorycode = $input->param('categorycode'); @@ -365,6 +397,7 @@ 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_defaults') ? 1 : 0; if ($branch eq "*") { Koha::CirculationRules->set_rules( @@ -382,6 +415,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, @@ -404,6 +438,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 +462,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_by_cat') ? 1 : 0; if ($branch eq "*") { if ($categorycode eq "*") { @@ -447,7 +483,16 @@ elsif ($op eq "cud-add-branch-cat") { categorycode => $categorycode, branchcode => undef, rules => { - max_holds => $max_holds, + max_holds => $max_holds, + } + } + ); + Koha::CirculationRules->set_rules( + { + categorycode => $categorycode, + branchcode => undef, + has_priority => $has_priority, + rules => { patron_maxissueqty => $patron_maxissueqty, patron_maxonsiteissueqty => $patron_maxonsiteissueqty, } @@ -758,7 +803,6 @@ while ( my $r = $all_rules->next ) { } $template->param(show_branch_cat_rule_form => 1); - $template->param( used_categorycodes => \@used_categorycodes, used_itemtypes => \@used_itemtypes, 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 c4943b34072..5ed68ccfb81 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 @@ -113,6 +113,7 @@ Note Current checkouts allowed Current on-site checkouts allowed + Priority checkouts rule Loan period Days mode Unit @@ -167,6 +168,8 @@ [% SET note = all_rules.$c.$i.note %] [% SET maxissueqty = all_rules.$c.$i.maxissueqty %] [% SET maxonsiteissueqty = all_rules.$c.$i.maxonsiteissueqty %] + [% SET maxissueqty = CirculationRules.Search( current_branch, c, i, 'maxissueqty', { want_rule => 1 } ) %] + [% SET maxonsiteissueqty = CirculationRules.Search( current_branch, c, i, 'maxonsiteissueqty', { want_rule => 1 } ) %] [% SET issuelength = all_rules.$c.$i.issuelength %] [% SET daysmode = all_rules.$c.$i.daysmode %] [% SET lengthunit = all_rules.$c.$i.lengthunit %] @@ -204,8 +207,9 @@ [% SET recall_overdue_fine = all_rules.$c.$i.recall_overdue_fine %] [% SET recall_shelf_time = all_rules.$c.$i.recall_shelf_time %] [% SET holds_pickup_period = all_rules.$c.$i.holds_pickup_period %] + [% SET has_priority_rule = maxissueqty.has_priority && maxonsiteissueqty.has_priority %] - [% SET show_rule = note || maxissueqty || maxonsiteissueqty || issuelength || daysmode || lengthunit || hardduedate || hardduedatecompare || fine || chargeperiod || chargeperiod_charge_at || firstremind || overduefinescap || cap_fine_to_replacement_price || finedays || maxsuspensiondays || suspension_chargeperiod || renewalsallowed || unseenrenewalsallowed || renewalperiod || norenewalbefore || noautorenewalbefore || auto_renew || no_auto_renewal_after || no_auto_renewal_after_hard_limit || reservesallowed || holds_per_day || holds_per_record || onshelfholds || opacitemholds || article_requests || rentaldiscount || decreaseloanholds || recalls_allowed || recalls_per_record || on_shelf_recalls || recall_due_date_interval || recall_overdue_fine || recall_shelf_time || holds_pickup_period %] + [% SET show_rule = note || maxissueqty.rule_value || maxonsiteissueqty.rule_value || issuelength || daysmode || lengthunit || hardduedate || hardduedatecompare || fine || chargeperiod || chargeperiod_charge_at || firstremind || overduefinescap || cap_fine_to_replacement_price || finedays || maxsuspensiondays || suspension_chargeperiod || renewalsallowed || unseenrenewalsallowed || renewalperiod || norenewalbefore || noautorenewalbefore || auto_renew || no_auto_renewal_after || no_auto_renewal_after_hard_limit || reservesallowed || holds_per_day || holds_per_record || onshelfholds || opacitemholds || article_requests || rentaldiscount || decreaseloanholds || recalls_allowed || recalls_per_record || on_shelf_recalls || recall_due_date_interval || recall_overdue_fine || recall_shelf_time || holds_pickup_period %] [% IF show_rule %] [% SET row_count = row_count + 1 %] @@ -243,19 +247,22 @@ [% ELSE %] [% END %] - [% IF maxissueqty.defined && maxissueqty != '' %] - [% maxissueqty | html %] + [% IF maxissueqty.rule_value && maxissueqty.rule_value != '' %] + [% maxissueqty.rule_value | html %] [% ELSE %] Unlimited [% END %] - [% IF maxonsiteissueqty.defined && maxonsiteissueqty != '' %] - [% maxonsiteissueqty | html %] + [% IF maxonsiteissueqty.rule_value && maxonsiteissueqty.rule_value != '' %] + [% maxonsiteissueqty.rule_value | html %] [% ELSE %] Unlimited [% END %] + + + [% issuelength | html %] [% SWITCH daysmode %] @@ -446,6 +453,9 @@ + + + + @@ -817,6 +833,7 @@ + @@ -825,11 +842,11 @@ [% FOREACH c IN categorycodes %] [% NEXT UNLESS c %] [% SET i = undef %] - [% 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 %] - - [% IF ( patron_maxissueqty.defined && patron_maxissueqty != '' ) || ( patron_maxonsiteissueqty.defined && patron_maxonsiteissueqty != '' ) || ( max_holds.defined && max_holds != '' ) %] + [% SET patron_maxissueqty = CirculationRules.Search( current_branch, c, undef, 'patron_maxissueqty', { want_rule => 1 } ) %] + [% SET patron_maxonsiteissueqty = CirculationRules.Search( current_branch, c, undef, 'patron_maxonsiteissueqty', { want_rule => 1 } ) %] + [% SET max_holds = CirculationRules.Search( current_branch, c, undef, 'max_holds', { want_rule => 1 } ) %] + [% SET has_priority_by_cat = patron_maxissueqty.has_priority && patron_maxonsiteissueqty.has_priority %] + [% IF ( patron_maxissueqty.rule_value && patron_maxissueqty.rule_value != '' ) || ( patron_maxonsiteissueqty.rule_value && patron_maxonsiteissueqty.rule_value != '' ) || ( max_holds.rule_value && max_holds.rule_value != '' ) %] + - @@ -874,6 +893,9 @@ [% END %] + @@ -1577,6 +1599,10 @@ $(current_column).find("input[type='text']").val(""); } } + if($(current_column).hasClass('hasPriority')) { + var has_priority_rule_checked = $(this).children("input[type='checkbox']").prop('checked'); + $(current_column).children("input[name='has_priority_rule']").prop('checked', has_priority_rule_checked); + } } }); $("#default-circulation-rules tr:last td:eq(0) select").prop('disabled', true); @@ -1678,6 +1704,11 @@ return f.submit(); }); + if($("#branch").val() != '*') { + $(".hasPriority").each(function() { + $(this).remove(); + }); + } }); [% END %] -- 2.30.2
Patron categoryPriority checkouts rule Total current checkouts allowed Total current on-site checkouts allowed Total holds allowed
[% IF c == undef %] @@ -838,28 +855,30 @@ [% Categories.GetName(c) | html %] [% END %] + + - [% IF patron_maxissueqty.defined && patron_maxissueqty != '' %] - [% patron_maxissueqty | html %] + [% IF patron_maxissueqty.rule_value && patron_maxissueqty.rule_value != '' %] + [% patron_maxissueqty.rule_value | html %] [% ELSE %] Unlimited [% END %] - [% IF patron_maxonsiteissueqty.defined && patron_maxonsiteissueqty != '' %] - [% patron_maxonsiteissueqty | html %] + [% IF patron_maxonsiteissueqty.rule_value && patron_maxonsiteissueqty.rule_value != '' %] + [% patron_maxonsiteissueqty.rule_value | html %] [% ELSE %] Unlimited [% END %] - [% IF max_holds.defined && max_holds != '' %] - [% max_holds | html %] + [% IF max_holds.rule_value && max_holds.rule_value != '' %] + [% max_holds.rule_value | html %] [% ELSE %] Unlimited [% END %] Delete + +