Bugzilla – Attachment 85481 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.06 KB, created by
Jonathan Druart
on 2019-02-21 20:05:50 UTC
(
hide
)
Description:
Bug 18925: (follow-up) Change name of rule to fix ambiguity
Filename:
MIME Type:
Creator:
Jonathan Druart
Created:
2019-02-21 20:05:50 UTC
Size:
21.06 KB
patch
obsolete
>From ba315d187a916c05650bb0fe4c66e39786579e9b 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 f13cba21c0..3775e73a4a 100644 >--- 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 }; > } > >@@ -1587,11 +1587,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. > >@@ -1604,8 +1604,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 >@@ -1642,12 +1642,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 98d857f693..e26dbcd4d3 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
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