[Bug 42545] New: Koha::Calendar::days_between skips holiday subtraction for end date if time is early
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42545 Bug ID: 42545 Summary: Koha::Calendar::days_between skips holiday subtraction for end date if time is early Initiative type: --- Sponsorship --- status: Product: Koha Version: 24.11 Hardware: All OS: All Status: NEW Severity: critical Priority: P5 - low Component: OPAC Assignee: oleonard@myacpl.org Reporter: apirak@punsarn.asia QA Contact: testopia@bugs.koha-community.org Created attachment 198697 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=198697&action=edit This patch truncates both dates to the beginning of the day after cloning and setting the time zone, ensuring the loop correctly processes all calendar days between the two dates. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42545 --- Comment #1 from apirak <apirak@punsarn.asia> --- Test Plan: 1. Set 'finesCalendar' syspref to 'noFinesWhenClosed'. 2. Set 'CalculateFinesOnReturn' syspref to 'Yes'. 3. Set 'finesMode' syspref to 'Production'. 4. Define a special holiday for today in the library calendar. 5. Identify an overdue item for a patron. Manually set its 'date_due' in the database to several days ago with a time of 23:59:00. 6. Renew the item today at any time BEFORE 23:59:00 (e.g., 10:00 AM). 7. Observe that an extra day of fine is charged because today (the holiday) was not subtracted. 8. Apply this patch. 9. Repeat the test and observe that the holiday is now correctly subtracted. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42545 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff CC| |martin.renvoize@openfifth.c | |o.uk -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42545 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- QA Contact|testopia@bugs.koha-communit |martin.renvoize@openfifth.c |y.org |o.uk Assignee|oleonard@myacpl.org |apirak@punsarn.asia -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42545 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Version|24.11 |Main -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42545 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Sponsorship status|--- |Unsponsored Patch complexity|--- |Trivial patch Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42545 --- Comment #2 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 199257 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=199257&action=edit Bug 42545: Koha::Calendar::days_between skips holiday subtraction for end date if time is early In Koha::Calendar::days_between, the holiday-subtraction loop uses a comparison that includes time. If the end_dt is a holiday and its time is earlier than the start_dt time (common in renewals), the loop terminates before processing the end_dt, leading to an extra fine day. This patch truncates both dates to the beginning of the day after cloning and setting the time zone, ensuring the loop correctly processes all calendar days between the two dates. Test Plan: 1. Set 'finesCalendar' syspref to 'noFinesWhenClosed'. 2. Set 'CalculateFinesOnReturn' syspref to 'Yes'. 3. Set 'finesMode' syspref to 'Production'. 4. Define a special holiday for today in the library calendar. 5. Identify an overdue item for a patron. Manually set its 'date_due' in the database to several days ago with a time of 23:59:00. 6. Renew the item today at any time BEFORE 23:59:00 (e.g., 10:00 AM). 7. Observe that an extra day of fine is charged because today (the holiday) was not subtracted. 8. Apply this patch. 9. Repeat the test and observe that the holiday is now correctly subtracted. Signed-off-by: Martin Renvoize <martin.renvoize@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=42545 --- Comment #3 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 199258 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=199258&action=edit Bug 42545: (QA follow-up) Add POD for _init and _holidays and fix incorrect test expectation The existing 'days: 289 hours, 2 holidays' test asserted 8-1=7 but its own description named 2 holidays. The expected value matched the buggy behaviour (loop exiting before the holiday end date) rather than the correct result. Update to 8-2=6 to reflect the fix. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42545 --- Comment #4 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 199259 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=199259&action=edit Bug 42545: (QA follow-up) Add unit tests for holiday subtraction when return time is before due time Adds a dedicated subtest covering the specific scenario: daily loans have a due time of 23:59, so a return earlier in the day on a holiday would previously cause days_between to exit its loop before processing the end date, silently skipping the holiday subtraction. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42545 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #198697|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42545 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42545 --- Comment #5 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- This goes back to at least 24.11 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42545 Lucas Gass (lukeg) <lucas@bywatersolutions.com> 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=42545 --- Comment #6 from Lucas Gass (lukeg) <lucas@bywatersolutions.com> --- Nice work everyone! Pushed to main for 26.05 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42545 Jacob O'Mara <jacob.omara@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)|26.05.00 |26.05.00,25.11.05 released in| | Status|Pushed to main |Pushed to stable -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42545 --- Comment #7 from Jacob O'Mara <jacob.omara@openfifth.co.uk> --- Thanks all, pushed to 25.11.x -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42545 Wainui Witika-Park <wainuiwitikapark@catalyst.net.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |wainuiwitikapark@catalyst.n | |et.nz --- Comment #8 from Wainui Witika-Park <wainuiwitikapark@catalyst.net.nz> --- On 25.05: finesCalendar has options: - Use the calendar - Ignore the calendar CalculateFinesOnReturn has options: - Do - Don't finesMode has options: - Don't calculate - Calculate and charge -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42545 Wainui Witika-Park <wainuiwitikapark@catalyst.net.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to stable |Pushed to oldstable -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42545 Fridolin Somers <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |fridolin.somers@biblibre.co | |m --- Comment #9 from Fridolin Somers <fridolin.somers@biblibre.com> --- On 25.05 I'd say try the unit test change, then check if main patch fixes the test -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42545 Wainui Witika-Park <wainuiwitikapark@catalyst.net.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)|26.05.00,25.11.05 |26.05.00,25.11.05,25.05.13 released in| | Status|Pushed to oldstable |Pushed to oldoldstable --- Comment #10 from Wainui Witika-Park <wainuiwitikapark@catalyst.net.nz> --- (In reply to Fridolin Somers from comment #9)
On 25.05 I'd say try the unit test change, then check if main patch fixes the test
Thank you! t/db_dependent/Calendar.t passed before applying any patches Applied the unit test -> t/db_dependent/Calendar.t failed Applied the main patches -> t/db_dependent/Calendar.t passes ------------------ 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=42545 Fridolin Somers <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to oldoldstable |Needs documenting --- Comment #11 from Fridolin Somers <fridolin.somers@biblibre.com> --- This can have a lot of impact on important feature. So I choose to not impact 24.11 LTS for stability -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org