The 'needs_advance' method can fall foul of DateTime math errors when crossing a DST boundary which can in turn the cron script failing.
Created attachment 133233 [details] [review] 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
Created attachment 133234 [details] [review] 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.
Created attachment 133295 [details] [review] 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>
Created attachment 133296 [details] [review] 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>
Trivial and good change, covered by tests, passing QA
Pushed to master for 22.05, thanks to everybody involved [U+1F984]
Pushed to 21.11.x for 21.11.06
Pushed to 21.05.x for 21.05.15
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
Does this need to be backported to 19.11.x?