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.

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

diff --git a/C4/Circulation.pm b/C4/Circulation.pm
index 66c208a..658286e 100644 (file)
--- a/C4/Circulation.pm
+++ b/C4/Circulation.pm
@@ -505,7 +505,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
@@ -525,8 +525,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 )  {
@@ -557,7 +557,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 };
     }
 
@@ -1588,11 +1588,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.
 
@@ -1605,8 +1605,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
@@ -1643,12 +1643,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 76fed0a..fbdbd86 100755 (executable)
--- 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,
             }
         }
     );
@@ -237,16 +237,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;
@@ -275,8 +275,8 @@ elsif ($op eq "set-branch-defaults") {
                 itemtype     => undef,
                 branchcode   => undef,
                 rules        => {
-                    maxissueqty       => $maxissueqty,
-                    maxonsiteissueqty => $maxonsiteissueqty,
+                    patron_maxissueqty             => $patron_maxissueqty,
+                    patron_maxonsiteissueqty       => $patron_maxonsiteissueqty,
                 }
             }
         );
@@ -304,8 +304,8 @@ elsif ($op eq "set-branch-defaults") {
                 itemtype     => undef,
                 branchcode   => $branch,
                 rules        => {
-                    maxissueqty       => $maxissueqty,
-                    maxonsiteissueqty => $maxonsiteissueqty,
+                    patron_maxissueqty             => $patron_maxissueqty,
+                    patron_maxonsiteissueqty       => $patron_maxonsiteissueqty,
                 }
             }
         );
@@ -322,13 +322,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+/;
 
@@ -341,8 +341,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,
                     }
                 }
             );
@@ -354,8 +354,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,
                     }
                 }
             );
@@ -368,8 +368,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,
                 }
             }
         );
@@ -381,8 +381,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 b80dd29..cafd0af 100644 (file)
--- 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 98d857f..e26dbcd 100644 (file)
--- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/smart-rules.tt
+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/smart-rules.tt
@@ -410,12 +410,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 | html %]"/>
+                        [% SET patron_maxissueqty = CirculationRules.Get( branchcode, undef, undef, 'patron_maxissueqty' ) %]
+                        <input type="text" name="patron_maxissueqty" size="3" value="[% patron_maxissueqty | html %]"/>
                     </td>
                     <td>
-                        [% SET maxonsiteissueqty  = CirculationRules.Get( branchcode, undef, undef, 'maxonsiteissueqty' ) %]
-                        <input type="text" name="maxonsiteissueqty" size="3" value="[% maxonsiteissueqty | html %]"/>
+                        [% SET patron_maxonsiteissueqty = CirculationRules.Get( branchcode, undef, undef, 'patron_maxonsiteissueqty' ) %]
+                        <input type="text" name="patron_maxonsiteissueqty" size="3" value="[% patron_maxonsiteissueqty | html %]"/>
                     </td>
                     <td>
                         [% SET rule_value = CirculationRules.Get( current_branch, '*', undef, 'max_holds' ) %]
@@ -533,11 +533,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 == '*'%]
@@ -547,15 +547,15 @@
                             [% END %]
                         </td>
                         <td>
-                            [% IF maxissueqty  %]
-                                [% maxissueqty | html %]
+                            [% IF patron_maxissueqty  %]
+                                [% patron_maxissueqty | html %]
                             [% ELSE %]
                                 <span>Unlimited</span>
                             [% END %]
                         </td>
                         <td>
-                            [% IF maxonsiteissueqty  %]
-                                [% maxonsiteissueqty | html %]
+                            [% IF patron_maxonsiteissueqty  %]
+                                [% patron_maxonsiteissueqty | html %]
                             [% ELSE %]
                                 <span>Unlimited</span>
                             [% END %]
@@ -583,8 +583,8 @@
                         [% END %]
                         </select>
                     </td>
-                    <td><input name="maxissueqty" size="3" type="text" /></td>
-                    <td><input name="maxonsiteissueqty" size="3" type="text" /></td>
+                    <td><input name="patron_maxissueqty" size="3" type="text" /></td>
+                    <td><input name="patron_maxonsiteissueqty" size="3" type="text" /></td>
                     <td><input name="max_holds" size="3" type="text" /></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 22e491d..db2c169 100644 (file)
--- 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