The searches in the module use $branchcode for all three parameters, looks like a copy/paste issue 29 $branchcode = undef if $branchcode eq q{} or $branchcode eq q{*}; 30 $categorycode = undef if $categorycode eq q{} or $branchcode eq q{*}; 31 $itemtype = undef if $itemtype eq q{} or $branchcode eq q{*}; The admin page uses 'Get' instead of 'Search' for maxisseqty/maxonsiteissueqty - this returns the 'effective' rule and so may return the wrong rule if things are not set 139 <td> 140 [% SET rule_value = CirculationRules.Get( rule.branchcode, rule.categorycode, rule.itemtype, 'maxissueqty' ) %] 141 [% IF rule_value || rule_value == "0" %] 142 [% rule_value | html %] 143 [% ELSE %] 144 <span>Unlimited</span> 145 [% END %] 146 </td> We discovered this from upgrade issues resulting from bug 23813
ARG! Impossible, I never copy paste, never!
Created attachment 95365 [details] [review] Bug 24026: (bug 22847 follow-up) Remove c/p issues
Created attachment 95366 [details] [review] Bug 24026: (bug 22847 follow-up) Display correctly max[onsite]issueqty (not the propagated ones) See bug 22847. We should never call ->Search in this template, we want to display the values in the DB, not the calculated ones
Upping severity.
Created attachment 95369 [details] [review] Bug 24026: (bug 22847 follow-up) Remove c/p issues Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Created attachment 95370 [details] [review] Bug 24026: (bug 22847 follow-up) Display correctly max[onsite]issueqty (not the propagated ones) See bug 22847. We should never call ->Search in this template, we want to display the values in the DB, not the calculated ones Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
(In reply to Jonathan Druart from comment #3) > See bug 22847. We should never call ->Search in this template, we want > to display the values in the DB, not the calculated ones We should never call 'Get', we should always use 'Search' - I think just the comment was wrong
To test: Add a standard rule for all libraries with maxisseqty and maxonsiteissueqty set to some value, say 5 Add several itemtype or categorycode specific rules, leaving max checkouts/onsite as blank to get unlimited Set an itemtype or cateogory specific rule for a branch as well. Remove the specific rules from the DB: delete from circulation_rules WHERE itemtype IS NOT NULL OR categorycode IS NOT NULL; Note that all rules for all libraries use the 'All/All' checkout limit Note that the specific library does as well Apply patch unset rules appear as unlimited now Note:thigns still won't work in this state, as we are introducing bad data, but it helps highlight the issue
Created attachment 95373 [details] [review] Bug 24026: (bug 22847 follow-up) Display correctly max[onsite]issueqty (not the propagated ones) See bug 22847. We should never call ->Get in this template, we want to display the values in the DB, not the calculated ones Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
(In reply to Nick Clemens from comment #7) > (In reply to Jonathan Druart from comment #3) > > See bug 22847. We should never call ->Search in this template, we want > > to display the values in the DB, not the calculated ones > > We should never call 'Get', we should always use 'Search' - I think just the > comment was wrong Move confusion, more fun!
Created attachment 95386 [details] [review] Bug 24026: (bug 22847 follow-up) Remove c/p issues Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 95387 [details] [review] Bug 24026: (bug 22847 follow-up) Display correctly max[onsite]issueqty (not the propagated ones) See bug 22847. We should never call ->Get in this template, we want to display the values in the DB, not the calculated ones Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Nice work! Pushed to master for 19.11.00
Pushed to 19.05.x for 19.05.05