From 87d7e7f6951569209d316122f0539bf3f913001e 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. --- 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 d7b2445dda..e87ee2946d 100644 --- a/Koha/CirculationRules.pm +++ b/Koha/CirculationRules.pm @@ -605,6 +605,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 5edacb56a4..db8e726021 100644 --- a/Koha/Hold.pm +++ b/Koha/Hold.pm @@ -605,19 +605,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 7117484d20..d9096ef366 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; @@ -656,6 +656,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.35.3