Bug 25601 - Error when unsetting default checkout, hold and return policy for a specific library
Summary: Error when unsetting default checkout, hold and return policy for a specific ...
Status: Pushed to oldstable
Alias: None
Product: Koha
Classification: Unclassified
Component: System Administration (show other bugs)
Version: master
Hardware: All All
: P5 - low major (vote)
Assignee: Jonathan Druart
QA Contact: Tomás Cohen Arazi
URL:
Keywords:
Depends on: 18928
Blocks:
  Show dependency treegraph
 
Reported: 2020-05-26 04:35 UTC by Victor Grousset/tuxayo
Modified: 2020-06-17 19:50 UTC (History)
4 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Who signed the patch off:
Text to go in the release notes:
Version(s) released in:
20.05.00, 19.11.07


Attachments
Bug 25601: (bug 18928 follow-up) Fix "Unset" for "return policy for $branch" (1.72 KB, patch)
2020-05-26 08:13 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 25601: (bug 18928 follow-up) Fix "Unset" for "return policy for $branch" (1.78 KB, patch)
2020-05-27 01:39 UTC, Victor Grousset/tuxayo
Details | Diff | Splinter Review
Bug 25601: (bug 18928 follow-up) Fix "Unset" for "return policy for $branch" (1.84 KB, patch)
2020-05-27 12:17 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Victor Grousset/tuxayo 2020-05-26 04:35:15 UTC
== Test plan ==
1. Home › Administration › Circulation and fine rules
2. "Select a library" → pick one in the list
3. "Default checkout, hold and return policy for XXXXXX" → unset
4. error


set_rule needs 'itemtype' to set 'returnbranch'! at /kohadevbox/koha/admin/smart-rules.pl line 169.
at /usr/share/perl/5.28/Carp.pm line 289

1 in Carp::croak at /usr/share/perl/5.28/Carp.pm line 289

2 in Koha::CirculationRules::set_rule at /kohadevbox/koha/Koha/CirculationRules.pm line 264
  261: 
  262:     # Enforce scope; a rule should be set for its defined scope, no more, no less.
  263:     foreach my $scope_level ( qw( branchcode categorycode itemtype ) ) {
  264:         if ( grep /$scope_level/, @{ $kind_info->{scope} } ) {
  265:             croak "set_rule needs '$scope_level' to set '$params->{rule_name}'!"
  266:                 unless exists $params->{$scope_level};
  267:         } else {
  

3 in Koha::CirculationRules::set_rules at /kohadevbox/koha/Koha/CirculationRules.pm line 333
  330: 
  331:     my $rule_objects = [];
  332:     while ( my ( $rule_name, $rule_value ) = each %$rules ) {
  333:         my $rule_object = Koha::CirculationRules->set_rule(
  334:             {
  335:                 %set_params,
  336:                 rule_name    => $rule_name,
  

4 in (eval) at /kohadevbox/koha/admin/smart-rules.pl line 169
  166:                 }
  167:             }
  168:         );
  169:         Koha::CirculationRules->set_rules(
  170:             {
  171:                 branchcode   => $branch,
  172:                 rules        => {
Comment 1 Victor Grousset/tuxayo 2020-05-26 04:48:32 UTC
Workaround: empty the fields and set the lists to the default value and save instead of using the unset button.
Comment 2 Victor Grousset/tuxayo 2020-05-26 04:52:52 UTC
It looks like a regression, I couldn't reproduce on 4 different demo instances running version 19.11

Setting severity to major (I don't know how actually bad this bug is)
Comment 3 Jonathan Druart 2020-05-26 08:13:42 UTC
Created attachment 105348 [details] [review]
Bug 25601: (bug 18928 follow-up) Fix "Unset" for "return policy for $branch"

returnbranch has to pass an itemtype key when setting the rule (here
setting it to unlimited for any item types)

Test plan:
1. Go the "Circulation and fine rules" admin page
2. Set a rule for "Checkout, hold policy by patron category"
3. Unset it
4. Select a library
5. Set a rule for "Checkout, hold policy by patron category"
6. Unset it

Display should be consistent with what you selected
Comment 4 Victor Grousset/tuxayo 2020-05-27 01:39:34 UTC
Created attachment 105372 [details] [review]
Bug 25601: (bug 18928 follow-up) Fix "Unset" for "return policy for $branch"

returnbranch has to pass an itemtype key when setting the rule (here
setting it to unlimited for any item types)

Test plan:
1. Go the "Circulation and fine rules" admin page
2. Set a rule for "Default checkout, hold and return policy"
3. Unset it
4. Select a library
5. Set a rule for "Default checkout, hold and return policy"
6. Unset it

Display should be consistent with what you selected

Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Comment 5 Victor Grousset/tuxayo 2020-05-27 01:40:26 UTC
> 2. Set a rule for "Checkout, hold policy by patron category"

You meant "Default checkout, hold and return policy" right?

Assumed yes, test plan was changed.
Comment 6 Jonathan Druart 2020-05-27 07:59:06 UTC
(In reply to Victor Grousset/tuxayo from comment #5)
> > 2. Set a rule for "Checkout, hold policy by patron category"
> 
> You meant "Default checkout, hold and return policy" right?
> 
> Assumed yes, test plan was changed.

Yes, I c/p the wrong header!
Comment 7 Tomás Cohen Arazi 2020-05-27 12:17:25 UTC
Created attachment 105392 [details] [review]
Bug 25601: (bug 18928 follow-up) Fix "Unset" for "return policy for $branch"

returnbranch has to pass an itemtype key when setting the rule (here
setting it to unlimited for any item types)

Test plan:
1. Go the "Circulation and fine rules" admin page
2. Set a rule for "Default checkout, hold and return policy"
3. Unset it
4. Select a library
5. Set a rule for "Default checkout, hold and return policy"
6. Unset it

Display should be consistent with what you selected

Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 8 Jonathan Druart 2020-05-29 09:49:01 UTC
Pushed to master for 20.05
Comment 9 Aleisha Amohia 2020-06-14 21:35:59 UTC
backported to 19.11.x for 19.11.07
Comment 10 Victor Grousset/tuxayo 2020-06-17 19:50:28 UTC
No need to backport to 19.05.x, it's not affected by this bug.