From 4feec56d19f2d3c55a2ed39dcdd8b3d6db173610 Mon Sep 17 00:00:00 2001 From: Jesse Weaver <pianohacker@gmail.com> 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 33b123c579..e401f67082 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 8caec4c4d1..327a567ed9 100755 --- a/admin/smart-rules.pl +++ b/admin/smart-rules.pl @@ -92,8 +92,8 @@ elsif ($op eq 'delete-branch-cat') { itemtype => undef, rules => { max_holds => undef, - maxissueqty => undef, - maxonsiteissueqty => undef, + patron_maxissueqty => undef, + patron_maxonsiteissueqty => undef, } } ); @@ -219,16 +219,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; @@ -257,8 +257,8 @@ elsif ($op eq "set-branch-defaults") { itemtype => undef, branchcode => undef, rules => { - maxissueqty => $maxissueqty, - maxonsiteissueqty => $maxonsiteissueqty, + patron_maxissueqty => $patron_maxissueqty, + patron_maxonsiteissueqty => $patron_maxonsiteissueqty, } } ); @@ -286,8 +286,8 @@ elsif ($op eq "set-branch-defaults") { itemtype => undef, branchcode => $branch, rules => { - maxissueqty => $maxissueqty, - maxonsiteissueqty => $maxonsiteissueqty, + patron_maxissueqty => $patron_maxissueqty, + patron_maxonsiteissueqty => $patron_maxonsiteissueqty, } } ); @@ -304,13 +304,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+/; @@ -323,8 +323,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, } } ); @@ -336,8 +336,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, } } ); @@ -350,8 +350,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, } } ); @@ -363,8 +363,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 b8fb043106..748edf0dcc 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 @@ -390,12 +390,12 @@ <tr> <td><em>Defaults[% UNLESS ( default_rules ) %] (not set)[% END %]</em></td> <td> - [% SET maxissueqty = CirculationRules.Get( branchcode, undef, undef, 'maxissueqty' ) %] - <input type="text" name="maxissueqty" size="3" value="[% maxissueqty %]"/> + [% SET patron_maxissueqty = CirculationRules.Get( branchcode, undef, undef, 'patron_maxissueqty' ) %] + <input type="text" name="patron_maxissueqty" size="3" value="[% patron_maxissueqty %]"/> </td> <td> - [% SET maxonsiteissueqty = CirculationRules.Get( branchcode, undef, undef, 'maxonsiteissueqty' ) %] - <input type="text" name="maxonsiteissueqty" size="3" value="[% maxonsiteissueqty %]"/> + [% SET patron_maxonsiteissueqty = CirculationRules.Get( branchcode, undef, undef, 'patron_maxonsiteissueqty' ) %] + <input type="text" name="patron_maxonsiteissueqty" size="3" value="[% patron_maxonsiteissueqty %]"/> </td> <td> [% SET rule_value = CirculationRules.Get( current_branch, '*', undef, 'max_holds' ) %] @@ -513,11 +513,11 @@ <th> </th> </tr> [% 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 %] <tr> <td> [% IF c == '*'%] @@ -527,15 +527,15 @@ [% END %] </td> <td> - [% IF maxissueqty %] - [% maxissueqty %] + [% IF patron_maxissueqty %] + [% patron_maxissueqty %] [% ELSE %] <span>Unlimited</span> [% END %] </td> <td> - [% IF maxonsiteissueqty %] - [% maxonsiteissueqty %] + [% IF patron_maxonsiteissueqty %] + [% patron_maxonsiteissueqty %] [% ELSE %] <span>Unlimited</span> [% END %] @@ -563,8 +563,8 @@ [% END %] </select> </td> - <td><input name="maxissueqty" size="3" /></td> - <td><input name="maxonsiteissueqty" size="3" /></td> + <td><input name="patron_maxissueqty" size="3" /></td> + <td><input name="patron_maxonsiteissueqty" size="3" /></td> <td><input name="max_holds" size="3" /></td> <td class="actions"><button type="submit" class="btn btn-default btn-xs"><i class="fa fa-plus"></i> Add</td> </tr> diff --git a/t/db_dependent/Circulation/Branch.t b/t/db_dependent/Circulation/Branch.t index 1f8c0e577f..d7473bc826 100644 --- a/t/db_dependent/Circulation/Branch.t +++ b/t/db_dependent/Circulation/Branch.t @@ -148,8 +148,8 @@ my $borrower_id1 = Koha::Patron->new({ is_deeply( GetBranchBorrowerCircRule(), - { maxissueqty => undef, maxonsiteissueqty => undef }, -"Without parameter, GetBranchBorrower returns undef (unilimited) for maxissueqty and maxonsiteissueqty if no rules defined" + { patron_maxissueqty => undef, patron_maxonsiteissueqty => undef }, +"Without parameter, GetBranchBorrower returns undef (unilimited) for patron_maxissueqty and patron_maxonsiteissueqty if no rules defined" ); Koha::CirculationRules->set_rules( @@ -158,8 +158,8 @@ Koha::CirculationRules->set_rules( categorycode => $samplecat->{categorycode}, itemtype => undef, rules => { - maxissueqty => 5, - maxonsiteissueqty => 6, + patron_maxissueqty => 5, + patron_maxonsiteissueqty => 6, } } ); @@ -176,8 +176,8 @@ Koha::CirculationRules->set_rules( categorycode => undef, itemtype => undef, rules => { - maxissueqty => 3, - maxonsiteissueqty => 2, + patron_maxissueqty => 3, + patron_maxonsiteissueqty => 2, } } ); @@ -194,8 +194,8 @@ Koha::CirculationRules->set_rules( categorycode => undef, itemtype => undef, rules => { - maxissueqty => 4, - maxonsiteissueqty => 5, + patron_maxissueqty => 4, + patron_maxonsiteissueqty => 5, } } ); @@ -222,26 +222,26 @@ $sth->execute( #Test GetBranchBorrowerCircRule is_deeply( GetBranchBorrowerCircRule(), - { maxissueqty => 4, maxonsiteissueqty => 5 }, -"Without parameter, GetBranchBorrower returns the maxissueqty and maxonsiteissueqty of default_circ_rules" + { patron_maxissueqty => 4, patron_maxonsiteissueqty => 5 }, +"Without parameter, GetBranchBorrower returns the patron_maxissueqty and patron_maxonsiteissueqty of default_circ_rules" ); is_deeply( GetBranchBorrowerCircRule( $samplebranch2->{branchcode} ), - { maxissueqty => 3, maxonsiteissueqty => 2 }, -"Without only the branchcode specified, GetBranchBorrower returns the maxissueqty and maxonsiteissueqty corresponding" + { patron_maxissueqty => 3, patron_maxonsiteissueqty => 2 }, +"Without only the branchcode specified, GetBranchBorrower returns the patron_maxissueqty and patron_maxonsiteissueqty corresponding" ); is_deeply( GetBranchBorrowerCircRule( $samplebranch1->{branchcode}, $samplecat->{categorycode} ), - { maxissueqty => 5, maxonsiteissueqty => 6 }, - "GetBranchBorrower returns the maxissueqty and maxonsiteissueqty of the branch1 and the category1" + { patron_maxissueqty => 5, patron_maxonsiteissueqty => 6 }, + "GetBranchBorrower returns the patron_maxissueqty and patron_maxonsiteissueqty of the branch1 and the category1" ); is_deeply( GetBranchBorrowerCircRule( -1, -1 ), - { maxissueqty => 4, maxonsiteissueqty => 5 }, -"GetBranchBorrower with wrong parameters returns the maxissueqty and maxonsiteissueqty of default_circ_rules" + { patron_maxissueqty => 4, patron_maxonsiteissueqty => 5 }, +"GetBranchBorrower with wrong parameters returns the patron_maxissueqty and patron_maxonsiteissueqty of default_circ_rules" ); #Test GetBranchItemRule -- 2.11.0