gitea
/
Koha-clms
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

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>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
This commit is contained in:
Martin Renvoize 2026-02-02 21:46:53 +00:00 committed by Lucas Gass
parent e9d675fcbc
commit 1f51dab88e
No known key found for this signature in database
GPG Key ID: 60B447398B0D36CA
1 changed files with 71 additions and 224 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

295
admin/smart-rules.pl
View File

@ -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,
}
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 => undef,
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,
}
);
} 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,
}
}
);
Koha::CirculationRules->set_rules(
{
categorycode => undef,
branchcode => $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,
}
}
);
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 ( $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,
}
}
);
}
);
} 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,
}
}
);
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 ( $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,
}
}
);
}
);
} elsif ( $op eq 'cud-mod-refund-lost-item-fee-rule' ) {
my $lostreturn = $input->param('lostreturn');