| Summary: | Adding 0.00 as value for "Expired hold charge" in circulation rules can lead to exception Koha::Exceptions::Account::AmountNotPositive | ||
|---|---|---|---|
| Product: | Koha | Reporter: | Emmi Takkinen <emmi.takkinen> |
| Component: | Fines and fees | Assignee: | Emmi Takkinen <emmi.takkinen> |
| Status: | Needs Signoff --- | QA Contact: | Testopia <testopia> |
| Severity: | normal | ||
| Priority: | P5 - low | ||
| Version: | Main | ||
| Hardware: | All | ||
| OS: | All | ||
| GIT URL: | Initiative type: | --- | |
| Sponsorship status: | --- | Comma delimited list of Sponsors: | |
| Crowdfunding goal: | 0 | Patch complexity: | --- |
| Documentation contact: | Documentation submission: | ||
| Text to go in the release notes: | Version(s) released in: | ||
| Circulation function: | |||
| Attachments: | Bug 41386: Do not allow 0.00 values to be passed from Koha::Hold->cancel | ||
|
Description
Emmi Takkinen
2025-12-05 13:37:19 UTC
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 |