Bugzilla – Attachment 105154 Details for
Bug 25554
Refactor rule kinds assignments in CirculationRules.pm
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 25554: Refactor rule kinds assignments in CirculationRules.pm
Bug-25554-Refactor-rule-kinds-assignments-in-Circu.patch (text/plain), 7.52 KB, created by
Peter Vashchuk
on 2020-05-20 15:02:53 UTC
(
hide
)
Description:
Bug 25554: Refactor rule kinds assignments in CirculationRules.pm
Filename:
MIME Type:
Creator:
Peter Vashchuk
Created:
2020-05-20 15:02:53 UTC
Size:
7.52 KB
patch
obsolete
>From 4079076e9b0223346b5a33502cefaedf5218a32e Mon Sep 17 00:00:00 2001 >From: Petro Vashchuk <stalkernoid@gmail.com> >Date: Wed, 20 May 2020 17:48:16 +0300 >Subject: [PATCH] Bug 25554: Refactor rule kinds assignments in > CirculationRules.pm > >CirculationRules.pm have repeated code with the same values assigned many times to rule kinds and also same hardcoded values appear in foreach loop. >Refactored by creating arrays once and then assigning and looping their values. > >Mentored-by: Andrew Nugged <nugged@gmail.com> >--- > Koha/CirculationRules.pm | 161 ++++++++++++--------------------------- > 1 file changed, 48 insertions(+), 113 deletions(-) > >diff --git a/Koha/CirculationRules.pm b/Koha/CirculationRules.pm >index 5042e6f55d..5efbc1f092 100644 >--- a/Koha/CirculationRules.pm >+++ b/Koha/CirculationRules.pm >@@ -45,118 +45,53 @@ Any attempt to set a rule with a nonsensical scope (for instance, setting the C< > > =cut > >+my $scope1 = [ 'branchcode', 'categorycode' ]; >+my $scope2 = [ 'branchcode', 'itemtype' ]; >+my $scope3 = [ 'branchcode', 'categorycode', 'itemtype' ]; >+ > our $RULE_KINDS = { >- refund => { >- scope => [ 'branchcode' ], >- }, >- >- patron_maxissueqty => { >- scope => [ 'branchcode', 'categorycode' ], >- }, >- patron_maxonsiteissueqty => { >- scope => [ 'branchcode', 'categorycode' ], >- }, >- max_holds => { >- scope => [ 'branchcode', 'categorycode' ], >- }, >- >- holdallowed => { >- scope => [ 'branchcode', 'itemtype' ], >- }, >- hold_fulfillment_policy => { >- scope => [ 'branchcode', 'itemtype' ], >- }, >- returnbranch => { >- scope => [ 'branchcode', 'itemtype' ], >- }, >- >- article_requests => { >- scope => [ 'branchcode', 'categorycode', 'itemtype' ], >- }, >- auto_renew => { >- scope => [ 'branchcode', 'categorycode', 'itemtype' ], >- }, >- cap_fine_to_replacement_price => { >- scope => [ 'branchcode', 'categorycode', 'itemtype' ], >- }, >- chargeperiod => { >- scope => [ 'branchcode', 'categorycode', 'itemtype' ], >- }, >- chargeperiod_charge_at => { >- scope => [ 'branchcode', 'categorycode', 'itemtype' ], >- }, >- fine => { >- scope => [ 'branchcode', 'categorycode', 'itemtype' ], >- }, >- finedays => { >- scope => [ 'branchcode', 'categorycode', 'itemtype' ], >- }, >- firstremind => { >- scope => [ 'branchcode', 'categorycode', 'itemtype' ], >- }, >- hardduedate => { >- scope => [ 'branchcode', 'categorycode', 'itemtype' ], >- }, >- hardduedatecompare => { >- scope => [ 'branchcode', 'categorycode', 'itemtype' ], >- }, >- holds_per_day => { >- scope => [ 'branchcode', 'categorycode', 'itemtype' ], >- }, >- holds_per_record => { >- scope => [ 'branchcode', 'categorycode', 'itemtype' ], >- }, >- issuelength => { >- scope => [ 'branchcode', 'categorycode', 'itemtype' ], >- }, >- lengthunit => { >- scope => [ 'branchcode', 'categorycode', 'itemtype' ], >- }, >- maxissueqty => { >- scope => [ 'branchcode', 'categorycode', 'itemtype' ], >- }, >- maxonsiteissueqty => { >- scope => [ 'branchcode', 'categorycode', 'itemtype' ], >- }, >- maxsuspensiondays => { >- scope => [ 'branchcode', 'categorycode', 'itemtype' ], >- }, >- no_auto_renewal_after => { >- scope => [ 'branchcode', 'categorycode', 'itemtype' ], >- }, >- no_auto_renewal_after_hard_limit => { >- scope => [ 'branchcode', 'categorycode', 'itemtype' ], >- }, >- norenewalbefore => { >- scope => [ 'branchcode', 'categorycode', 'itemtype' ], >- }, >- onshelfholds => { >- scope => [ 'branchcode', 'categorycode', 'itemtype' ], >- }, >- opacitemholds => { >- scope => [ 'branchcode', 'categorycode', 'itemtype' ], >- }, >- overduefinescap => { >- scope => [ 'branchcode', 'categorycode', 'itemtype' ], >- }, >- renewalperiod => { >- scope => [ 'branchcode', 'categorycode', 'itemtype' ], >- }, >- renewalsallowed => { >- scope => [ 'branchcode', 'categorycode', 'itemtype' ], >- }, >- rentaldiscount => { >- scope => [ 'branchcode', 'categorycode', 'itemtype' ], >- }, >- reservesallowed => { >- scope => [ 'branchcode', 'categorycode', 'itemtype' ], >- }, >- suspension_chargeperiod => { >- scope => [ 'branchcode', 'categorycode', 'itemtype' ], >- }, >- note => { # This is not really a rule. Maybe we will want to separate this later. >- scope => [ 'branchcode', 'categorycode', 'itemtype' ], >- }, >+ refund => { scope => ['branchcode'] }, >+ >+ patron_maxissueqty => { scope => $scope1 }, >+ patron_maxonsiteissueqty => { scope => $scope1 }, >+ max_holds => { scope => $scope1 }, >+ >+ holdallowed => { scope => $scope2 }, >+ hold_fulfillment_policy => { scope => $scope2 }, >+ returnbranch => { scope => $scope2 }, >+ >+ article_requests => { scope => $scope3 }, >+ auto_renew => { scope => $scope3 }, >+ cap_fine_to_replacement_price => { scope => $scope3 }, >+ chargeperiod => { scope => $scope3 }, >+ chargeperiod_charge_at => { scope => $scope3 }, >+ fine => { scope => $scope3 }, >+ finedays => { scope => $scope3 }, >+ firstremind => { scope => $scope3 }, >+ hardduedate => { scope => $scope3 }, >+ hardduedatecompare => { scope => $scope3 }, >+ holds_per_day => { scope => $scope3 }, >+ holds_per_record => { scope => $scope3 }, >+ issuelength => { scope => $scope3 }, >+ lengthunit => { scope => $scope3 }, >+ maxissueqty => { scope => $scope3 }, >+ maxonsiteissueqty => { scope => $scope3 }, >+ maxsuspensiondays => { scope => $scope3 }, >+ no_auto_renewal_after => { scope => $scope3 }, >+ no_auto_renewal_after_hard_limit => { scope => $scope3 }, >+ norenewalbefore => { scope => $scope3 }, >+ onshelfholds => { scope => $scope3 }, >+ opacitemholds => { scope => $scope3 }, >+ overduefinescap => { scope => $scope3 }, >+ renewalperiod => { scope => $scope3 }, >+ renewalsallowed => { scope => $scope3 }, >+ rentaldiscount => { scope => $scope3 }, >+ reservesallowed => { scope => $scope3 }, >+ suspension_chargeperiod => { scope => $scope3 }, >+ >+ note => { scope => $scope3 }, >+ # This is not really a rule. Maybe we will want to separate this later. >+ > # Not included (deprecated?): > # * accountsent > # * reservecharge >@@ -192,7 +127,7 @@ sub get_effective_rule { > } > > my $order_by = $params->{order_by} >- // { -desc => [ 'branchcode', 'categorycode', 'itemtype' ] }; >+ // { -desc => $scope3 }; > > my $search_params; > $search_params->{rule_name} = $rule_name; >@@ -260,7 +195,7 @@ sub set_rule { > unless defined $kind_info; > > # Enforce scope; a rule should be set for its defined scope, no more, no less. >- foreach my $scope_level ( qw( branchcode categorycode itemtype ) ) { >+ foreach my $scope_level ( @$scope3 ) { > if ( grep /$scope_level/, @{ $kind_info->{scope} } ) { > croak "set_rule needs '$scope_level' to set '$params->{rule_name}'!" > unless exists $params->{$scope_level}; >-- >2.17.1
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 25554
: 105154