From 9fa44e5f212dd21088fe9e9b5ce86f3888d38883 Mon Sep 17 00:00:00 2001 From: Jesse Weaver Date: Sat, 16 Sep 2017 17:23:01 -0600 Subject: [PATCH] Bug 18925: (follow-up) Change name of rule to fix ambiguity There was previously an ambiguity between the branch/category/itemtype specific max{,onsite}issueqty and the total-per-patron max{,onsite}issueqty. The latter has been renamed to patron_max{,onsite}issueqty. --- C4/Circulation.pm | 22 +-- admin/smart-rules.pl | 158 +++++++++++++++------ installer/data/mysql/atomicupdate/bug_18925.perl | 16 +-- .../prog/en/modules/admin/smart-rules.tt | 26 ++-- t/db_dependent/Circulation/Branch.t | 36 ++--- 5 files changed, 164 insertions(+), 94 deletions(-) diff --git a/C4/Circulation.pm b/C4/Circulation.pm index 5ae96f71e8..7b97ebc80d 100644 --- a/C4/Circulation.pm +++ b/C4/Circulation.pm @@ -515,7 +515,7 @@ sub TooMany { # Now count total loans against the limit for the branch my $branch_borrower_circ_rule = GetBranchBorrowerCircRule($branch, $cat_borrower); - if (defined($branch_borrower_circ_rule->{maxissueqty})) { + if (defined($branch_borrower_circ_rule->{patron_maxissueqty})) { my @bind_params = (); my $branch_count_query = q| SELECT COUNT(*) AS total, COALESCE(SUM(onsite_checkout), 0) AS onsite_checkouts @@ -535,8 +535,8 @@ sub TooMany { push @bind_params, $branch; } my ( $checkout_count, $onsite_checkout_count ) = $dbh->selectrow_array( $branch_count_query, {}, @bind_params ); - my $max_checkouts_allowed = $branch_borrower_circ_rule->{maxissueqty}; - my $max_onsite_checkouts_allowed = $branch_borrower_circ_rule->{maxonsiteissueqty}; + my $max_checkouts_allowed = $branch_borrower_circ_rule->{patron_maxissueqty}; + my $max_onsite_checkouts_allowed = $branch_borrower_circ_rule->{patron_maxonsiteissueqty}; if ( $onsite_checkout and defined $max_onsite_checkouts_allowed ) { if ( $onsite_checkout_count >= $max_onsite_checkouts_allowed ) { @@ -567,7 +567,7 @@ sub TooMany { } } - if ( not defined( $maxissueqty_rule ) and not defined($branch_borrower_circ_rule->{maxissueqty}) ) { + if ( not defined( $maxissueqty_rule ) and not defined($branch_borrower_circ_rule->{patron_maxissueqty}) ) { return { reason => 'NO_RULE_DEFINED', max_allowed => 0 }; } @@ -1591,11 +1591,11 @@ Retrieves circulation rule attributes that apply to the given branch and patron category, regardless of item type. The return value is a hashref containing the following key: -maxissueqty - maximum number of loans that a +patron_maxissueqty - maximum number of loans that a patron of the given category can have at the given branch. If the value is undef, no limit. -maxonsiteissueqty - maximum of on-site checkouts that a +patron_maxonsiteissueqty - maximum of on-site checkouts that a patron of the given category can have at the given branch. If the value is undef, no limit. @@ -1608,8 +1608,8 @@ default branch and category If no rule has been found in the database, it will default to the buillt in rule: -maxissueqty - undef -maxonsiteissueqty - undef +patron_maxissueqty - undef +patron_maxonsiteissueqty - undef C<$branchcode> and C<$categorycode> should contain the literal branch code and patron category code, respectively - no @@ -1646,12 +1646,12 @@ sub GetBranchBorrowerCircRule { # Initialize default values my $rules = { - maxissueqty => undef, - maxonsiteissueqty => undef, + patron_maxissueqty => undef, + patron_maxonsiteissueqty => undef, }; # Search for rules! - foreach my $rule_name (qw( maxissueqty maxonsiteissueqty )) { + foreach my $rule_name (qw( patron_maxissueqty patron_maxonsiteissueqty )) { foreach my $params (@params) { my $rule = Koha::CirculationRules->search( { diff --git a/admin/smart-rules.pl b/admin/smart-rules.pl index 30be757801..7aa4eb624e 100755 --- a/admin/smart-rules.pl +++ b/admin/smart-rules.pl @@ -85,42 +85,102 @@ elsif ($op eq 'delete-branch-cat') { my $categorycode = $input->param('categorycode'); if ($branch eq "*") { if ($categorycode eq "*") { - my $sth_delete = $dbh->prepare("DELETE FROM default_circ_rules"); - $sth_delete->execute(); + Koha::CirculationRules->set_rules( + { + categorycode => undef, + branchcode => undef, + itemtype => undef, + rules => { + patron_maxissueqty => undef, + patron_maxonsiteissueqty => undef, + holdallowed => undef, + hold_fulfillment_policy => undef, + returnbranch => undef, + } + } + ); + } else { + Koha::CirculationRules->set_rules( + { + categorycode => $categorycode, + branchcode => undef, + itemtype => undef, + rules => { + max_holds => undef, + patron_maxissueqty => undef, + patron_maxonsiteissueqty => undef, + } + } + ); } } elsif ($categorycode eq "*") { - my $sth_delete = $dbh->prepare("DELETE FROM default_branch_circ_rules - WHERE branchcode = ?"); - $sth_delete->execute($branch); - } - Koha::CirculationRules->set_rules( - { - categorycode => $categorycode eq '*' ? undef : $categorycode, - branchcode => $branch eq '*' ? undef : $branch, - itemtype => undef, - rules => { - max_holds => undef, - maxissueqty => undef, - maxonsiteissueqty => undef, + Koha::CirculationRules->set_rules( + { + categorycode => undef, + branchcode => $branch, + itemtype => undef, + rules => { + patron_maxissueqty => undef, + patron_maxonsiteissueqty => undef, + holdallowed => undef, + hold_fulfillment_policy => undef, + returnbranch => undef, + } } - } - ); + ); + } else { + Koha::CirculationRules->set_rules( + { + categorycode => $categorycode, + branchcode => $branch, + itemtype => undef, + rules => { + max_holds => undef, + patron_maxissueqty => undef, + patron_maxonsiteissueqty => undef, + } + } + ); + } } elsif ($op eq 'delete-branch-item') { my $itemtype = $input->param('itemtype'); if ($branch eq "*") { if ($itemtype eq "*") { - my $sth_delete = $dbh->prepare("DELETE FROM default_circ_rules"); - $sth_delete->execute(); + Koha::CirculationRules->set_rules( + { + categorycode => undef, + branchcode => undef, + itemtype => undef, + rules => { + patron_maxissueqty => undef, + patron_maxonsiteissueqty => undef, + holdallowed => undef, + hold_fulfillment_policy => undef, + returnbranch => undef, + } + } + ); } else { my $sth_delete = $dbh->prepare("DELETE FROM default_branch_item_rules WHERE itemtype = ?"); $sth_delete->execute($itemtype); } } elsif ($itemtype eq "*") { - my $sth_delete = $dbh->prepare("DELETE FROM default_branch_circ_rules - WHERE branchcode = ?"); - $sth_delete->execute($branch); + Koha::CirculationRules->set_rules( + { + categorycode => undef, + branchcode => $branch, + itemtype => undef, + rules => { + patron_maxissueqty => undef, + patron_maxonsiteissueqty => undef, + holdallowed => undef, + hold_fulfillment_policy => undef, + returnbranch => undef, + } + } + ); } else { my $sth_delete = $dbh->prepare("DELETE FROM branch_item_rules WHERE branchcode = ? @@ -225,16 +285,16 @@ elsif ($op eq 'add') { } elsif ($op eq "set-branch-defaults") { my $categorycode = $input->param('categorycode'); - my $maxissueqty = $input->param('maxissueqty'); - my $maxonsiteissueqty = $input->param('maxonsiteissueqty'); + my $patron_maxissueqty = $input->param('patron_maxissueqty'); + my $patron_maxonsiteissueqty = $input->param('patron_maxonsiteissueqty'); my $holdallowed = $input->param('holdallowed'); my $hold_fulfillment_policy = $input->param('hold_fulfillment_policy'); my $returnbranch = $input->param('returnbranch'); my $max_holds = $input->param('max_holds'); - $maxissueqty =~ s/\s//g; - $maxissueqty = undef if $maxissueqty !~ /^\d+/; - $maxonsiteissueqty =~ s/\s//g; - $maxonsiteissueqty = undef if $maxonsiteissueqty !~ /^\d+/; + $patron_maxissueqty =~ s/\s//g; + $patron_maxissueqty = undef if $patron_maxissueqty !~ /^\d+/; + $patron_maxonsiteissueqty =~ s/\s//g; + $patron_maxonsiteissueqty = undef if $patron_maxonsiteissueqty !~ /^\d+/; $holdallowed =~ s/\s//g; $holdallowed = undef if $holdallowed !~ /^\d+/; $max_holds =~ s/\s//g; @@ -263,8 +323,11 @@ elsif ($op eq "set-branch-defaults") { itemtype => undef, branchcode => undef, rules => { - maxissueqty => $maxissueqty, - maxonsiteissueqty => $maxonsiteissueqty, + patron_maxissueqty => $patron_maxissueqty, + patron_maxonsiteissueqty => $patron_maxonsiteissueqty, + holdallowed => $holdallowed, + hold_fulfillment_policy => $hold_fulfillment_policy, + returnbranch => $returnbranch, } } ); @@ -292,8 +355,11 @@ elsif ($op eq "set-branch-defaults") { itemtype => undef, branchcode => $branch, rules => { - maxissueqty => $maxissueqty, - maxonsiteissueqty => $maxonsiteissueqty, + patron_maxissueqty => $patron_maxissueqty, + patron_maxonsiteissueqty => $patron_maxonsiteissueqty, + holdallowed => $holdallowed, + hold_fulfillment_policy => $hold_fulfillment_policy, + returnbranch => $returnbranch, } } ); @@ -310,13 +376,13 @@ elsif ($op eq "set-branch-defaults") { } elsif ($op eq "add-branch-cat") { my $categorycode = $input->param('categorycode'); - my $maxissueqty = $input->param('maxissueqty'); - my $maxonsiteissueqty = $input->param('maxonsiteissueqty'); + my $patron_maxissueqty = $input->param('patron_maxissueqty'); + my $patron_maxonsiteissueqty = $input->param('patron_maxonsiteissueqty'); my $max_holds = $input->param('max_holds'); - $maxissueqty =~ s/\s//g; - $maxissueqty = undef if $maxissueqty !~ /^\d+/; - $maxonsiteissueqty =~ s/\s//g; - $maxonsiteissueqty = undef if $maxonsiteissueqty !~ /^\d+/; + $patron_maxissueqty =~ s/\s//g; + $patron_maxissueqty = undef if $patron_maxissueqty !~ /^\d+/; + $patron_maxonsiteissueqty =~ s/\s//g; + $patron_maxonsiteissueqty = undef if $patron_maxonsiteissueqty !~ /^\d+/; $max_holds =~ s/\s//g; $max_holds = undef if $max_holds !~ /^\d+/; @@ -329,8 +395,8 @@ elsif ($op eq "add-branch-cat") { branchcode => undef, rules => { max_holds => $max_holds, - maxissueqty => $maxissueqty, - maxonsiteissueqty => $maxonsiteissueqty, + patron_maxissueqty => $patron_maxissueqty, + patron_maxonsiteissueqty => $patron_maxonsiteissueqty, } } ); @@ -342,8 +408,8 @@ elsif ($op eq "add-branch-cat") { itemtype => undef, rules => { max_holds => $max_holds, - maxissueqty => $maxissueqty, - maxonsiteissueqty => $maxonsiteissueqty, + patron_maxissueqty => $patron_maxissueqty, + patron_maxonsiteissueqty => $patron_maxonsiteissueqty, } } ); @@ -356,8 +422,8 @@ elsif ($op eq "add-branch-cat") { branchcode => $branch, rules => { max_holds => $max_holds, - maxissueqty => $maxissueqty, - maxonsiteissueqty => $maxonsiteissueqty, + patron_maxissueqty => $patron_maxissueqty, + patron_maxonsiteissueqty => $patron_maxonsiteissueqty, } } ); @@ -369,8 +435,8 @@ elsif ($op eq "add-branch-cat") { branchcode => $branch, rules => { max_holds => $max_holds, - maxissueqty => $maxissueqty, - maxonsiteissueqty => $maxonsiteissueqty, + patron_maxissueqty => $patron_maxissueqty, + patron_maxonsiteissueqty => $patron_maxonsiteissueqty, } } ); diff --git a/installer/data/mysql/atomicupdate/bug_18925.perl b/installer/data/mysql/atomicupdate/bug_18925.perl index b80dd29d65..cafd0af504 100644 --- a/installer/data/mysql/atomicupdate/bug_18925.perl +++ b/installer/data/mysql/atomicupdate/bug_18925.perl @@ -3,12 +3,12 @@ if( CheckVersion( $DBversion ) ) { if ( column_exists( 'branch_borrower_circ_rules', 'maxissueqty' ) ) { $dbh->do(" INSERT INTO circulation_rules ( categorycode, branchcode, itemtype, rule_name, rule_value ) - SELECT categorycode, branchcode, NULL, 'maxissueqty', maxissueqty + SELECT categorycode, branchcode, NULL, 'patron_maxissueqty', maxissueqty FROM branch_borrower_circ_rules "); $dbh->do(" INSERT INTO circulation_rules ( categorycode, branchcode, itemtype, rule_name, rule_value ) - SELECT categorycode, branchcode, NULL, 'maxonsiteissueqty', maxonsiteissueqty + SELECT categorycode, branchcode, NULL, 'patron_maxonsiteissueqty', maxonsiteissueqty FROM branch_borrower_circ_rules "); $dbh->do("DROP TABLE branch_borrower_circ_rules"); @@ -17,12 +17,12 @@ if( CheckVersion( $DBversion ) ) { if ( column_exists( 'default_borrower_circ_rules', 'maxissueqty' ) ) { $dbh->do(" INSERT INTO circulation_rules ( categorycode, branchcode, itemtype, rule_name, rule_value ) - SELECT categorycode, NULL, NULL, 'maxissueqty', maxissueqty + SELECT categorycode, NULL, NULL, 'patron_maxissueqty', maxissueqty FROM default_borrower_circ_rules "); $dbh->do(" INSERT INTO circulation_rules ( categorycode, branchcode, itemtype, rule_name, rule_value ) - SELECT categorycode, NULL, NULL, 'maxonsiteissueqty', maxonsiteissueqty + SELECT categorycode, NULL, NULL, 'patron_maxonsiteissueqty', maxonsiteissueqty FROM default_borrower_circ_rules "); $dbh->do("DROP TABLE default_borrower_circ_rules"); @@ -31,12 +31,12 @@ if( CheckVersion( $DBversion ) ) { if ( column_exists( 'default_circ_rules', 'maxissueqty' ) ) { $dbh->do(" INSERT INTO circulation_rules ( categorycode, branchcode, itemtype, rule_name, rule_value ) - SELECT NULL, NULL, NULL, 'maxissueqty', maxissueqty + SELECT NULL, NULL, NULL, 'patron_maxissueqty', maxissueqty FROM default_circ_rules "); $dbh->do(" INSERT INTO circulation_rules ( categorycode, branchcode, itemtype, rule_name, rule_value ) - SELECT NULL, NULL, NULL, 'maxonsiteissueqty', maxonsiteissueqty + SELECT NULL, NULL, NULL, 'patron_maxonsiteissueqty', maxonsiteissueqty FROM default_circ_rules "); $dbh->do("ALTER TABLE default_circ_rules DROP COLUMN maxissueqty, DROP COLUMN maxonsiteissueqty"); @@ -45,12 +45,12 @@ if( CheckVersion( $DBversion ) ) { if ( column_exists( 'default_branch_circ_rules', 'maxissueqty' ) ) { $dbh->do(" INSERT INTO circulation_rules ( categorycode, branchcode, itemtype, rule_name, rule_value ) - SELECT NULL, branchcode, NULL, 'maxissueqty', maxissueqty + SELECT NULL, branchcode, NULL, 'patron_maxissueqty', maxissueqty FROM default_branch_circ_rules "); $dbh->do(" INSERT INTO circulation_rules ( categorycode, branchcode, itemtype, rule_name, rule_value ) - SELECT NULL, NULL, NULL, 'maxonsiteissueqty', maxonsiteissueqty + SELECT NULL, NULL, NULL, 'patron_maxonsiteissueqty', maxonsiteissueqty FROM default_branch_circ_rules "); $dbh->do("ALTER TABLE default_branch_circ_rules DROP COLUMN maxissueqty, DROP COLUMN maxonsiteissueqty"); 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 ad19f962cf..57dd516950 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 @@ -372,12 +372,12 @@ Defaults[% UNLESS ( default_rules ) %] (not set)[% END %] - [% SET maxissueqty = CirculationRules.Get( branchcode, undef, undef, 'maxissueqty' ) %] - + [% SET patron_maxissueqty = CirculationRules.Get( branchcode, undef, undef, 'patron_maxissueqty' ) %] + - [% SET maxonsiteissueqty = CirculationRules.Get( branchcode, undef, undef, 'maxonsiteissueqty' ) %] - + [% SET patron_maxonsiteissueqty = CirculationRules.Get( branchcode, undef, undef, 'patron_maxonsiteissueqty' ) %] + [% SET rule_value = CirculationRules.Get( current_branch, '*', undef, 'max_holds' ) %] @@ -495,11 +495,11 @@   [% FOREACH c IN categorycodes %] - [% SET maxissueqty = CirculationRules.Get( branchcode, c, undef, 'maxissueqty' ) %] - [% SET maxonsiteissueqty = CirculationRules.Get( branchcode, c, undef, 'maxonsiteissueqty' ) %] + [% SET patron_maxissueqty = CirculationRules.Get( branchcode, c, undef, 'patron_maxissueqty' ) %] + [% SET patron_maxonsiteissueqty = CirculationRules.Get( branchcode, c, undef, 'patron_maxonsiteissueqty' ) %] [% SET max_holds = CirculationRules.Get( branchcode, c, undef, 'max_holds' ) %] - [% IF maxissueqty || maxissueqty || max_holds %] + [% IF patron_maxissueqty || patron_maxonsiteissueqty || max_holds %] [% IF c == '*'%] @@ -509,15 +509,15 @@ [% END %] - [% IF maxissueqty %] - [% maxissueqty %] + [% IF patron_maxissueqty %] + [% patron_maxissueqty %] [% ELSE %] Unlimited [% END %] - [% IF maxonsiteissueqty %] - [% maxonsiteissueqty %] + [% IF patron_maxonsiteissueqty %] + [% patron_maxonsiteissueqty %] [% ELSE %] Unlimited [% END %] @@ -544,8 +544,8 @@ [% END %] - - + +