If one adds 0.00 as "Expired hold charge" rules value in circulation rules and charge is then tried to add to a patron via Koha::Account->add_debit, exception Koha::Exceptions::Account::AmountNotPositive is raised. This can be really confusing for the users since monetary values in circulation rules typically allow using decimal values in them.
The problem seems to be in the 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 then calls exception AmountNotPositive. This also happens when syspref ExpireReservesMaxPickUpDelayCharge has value 0.00, not just with rule "Expired hold charge". To reproduce: 1. Enable syspref ExpireReservesMaxPickUpDelay. 2. Set syspref ReservesMaxPickUpDelay value as 1. 3. Set syspref ExpireReservesMaxPickUpDelayCharge value as 0.00. 4. Find a waiting hold which has expiration date further than ReservesMaxPickUpDelay value is. 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 cron again. => NO exception is raised.
Created attachment 190387 [details] [review] 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