From d5d7a1f5e0fa50bdbea02316be87e418af06f883 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 0d9cc0c72b..4e98ffcaf4 100644
--- a/C4/Circulation.pm
+++ b/C4/Circulation.pm
@@ -504,7 +504,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
@@ -524,8 +524,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 ) {
@@ -556,7 +556,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 };
}
@@ -1576,11 +1576,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.
@@ -1593,8 +1593,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
@@ -1631,12 +1631,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 76fed0a5a0..fbdbd8617d 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,
}
}
);
@@ -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 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 825f6eb3b3..bac378953d 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
@@ -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> </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 22e491dda2..db2c169715 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