From 85aed15de61acae2e418ed92a1b24d59055a9b45 Mon Sep 17 00:00:00 2001 From: Petro Vashchuk Date: Mon, 9 Aug 2021 17:45:14 +0300 Subject: [PATCH] Bug 25711: move get_effective_expire_charge to Koha::CirculationRules Move get_effective_expire_reserves_charge as a new method in Koha::CirculationRules to retrieve the effective value of the rule, which means it will get the value from existing circulation rule and if it doesn't exist or set to undefined it will get the default value from the preferences. Also added tests for this logic. Signed-off-by: Martin Renvoize Signed-off-by: Kyle M Hall --- Koha/CirculationRules.pm | 28 ++++++++ Koha/Hold.pm | 11 +-- t/db_dependent/Koha/CirculationRules.t | 94 +++++++++++++++++++++++++- 3 files changed, 124 insertions(+), 9 deletions(-) diff --git a/Koha/CirculationRules.pm b/Koha/CirculationRules.pm index 22a6f0cf74..dc157c12de 100644 --- a/Koha/CirculationRules.pm +++ b/Koha/CirculationRules.pm @@ -747,6 +747,34 @@ sub get_effective_daysmode { } +=head3 get_effective_expire_reserves_charge + +Return the value for expire_reserves_charge defined in the circulation rules. +If not defined (or empty string), the value of the system preference ExpireReservesMaxPickUpDelayCharge is returned + +=cut + +sub get_effective_expire_reserves_charge { + my ( $class, $params ) = @_; + + my $itemtype = $params->{itemtype}; + my $branchcode = $params->{branchcode}; + my $categorycode = $params->{categorycode}; + + my $expire_reserves_charge_rule = $class->get_effective_rule( + { + itemtype => $itemtype, + branchcode => $branchcode, + categorycode => $categorycode, + rule_name => 'expire_reserves_charge', + } + ); + + # return the rule value (incl. 0) if rule found so there's an object in the variable, + # or return default value from sysprefs when rule wasn't found and there's undef + return $expire_reserves_charge_rule ? $expire_reserves_charge_rule->rule_value : C4::Context->preference("ExpireReservesMaxPickUpDelayCharge"); + +} =head3 type diff --git a/Koha/Hold.pm b/Koha/Hold.pm index 1245f94f8a..62648edbf4 100644 --- a/Koha/Hold.pm +++ b/Koha/Hold.pm @@ -740,19 +740,14 @@ sub cancel { # and, if desired, charge a cancel fee if ( $params->{'charge_cancel_fee'} ) { - my $charge; my $item = $self->item; - my $branchcode = C4::Reserves::GetReservesControlBranch($item->unblessed, $self->borrower->unblessed); - - my $rule = Koha::CirculationRules->get_effective_rule( + my $charge = Koha::CirculationRules->get_effective_expire_reserves_charge( { + itemtype => $item->effective_itemtype, + branchcode => C4::Reserves::GetReservesControlBranch($item->unblessed, $self->borrower->unblessed), categorycode => $self->borrower->categorycode, - itemtype => $item->effective_itemtype, - branchcode => $branchcode, - rule_name => 'expire_reserves_charge', } ); - $charge = $rule ? $rule->rule_value : C4::Context->preference("ExpireReservesMaxPickUpDelayCharge"); my $account = Koha::Account->new( { patron_id => $self->borrowernumber } ); diff --git a/t/db_dependent/Koha/CirculationRules.t b/t/db_dependent/Koha/CirculationRules.t index 90eb827aff..928fe6104b 100755 --- a/t/db_dependent/Koha/CirculationRules.t +++ b/t/db_dependent/Koha/CirculationRules.t @@ -20,7 +20,7 @@ use Modern::Perl; use Benchmark; -use Test::More tests => 7; +use Test::More tests => 8; use Test::Deep qw( cmp_methods ); use Test::Exception; @@ -729,6 +729,98 @@ subtest 'get_effective_daysmode' => sub { $schema->storage->txn_rollback; }; +subtest 'get_effective_expire_reserves_charge' => sub { + plan tests => 4; + + $schema->storage->txn_begin; + + Koha::CirculationRules->search({ rule_name => 'expire_reserves_charge' })->delete; + + t::lib::Mocks::mock_preference( 'ExpireReservesMaxPickUpDelayCharge', 10 ); + + is( + Koha::CirculationRules->get_effective_expire_reserves_charge( + { + itemtype => undef, + branchcode => undef, + categorycode => undef, + } + ), + '10', + 'use the default pref value as the circ rule does not exist' + ); + + Koha::CirculationRules->set_rule( + { + branchcode => '*', + categorycode => '*', + itemtype => '*', + rule_name => 'expire_reserves_charge', + rule_value => '20' + } + ); + + is( + Koha::CirculationRules->get_effective_expire_reserves_charge( + { + categorycode => undef, + itemtype => undef, + branchcode => undef + } + ), + '20', + "use the value from the circ rules" + ); + + t::lib::Mocks::mock_preference( 'ExpireReservesMaxPickUpDelayCharge', 30 ); + + Koha::CirculationRules->set_rule( + { + branchcode => '*', + categorycode => '*', + itemtype => '*', + rule_name => 'expire_reserves_charge', + rule_value => undef + } + ); + + is( + Koha::CirculationRules->get_effective_expire_reserves_charge( + { + categorycode => undef, + itemtype => undef, + branchcode => undef + } + ), + '30', + "use the default pref value for as the circ rule has undefined value" + ); + + Koha::CirculationRules->set_rule( + { + branchcode => '*', + categorycode => '*', + itemtype => '*', + rule_name => 'expire_reserves_charge', + rule_value => '0' + } + ); + + is( + Koha::CirculationRules->get_effective_expire_reserves_charge( + { + categorycode => undef, + itemtype => undef, + branchcode => undef + } + ), + '0', + "use the value from the circ rules for even though it's 0" + ); + + $schema->storage->txn_rollback; +}; + subtest 'get_lostreturn_policy() tests' => sub { plan tests => 7; -- 2.42.0