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>&nbsp;</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