https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41386 Emmi Takkinen <emmi.takkinen@koha-suomi.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED --- Comment #1 from Emmi Takkinen <emmi.takkinen@koha-suomi.fi> --- 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. -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41386 --- Comment #2 from Emmi Takkinen <emmi.takkinen@koha-suomi.fi> --- Created attachment 190387 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=190387&action=edit 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 -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41386 Bernard <bernard.scaife@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #190387|0 |1 is obsolete| | --- Comment #3 from Bernard <bernard.scaife@openfifth.co.uk> --- Created attachment 193588 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=193588&action=edit 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. [All 14 tests passed] Sponsored-by: Koha-Suomi Oy Signed-off-by: Bernard Scaife <bernard.scaife@openfifth.co.uk> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41386 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off Sponsorship status|--- |Sponsored Comma delimited| |Koha-Suomi Oy list of Sponsors| | -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41386 --- Comment #5 from David Nind <david@davidnind.com> --- Created attachment 195728 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=195728&action=edit 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 Signed-off-by: Bernard Scaife <bernard.scaife@openfifth.co.uk> Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41386 Emily Lamancusa (emlam) <emily.lamancusa@montgomerycountymd.gov> changed: What |Removed |Added ---------------------------------------------------------------------------- QA Contact|testopia@bugs.koha-communit |emily.lamancusa@montgomeryc |y.org |ountymd.gov CC| |emily.lamancusa@montgomeryc | |ountymd.gov -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41386 --- Comment #7 from Emily Lamancusa (emlam) <emily.lamancusa@montgomerycountymd.gov> --- Created attachment 195856 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=195856&action=edit 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 Signed-off-by: Bernard Scaife <bernard.scaife@openfifth.co.uk> Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41386 Emily Lamancusa (emlam) <emily.lamancusa@montgomerycountymd.gov> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA --- Comment #9 from Emily Lamancusa (emlam) <emily.lamancusa@montgomerycountymd.gov> --- Nice catch, thanks Emmi! t/db_dependent/Hold.t is failing, but not because of this patch - it's failing on main too. This one looks good, so passing QA (and I'll file a separate bug for the failing test if someone else hasn't already) -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41386 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to main Version(s)| |26.05.00 released in| | -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41386 Emmi Takkinen <emmi.takkinen@koha-suomi.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Text to go in the| |Using value 0.00 in release notes| |"Expired hold charge" rule | |on circulation rules caused | |Koha to die with exception | |Koha::Exceptions::Account:: | |AmountNotPositive when | |expired hold charge was | |added for patron. This was | |caused by error in if | |statement in method | |Koha::Hold->cancel which | |allowed value 0.00 to be | |passed to method add_debit. | |This method then raised | |exception since value 0.00 | |is not positive. This patch | |fixes the erroneous if | |statement in method | |Koha::Hold->cancel. --- Comment #12 from Emmi Takkinen <emmi.takkinen@koha-suomi.fi> --- Added release notes, feel free to adjust. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41386 Jacob O'Mara <jacob.omara@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to main |Pushed to stable Version(s)|26.05.00 |26.05.00,25.11.05 released in| | -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41386 Wainui Witika-Park <wainuiwitikapark@catalyst.net.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to stable |Pushed to oldstable Version(s)|26.05.00,25.11.05 |26.05.00,25.11.05,25.05.13 released in| | CC| |wainuiwitikapark@catalyst.n | |et.nz --- Comment #14 from Wainui Witika-Park <wainuiwitikapark@catalyst.net.nz> --- Backported to 25.05.x for 25.05.13 -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41386 Fridolin Somers <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on| |25711 Status|Pushed to oldoldstable |Needs documenting CC| |fridolin.somers@biblibre.co | |m --- Comment #15 from Fridolin Somers <fridolin.somers@biblibre.com> --- Depends on Bug 25711 not in 24.11.x Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25711 [Bug 25711] Move ExpireReservesMaxPickUpDelayCharge to the circulation rules -- You are receiving this mail because: You are watching all bug changes.