Bugzilla – Attachment 79643 Details for
Bug 18925
Move maxissueqty and maxonsiteissueqty to circulation_rules
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 18925: (follow-up) Change name of rule to fix ambiguity
Bug-18925-follow-up-Change-name-of-rule-to-fix-amb.patch (text/plain), 21.00 KB, created by
Jonathan Druart
on 2018-09-29 21:31:47 UTC
(
hide
)
Description:
Bug 18925: (follow-up) Change name of rule to fix ambiguity
Filename:
MIME Type:
Creator:
Jonathan Druart
Created:
2018-09-29 21:31:47 UTC
Size:
21.00 KB
patch
obsolete
>From 9fa9b8d4de4730bf0a16711ad5180f719fba25b8 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 e8f8d5b75a..9925672add 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 f363ab1cfe..308a898305 100755 >--- a/admin/smart-rules.pl >+++ b/admin/smart-rules.pl >@@ -96,8 +96,8 @@ elsif ($op eq 'delete-branch-cat') { > itemtype => undef, > rules => { > max_holds => undef, >- maxissueqty => undef, >- maxonsiteissueqty => undef, >+ patron_maxissueqty => undef, >+ patron_maxonsiteissueqty => undef, > } > } > ); >@@ -223,16 +223,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; >@@ -261,8 +261,8 @@ elsif ($op eq "set-branch-defaults") { > itemtype => undef, > branchcode => undef, > rules => { >- maxissueqty => $maxissueqty, >- maxonsiteissueqty => $maxonsiteissueqty, >+ patron_maxissueqty => $patron_maxissueqty, >+ patron_maxonsiteissueqty => $patron_maxonsiteissueqty, > } > } > ); >@@ -290,8 +290,8 @@ elsif ($op eq "set-branch-defaults") { > itemtype => undef, > branchcode => $branch, > rules => { >- maxissueqty => $maxissueqty, >- maxonsiteissueqty => $maxonsiteissueqty, >+ patron_maxissueqty => $patron_maxissueqty, >+ patron_maxonsiteissueqty => $patron_maxonsiteissueqty, > } > } > ); >@@ -308,13 +308,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+/; > >@@ -327,8 +327,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, > } > } > ); >@@ -340,8 +340,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, > } > } > ); >@@ -354,8 +354,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, > } > } > ); >@@ -367,8 +367,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 1512590e28..c0921c0abd 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 >@@ -391,12 +391,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' ) %] >@@ -514,11 +514,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 == '*'%] >@@ -528,15 +528,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 %] >@@ -564,8 +564,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 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
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 18925
:
65238
|
65239
|
65240
|
67024
|
67025
|
67026
|
67621
|
70992
|
70993
|
70994
|
70995
|
70996
|
71032
|
71033
|
71043
|
71044
|
71045
|
71046
|
71047
|
72225
|
72226
|
72227
|
72228
|
72229
|
77225
|
77226
|
77227
|
77228
|
77229
|
78881
|
78882
|
78883
|
78884
|
78885
|
78886
|
79642
|
79643
|
79644
|
79645
|
79646
|
81091
|
81092
|
81093
|
81094
|
81095
|
82927
|
82928
|
82929
|
82930
|
82931
|
85480
|
85481
|
85482
|
85483
|
85484
|
85485
|
85486
|
85939
|
85940
|
85941
|
85942
|
85943
|
85944
|
85945
|
85946
|
85947
|
85948
|
85949
|
85950
|
85951
|
85953
|
85954
|
85955
|
85956
|
85957
|
85958
|
85959
|
85960
|
85961
|
85962
|
85963
|
85964
|
85965
|
86068
|
86069
|
86070
|
86071
|
86072
|
86073
|
86074
|
86075
|
86076
|
86077
|
86078
|
86079
|
86080
|
86081
|
86121
|
86165
|
86206
|
86207