Bugzilla – Attachment 192383 Details for
Bug 41268
Circulation rules script has many conditionals
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 41268: (QA follow-up) Apply same simplification to remaining operations
Bug-41268-QA-follow-up-Apply-same-simplification-t.patch (text/plain), 14.12 KB, created by
Nick Clemens (kidclamp)
on 2026-02-03 13:05:38 UTC
(
hide
)
Description:
Bug 41268: (QA follow-up) Apply same simplification to remaining operations
Filename:
MIME Type:
Creator:
Nick Clemens (kidclamp)
Created:
2026-02-03 13:05:38 UTC
Size:
14.12 KB
patch
obsolete
>From e6ef2aec45950a20da33b4d113c8762e7d2684f9 Mon Sep 17 00:00:00 2001 >From: Martin Renvoize <martin.renvoize@openfifth.co.uk> >Date: Mon, 2 Feb 2026 21:46:53 +0000 >Subject: [PATCH] Bug 41268: (QA follow-up) Apply same simplification to > remaining operations > >This follow-up applies the same variable-based simplification pattern >to the remaining operations in smart-rules.pl: > >- cud-set-branch-defaults: 2 code paths reduced to 1 >- cud-add-branch-cat: 4 code paths reduced to 1 >- cud-add-open-article-requests-limit: 4 code paths reduced to 1 >- cud-del-open-article-requests-limit: 4 code paths reduced to 1 >- cud-add-branch-item: 4 code paths reduced to 1 > >Note: cud-delete-branch-cat was NOT simplified because it contains >additional business logic (setting itemtype rules only when deleting >the default categorycode), which is not simply a matter of converting >'*' to undef. > >Signed-off-by: Nick Clemens <nick@bywatersolutions.com> >--- > admin/smart-rules.pl | 295 +++++++++++-------------------------------- > 1 file changed, 71 insertions(+), 224 deletions(-) > >diff --git a/admin/smart-rules.pl b/admin/smart-rules.pl >index 577b4343155..f4534426b15 100755 >--- a/admin/smart-rules.pl >+++ b/admin/smart-rules.pl >@@ -341,58 +341,34 @@ elsif ( $op eq 'cud-add' ) { > my $returnbranch = $input->param('returnbranch'); > my $max_holds = strip_non_numeric( scalar $input->param('max_holds') ); > >- if ( $branch eq "*" ) { >- Koha::CirculationRules->set_rules( >- { >- itemtype => undef, >- branchcode => undef, >- rules => { >- holdallowed => $holdallowed, >- hold_fulfillment_policy => $hold_fulfillment_policy, >- bookings_lead_period => $bookings_lead_period, >- bookings_trail_period => $bookings_trail_period, >- returnbranch => $returnbranch, >- } >- } >- ); >- Koha::CirculationRules->set_rules( >- { >- categorycode => undef, >- branchcode => undef, >- rules => { >- patron_maxissueqty => $patron_maxissueqty, >- patron_maxonsiteissueqty => $patron_maxonsiteissueqty, >- } >- } >- ); >- } else { >- Koha::CirculationRules->set_rules( >- { >- itemtype => undef, >- branchcode => $branch, >- rules => { >- holdallowed => $holdallowed, >- hold_fulfillment_policy => $hold_fulfillment_policy, >- bookings_lead_period => $bookings_lead_period, >- bookings_trail_period => $bookings_trail_period, >- returnbranch => $returnbranch, >- } >+ my $rule_branch = $branch eq '*' ? undef : $branch; >+ >+ Koha::CirculationRules->set_rules( >+ { >+ itemtype => undef, >+ branchcode => $rule_branch, >+ rules => { >+ holdallowed => $holdallowed, >+ hold_fulfillment_policy => $hold_fulfillment_policy, >+ bookings_lead_period => $bookings_lead_period, >+ bookings_trail_period => $bookings_trail_period, >+ returnbranch => $returnbranch, > } >- ); >- Koha::CirculationRules->set_rules( >- { >- categorycode => undef, >- branchcode => $branch, >- rules => { >- patron_maxissueqty => $patron_maxissueqty, >- patron_maxonsiteissueqty => $patron_maxonsiteissueqty, >- } >+ } >+ ); >+ Koha::CirculationRules->set_rules( >+ { >+ categorycode => undef, >+ branchcode => $rule_branch, >+ rules => { >+ patron_maxissueqty => $patron_maxissueqty, >+ patron_maxonsiteissueqty => $patron_maxonsiteissueqty, > } >- ); >- } >+ } >+ ); > Koha::CirculationRules->set_rule( > { >- branchcode => $branch, >+ branchcode => $rule_branch, > categorycode => undef, > rule_name => 'max_holds', > rule_value => $max_holds, >@@ -406,57 +382,20 @@ elsif ( $op eq 'cud-add' ) { > my $max_holds = $input->param('max_holds'); > $max_holds = strip_non_numeric($max_holds); > >- if ( $branch eq "*" ) { >- if ( $categorycode eq "*" ) { >- Koha::CirculationRules->set_rules( >- { >- categorycode => undef, >- branchcode => undef, >- rules => { >- max_holds => $max_holds, >- patron_maxissueqty => $patron_maxissueqty, >- patron_maxonsiteissueqty => $patron_maxonsiteissueqty, >- } >- } >- ); >- } else { >- Koha::CirculationRules->set_rules( >- { >- categorycode => $categorycode, >- branchcode => undef, >- rules => { >- max_holds => $max_holds, >- patron_maxissueqty => $patron_maxissueqty, >- patron_maxonsiteissueqty => $patron_maxonsiteissueqty, >- } >- } >- ); >- } >- } elsif ( $categorycode eq "*" ) { >- Koha::CirculationRules->set_rules( >- { >- categorycode => undef, >- branchcode => $branch, >- rules => { >- max_holds => $max_holds, >- patron_maxissueqty => $patron_maxissueqty, >- patron_maxonsiteissueqty => $patron_maxonsiteissueqty, >- } >- } >- ); >- } else { >- Koha::CirculationRules->set_rules( >- { >- categorycode => $categorycode, >- branchcode => $branch, >- rules => { >- max_holds => $max_holds, >- patron_maxissueqty => $patron_maxissueqty, >- patron_maxonsiteissueqty => $patron_maxonsiteissueqty, >- } >+ my $rule_branch = $branch eq '*' ? undef : $branch; >+ my $rule_categorycode = $categorycode eq '*' ? undef : $categorycode; >+ >+ Koha::CirculationRules->set_rules( >+ { >+ categorycode => $rule_categorycode, >+ branchcode => $rule_branch, >+ rules => { >+ max_holds => $max_holds, >+ patron_maxissueqty => $patron_maxissueqty, >+ patron_maxonsiteissueqty => $patron_maxonsiteissueqty, > } >- ); >- } >+ } >+ ); > } elsif ( $op eq "cud-add-open-article-requests-limit" ) { > my $categorycode = $input->param('categorycode'); > my $open_article_requests_limit = strip_non_numeric( scalar $input->param('open_article_requests_limit') ); >@@ -465,78 +404,29 @@ elsif ( $op eq 'cud-add' ) { > if not defined $open_article_requests_limit # There is a JS check for that > || $open_article_requests_limit eq q{}; > >- if ( $branch eq "*" ) { >- if ( $categorycode eq "*" ) { >- Koha::CirculationRules->set_rules( >- { >- categorycode => undef, >- branchcode => undef, >- rules => { open_article_requests_limit => $open_article_requests_limit, } >- } >- ); >- } else { >- Koha::CirculationRules->set_rules( >- { >- categorycode => $categorycode, >- branchcode => undef, >- rules => { open_article_requests_limit => $open_article_requests_limit, } >- } >- ); >+ my $rule_branch = $branch eq '*' ? undef : $branch; >+ my $rule_categorycode = $categorycode eq '*' ? undef : $categorycode; >+ >+ Koha::CirculationRules->set_rules( >+ { >+ categorycode => $rule_categorycode, >+ branchcode => $rule_branch, >+ rules => { open_article_requests_limit => $open_article_requests_limit, } > } >- } elsif ( $categorycode eq "*" ) { >- Koha::CirculationRules->set_rules( >- { >- categorycode => undef, >- branchcode => $branch, >- rules => { open_article_requests_limit => $open_article_requests_limit, } >- } >- ); >- } else { >- Koha::CirculationRules->set_rules( >- { >- categorycode => $categorycode, >- branchcode => $branch, >- rules => { open_article_requests_limit => $open_article_requests_limit, } >- } >- ); >- } >+ ); > } elsif ( $op eq 'cud-del-open-article-requests-limit' ) { > my $categorycode = $input->param('categorycode'); >- if ( $branch eq "*" ) { >- if ( $categorycode eq "*" ) { >- Koha::CirculationRules->set_rules( >- { >- branchcode => undef, >- categorycode => undef, >- rules => { open_article_requests_limit => undef, } >- } >- ); >- } else { >- Koha::CirculationRules->set_rules( >- { >- categorycode => $categorycode, >- branchcode => undef, >- rules => { open_article_requests_limit => undef, } >- } >- ); >+ >+ my $rule_branch = $branch eq '*' ? undef : $branch; >+ my $rule_categorycode = $categorycode eq '*' ? undef : $categorycode; >+ >+ Koha::CirculationRules->set_rules( >+ { >+ branchcode => $rule_branch, >+ categorycode => $rule_categorycode, >+ rules => { open_article_requests_limit => undef, } > } >- } elsif ( $categorycode eq "*" ) { >- Koha::CirculationRules->set_rules( >- { >- branchcode => $branch, >- categorycode => undef, >- rules => { open_article_requests_limit => undef, } >- } >- ); >- } else { >- Koha::CirculationRules->set_rules( >- { >- categorycode => $categorycode, >- branchcode => $branch, >- rules => { open_article_requests_limit => undef, } >- } >- ); >- } >+ ); > } elsif ( $op eq "cud-set-article-request-fee" ) { > > my $category = $input->param('article_request_fee_category'); >@@ -573,65 +463,22 @@ elsif ( $op eq 'cud-add' ) { > my $bookings_trail_period = $input->param('bookings_trail_period'); > my $returnbranch = $input->param('returnbranch'); > >- if ( $branch eq "*" ) { >- if ( $itemtype eq "*" ) { >- Koha::CirculationRules->set_rules( >- { >- itemtype => undef, >- branchcode => undef, >- rules => { >- holdallowed => $holdallowed, >- hold_fulfillment_policy => $hold_fulfillment_policy, >- bookings_lead_period => $bookings_lead_period, >- bookings_trail_period => $bookings_trail_period, >- returnbranch => $returnbranch, >- } >- } >- ); >- } else { >- Koha::CirculationRules->set_rules( >- { >- itemtype => $itemtype, >- branchcode => undef, >- rules => { >- holdallowed => $holdallowed, >- hold_fulfillment_policy => $hold_fulfillment_policy, >- bookings_lead_period => $bookings_lead_period, >- bookings_trail_period => $bookings_trail_period, >- returnbranch => $returnbranch, >- } >- } >- ); >- } >- } elsif ( $itemtype eq "*" ) { >- Koha::CirculationRules->set_rules( >- { >- itemtype => undef, >- branchcode => $branch, >- rules => { >- holdallowed => $holdallowed, >- hold_fulfillment_policy => $hold_fulfillment_policy, >- bookings_lead_period => $bookings_lead_period, >- bookings_trail_period => $bookings_trail_period, >- returnbranch => $returnbranch, >- } >- } >- ); >- } else { >- Koha::CirculationRules->set_rules( >- { >- itemtype => $itemtype, >- branchcode => $branch, >- rules => { >- holdallowed => $holdallowed, >- hold_fulfillment_policy => $hold_fulfillment_policy, >- bookings_lead_period => $bookings_lead_period, >- bookings_trail_period => $bookings_trail_period, >- returnbranch => $returnbranch, >- } >+ my $rule_branch = $branch eq '*' ? undef : $branch; >+ my $rule_itemtype = $itemtype eq '*' ? undef : $itemtype; >+ >+ Koha::CirculationRules->set_rules( >+ { >+ itemtype => $rule_itemtype, >+ branchcode => $rule_branch, >+ rules => { >+ holdallowed => $holdallowed, >+ hold_fulfillment_policy => $hold_fulfillment_policy, >+ bookings_lead_period => $bookings_lead_period, >+ bookings_trail_period => $bookings_trail_period, >+ returnbranch => $returnbranch, > } >- ); >- } >+ } >+ ); > } elsif ( $op eq 'cud-mod-refund-lost-item-fee-rule' ) { > > my $lostreturn = $input->param('lostreturn'); >-- >2.39.5
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 41268
:
189689
|
192116
|
192343
|
192344
|
192382
| 192383