[Bug 30518] New: StockRotationItems crossing DST boundary throw invalid local time exception
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30518 Bug ID: 30518 Summary: StockRotationItems crossing DST boundary throw invalid local time exception Change sponsored?: --- Product: Koha Version: unspecified Hardware: All OS: All Status: NEW Severity: major Priority: P5 - low Component: Tools Assignee: koha-bugs@lists.koha-community.org Reporter: martin.renvoize@ptfs-europe.com QA Contact: testopia@bugs.koha-community.org The 'needs_advance' method can fall foul of DateTime math errors when crossing a DST boundary which can in turn the cron script failing. -- 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=30518 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|koha-bugs@lists.koha-commun |martin.renvoize@ptfs-europe |ity.org |.com -- 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=30518 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |dcook@prosentient.com.au -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30518 --- Comment #1 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 133233 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=133233&action=edit Bug 30518: Unit test This patch adds tests for checking the needs_advancing method against a timezone and datetime that would cause an Invalid local time failure To test: 1. Apply this patch 2. Run: $ kshell k$ prove t/db_dependent/StockRotationItems.t => FAIL: The feature is not implemented, tests fail to complete -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30518 --- Comment #2 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 133234 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=133234&action=edit Bug 30518: Correct DateTime maths for needs_advancing The needs_advancing method prior to this patch used basic DateTime arithmatic, adding a DateTime::Duration in Days to the Arrival date of the item and then comparing that to today. This, however, can cause bugs when the arrival + duration date coincides with a DST boundary and as such may result in an invalid local date. See https://metacpan.org/pod/DateTime#Making-Things-Simple for further details. This patch updates the code to use the DST safe delta_days method to count the days between arrival and now instead and then compares this integer to the defined duration of the stage. To test: 1. Re-run the unit tests, they should now pass. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30518 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30518 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |nick@bywatersolutions.com, | |tomascohen@gmail.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30518 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- 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=30518 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #133233|0 |1 is obsolete| | Attachment #133234|0 |1 is obsolete| | --- Comment #3 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 133295 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=133295&action=edit Bug 30518: Unit test This patch adds tests for checking the needs_advancing method against a timezone and datetime that would cause an Invalid local time failure To test: 1. Apply this patch 2. Run: $ kshell k$ prove t/db_dependent/StockRotationItems.t => FAIL: The feature is not implemented, tests fail to complete Signed-off-by: Nick Clemens <nick@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30518 --- Comment #4 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 133296 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=133296&action=edit Bug 30518: Correct DateTime maths for needs_advancing The needs_advancing method prior to this patch used basic DateTime arithmatic, adding a DateTime::Duration in Days to the Arrival date of the item and then comparing that to today. This, however, can cause bugs when the arrival + duration date coincides with a DST boundary and as such may result in an invalid local date. See https://metacpan.org/pod/DateTime#Making-Things-Simple for further details. This patch updates the code to use the DST safe delta_days method to count the days between arrival and now instead and then compares this integer to the defined duration of the stage. To test: 1. Re-run the unit tests, they should now pass. Signed-off-by: Nick Clemens <nick@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30518 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA --- Comment #5 from Nick Clemens <nick@bywatersolutions.com> --- Trivial and good change, covered by tests, passing QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30518 Fridolin Somers <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to master Version(s)| |22.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=30518 --- Comment #6 from Fridolin Somers <fridolin.somers@biblibre.com> --- Pushed to master for 22.05, thanks to everybody involved 🦄 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30518 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to master |Pushed to stable CC| |kyle@bywatersolutions.com Version(s)|22.05.00 |22.05.00,21.11.06 released in| | --- Comment #7 from Kyle M Hall <kyle@bywatersolutions.com> --- Pushed to 21.11.x for 21.11.06 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30518 Andrew Fuerste-Henry <andrew@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |andrew@bywatersolutions.com Status|Pushed to stable |Pushed to oldstable Version(s)|22.05.00,21.11.06 |22.05.00,21.11.06,21.05.15 released in| | --- Comment #8 from Andrew Fuerste-Henry <andrew@bywatersolutions.com> --- Pushed to 21.05.x for 21.05.15 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30518 Victor Grousset/tuxayo <victor@tuxayo.net> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)|22.05.00,21.11.06,21.05.15 |22.05.00,21.11.06,21.05.15, released in| |20.11.19 CC| |victor@tuxayo.net Status|Pushed to oldstable |Pushed to oldoldstable --- Comment #9 from Victor Grousset/tuxayo <victor@tuxayo.net> --- Trying to backport to 20.11.x Tests don't actually cover the changes. They pass without the implementation commit applied. So how should this be tested? I'm still trusting the test suite to cover for general regressions, so backported anyway. Backported: Pushed to 20.11.x branch for 20.11.19 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30518 wainuiwitikapark@catalyst.net.nz changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |wainuiwitikapark@catalyst.n | |et.nz --- Comment #10 from wainuiwitikapark@catalyst.net.nz --- Does this need to be backported to 19.11.x? -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org