From 8af2402b1783bd531baf9fd372470ae723f97c20 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 | 52 +++++++++++----------- installer/data/mysql/atomicupdate/bug_18925.perl | 16 +++---- .../prog/en/modules/admin/smart-rules.tt | 26 +++++------ t/db_dependent/Circulation/Branch.t | 32 ++++++------- 5 files changed, 74 insertions(+), 74 deletions(-) diff --git a/C4/Circulation.pm b/C4/Circulation.pm index ea522a06d5..3e047c8e3f 100644 --- a/C4/Circulation.pm +++ b/C4/Circulation.pm @@ -503,7 +503,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 @@ -523,8 +523,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 ) { @@ -555,7 +555,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 }; } @@ -1575,11 +1575,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. @@ -1592,8 +1592,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 @@ -1630,12 +1630,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 8c2d2bd865..2aedb197c1 100755 --- a/admin/smart-rules.pl +++ b/admin/smart-rules.pl @@ -104,8 +104,8 @@ elsif ($op eq 'delete-branch-cat') { itemtype => undef, rules => { max_holds => undef, - maxissueqty => undef, - maxonsiteissueqty => undef, + patron_maxissueqty => undef, + patron_maxonsiteissueqty => undef, } } ); @@ -235,16 +235,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; @@ -273,8 +273,8 @@ elsif ($op eq "set-branch-defaults") { itemtype => undef, branchcode => undef, rules => { - maxissueqty => $maxissueqty, - maxonsiteissueqty => $maxonsiteissueqty, + patron_maxissueqty => $patron_maxissueqty, + patron_maxonsiteissueqty => $patron_maxonsiteissueqty, } } ); @@ -302,8 +302,8 @@ elsif ($op eq "set-branch-defaults") { itemtype => undef, branchcode => $branch, rules => { - maxissueqty => $maxissueqty, - maxonsiteissueqty => $maxonsiteissueqty, + patron_maxissueqty => $patron_maxissueqty, + patron_maxonsiteissueqty => $patron_maxonsiteissueqty, } } ); @@ -320,13 +320,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+/; @@ -339,8 +339,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, } } ); @@ -352,8 +352,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, } } ); @@ -366,8 +366,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, } } ); @@ -379,8 +379,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 a438aa0036..2dbd4cdd56 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 @@ -402,12 +402,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' ) %] @@ -525,11 +525,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 == '*'%] @@ -539,15 +539,15 @@ [% END %] - [% IF maxissueqty %] - [% maxissueqty | html %] + [% IF patron_maxissueqty %] + [% patron_maxissueqty | html %] [% ELSE %] Unlimited [% END %] - [% IF maxonsiteissueqty %] - [% maxonsiteissueqty | html %] + [% IF patron_maxonsiteissueqty %] + [% patron_maxonsiteissueqty | html %] [% ELSE %] Unlimited [% END %] @@ -575,8 +575,8 @@ [% END %] - - + +