From edc57a8e13ddd9f508d08f20f9b09c2e0f59a2f5 Mon Sep 17 00:00:00 2001 From: Emmi Takkinen Date: Tue, 9 Dec 2025 14:56:09 +0200 Subject: [PATCH] Bug 41386: Do not allow 0.00 values to be passed from Koha::Hold->cancel In method Koha::Hold->cancel in the section where we charge a cancel fee there is a if statement "if $charge" after calling sub add_debit. This allows value "0.00" to be passed to the add_debit and it then gets caught in statement "unless ( $amount > 0 )" which calls exception AmountNotPositive. This happens at least when syspref ExpireReservesMaxPickUpDelayCharge or circulation rule "Expired hold charge" has value 0.00 and cronscript cancel_expired_holds.pl is run. This patch fixes the if statement in Koha::Hold->cancel. To test: 1. Enable syspref ExpireReservesMaxPickUpDelay. 2. Set syspref ReservesMaxPickUpDelay value as 1. 3. Set syspref ExpireReservesMaxPickUpDelayCharge value as 0.00. 4. Make sure you have a waiting hold which has expiration date further in the past than ReservesMaxPickUpDelay value in your database. If not, create one. 5. Run cronscript cancel_expired_holds.pl. => Exception 'Koha::Exceptions::Account::AmountNotPositive' thrown 'Debit amount passed is not positive' is raised. 6. Set syspref ExpireReservesMaxPickUpDelayCharge value as 0 and run cronscript again. => No exception is raised. 7. Apply this patch and restart services if needed. 8. Create a new waiting hold with expiration date in the past. 9. Run cronscript again. => No exception is raised. => Confirm that no cancel fee was added for patron. Also prove t/db_dependent/Koha/Holds.t. Sponsored-by: Koha-Suomi Oy --- Koha/Hold.pm | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Koha/Hold.pm b/Koha/Hold.pm index 66a236a853..e8f3326ade 100644 --- a/Koha/Hold.pm +++ b/Koha/Hold.pm @@ -940,7 +940,7 @@ sub cancel { type => 'RESERVE_EXPIRED', item_id => $self->itemnumber } - ) if $charge; + ) if $charge > 0; } C4::Log::logaction( 'HOLDS', 'CANCEL', $self->reserve_id, $self, undef, $original ) -- 2.34.1