From 4079076e9b0223346b5a33502cefaedf5218a32e Mon Sep 17 00:00:00 2001 From: Petro Vashchuk 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 --- 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