From af7bd592325c2f3145aceac09b53493c4c00e0fd Mon Sep 17 00:00:00 2001
From: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Date: Tue, 20 Apr 2021 15:14:41 +0100
Subject: [PATCH] Bug 25711: (QA follow-up) Refactor to clarify logic
At the point where 'ExpireReservesMaxPickUpDelayCharge' can/should kick
in, we should already have an item waiting (else what's triggered a pickup
delay). As such we can drop the fall through default of using the syspref
when we don't yet have an item.
We can also save us some calculations and database hits by only
calculating the charge value when we've actually been asked to charge.
So we also move the block inside the 'charge_cancel_fee' block.
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Petro Vashchuk <stalkernoid@gmail.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
---
Koha/Hold.pm | 11 ++++-------
1 file changed, 4 insertions(+), 7 deletions(-)
diff --git a/Koha/Hold.pm b/Koha/Hold.pm
index ad4e3a6c95..885b6139c9 100644
--- a/Koha/Hold.pm
+++ b/Koha/Hold.pm
@@ -604,8 +604,9 @@ sub cancel {
C4::Reserves::_FixPriority({ biblionumber => $self->biblionumber });
# and, if desired, charge a cancel fee
- my $charge;
- if (my $item = $self->item) {
+ 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(
@@ -618,11 +619,7 @@ sub cancel {
);
my $rule_value = $rule && $rule->rule_value // '';
$charge = $rule_value ne '' ? $rule_value : C4::Context->preference("ExpireReservesMaxPickUpDelayCharge");
- } else {
- $charge = C4::Context->preference("ExpireReservesMaxPickUpDelayCharge");
- }
- if ( $charge && $params->{'charge_cancel_fee'} ) {
my $account =
Koha::Account->new( { patron_id => $self->borrowernumber } );
$account->add_debit(
@@ -634,7 +631,7 @@ sub cancel {
type => 'RESERVE_EXPIRED',
item_id => $self->itemnumber
}
- );
+ ) if $charge;
}
C4::Log::logaction( 'HOLDS', 'CANCEL', $self->reserve_id, $self )
--
2.30.2