[Bug 9031] New: Overdue items crossing DST boundary throw invalid local time exception
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=9031 Priority: P5 - low Change sponsored?: --- Bug ID: 9031 CC: gmcharlt@gmail.com, kyle.m.hall@gmail.com Assignee: koha-bugs@lists.koha-community.org Summary: Overdue items crossing DST boundary throw invalid local time exception Severity: minor Classification: Unclassified OS: All Reporter: robsongalluci@hotmail.com Hardware: All Status: NEW Version: 3.8 Component: Circulation Product: Koha When checking in an overdue item with a DST boundary between original due date and actual check in date, we get the invalid local time error. The issue happens with places that set DST by removing the hour between 0:00 and 0:59, as you can see here (http://www.nntp.perl.org/group/perl.datetime/2012/06/msg7854.html), not with every DST boundary crossing. The problem seems to be in the days_between and hours_between subroutines at Koha::Calendar. I've made a quick fix converting the dates to UTC before the calculation of the number of days, but I don't know if it deserves to be a patch. Ideally, all internal date handling should be done in UTC, using local timezones for input and output only, as DateTime page recommends:
- use UTC for all calculations
If you do care about time zones (particularly DST) or leap seconds, try to use non-UTC time zones for presentation and user input only. Convert to UTC immediately and convert back to the local time zone for presentation
https://metacpan.org/module/DateTime#Making-Things-Simple -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=9031 --- Comment #1 from Robson Galluci <robsongalluci@hotmail.com> --- Created attachment 13714 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=13714&action=edit As described above, this patch converts $start_dt and $end_dt to UTC before performing the calculation of the number of days between due date and actual check in date. -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=9031 Robson Galluci <robsongalluci@hotmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #13714|As described above, this |Patch description|patch converts $start_dt | |and $end_dt to UTC before | |performing the calculation | |of the number of days | |between due date and actual | |check in date. | -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=9031 --- Comment #2 from Robson Galluci <robsongalluci@hotmail.com> --- Comment on attachment 13714 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=13714 Patch As described above, this patch converts $start_dt and $end_dt to UTC before performing the calculation of the number of days between due date and actual check in date. -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=9031 Chris Cormack <chris@bigballofwax.co.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |chris@bigballofwax.co.nz --- Comment #3 from Chris Cormack <chris@bigballofwax.co.nz> --- Robson, would you like me to convert your patch into one that has your email etc? (IE did you create this patch with git-format-patch ?) -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=9031 --- Comment #4 from Robson Galluci <robsongalluci@hotmail.com> --- (In reply to comment #3)
Robson, would you like me to convert your patch into one that has your email etc?
(IE did you create this patch with git-format-patch ?)
Yes, it was created with git-format-patch. I guess I butchered it somehow before posting. Thanks. -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=9031 --- Comment #5 from Chris Cormack <chris@bigballofwax.co.nz> --- Hmm yeah it seems to be missing a lot of stuff, it should look more like http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=13159 -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=9031 --- Comment #6 from Robson Galluci <robsongalluci@hotmail.com> --- Anyway, I've sent it to the patches list, but it's now awaiting moderation because I used my GMail account, while my subscription uses Hotmail. Forgive the noobness, it's my first time doing this stuff. I still have the file though, I can post it again. It looks like this:
From 588a40a14fb7e33229e07f8260b73532aa7ddd67 Mon Sep 17 00:00:00 2001 From: Robson Galluci <robsongalluci@gmail.com> Date: Tue, 27 Nov 2012 20:04:06 -0200 Subject: [PATCH] Bug 9031 - Overdue items crossing DST boundary throw invalid local time exception
This patch converts $start_dt and $end_dt to UTC before performing the calculation of the number of days between due date and actual check in date. --- Koha/Calendar.pm | 6 ++++++ 1 files changed, 6 insertions(+), 0 deletions(-) diff --git a/Koha/Calendar.pm b/Koha/Calendar.pm index 90069b6..7075c49 100644 --- a/Koha/Calendar.pm +++ b/Koha/Calendar.pm @@ -241,6 +241,8 @@ sub days_between { my $start_dt = shift; my $end_dt = shift; + $start_dt->set_time_zone('UTC'); + $end_dt->set_time_zone('UTC'); # start and end should not be closed days my $days = $start_dt->delta_days($end_dt)->delta_days; @@ -263,6 +265,10 @@ sub hours_between { my $duration = $end_dt->delta_ms($start_dt); $start_dt->truncate( to => 'day' ); $end_dt->truncate( to => 'day' ); + + $start_dt->set_time_zone('UTC'); + $end_dt->set_time_zone('UTC'); + # NB this is a kludge in that it assumes all days are 24 hours # However for hourly loans the logic should be expanded to # take into account open/close times then it would be a duration -- 1.7.2.5 -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=9031 Robson Galluci <robsongalluci@hotmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #13714|0 |1 is obsolete| | --- Comment #7 from Robson Galluci <robsongalluci@hotmail.com> --- Created attachment 13795 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=13795&action=edit As described above, this patch converts $start_dt and $end_dt to UTC before performing the calculation of the number of days between due date and actual check in date. Fixed patch -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=9031 Robson Galluci <robsongalluci@hotmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #13795|As described above, this |Bug 9031 description|patch converts $start_dt | |and $end_dt to UTC before | |performing the calculation | |of the number of days | |between due date and actual | |check in date. | -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=9031 Chris Cormack <chris@bigballofwax.co.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff Patch complexity|--- |Small patch --- Comment #8 from Chris Cormack <chris@bigballofwax.co.nz> --- Hi Robson Thanks for the valid patch, however it is causing the unit tests to fail. perl t/Calendar.t 1..26 ok 1 - use Koha::Calendar; ok 2 - use C4::Calendar; ok 3 - Calendar class returned isa Koha::Calendar ok 4 - Sunday is a closed day ok 5 - Monday is not a closed day ok 6 - month/day closed day test ok 7 - special closed day test ok 8 - open day test ok 9 - addDate skips closed Sunday ok 10 - Negative call to addDate ok 11 - days_between calculates correctly ok 12 - is holiday for the next test not ok 13 - Date should be the same after is_holiday # Failed test 'Date should be the same after is_holiday' # at t/Calendar.t line 99. # got: '2012-09-17T16:30:00' # expected: '2012-09-17T17:30:00' ok 14 - test larger intervals ok 15 - test positive intervals ok 16 - test parameter order not relevant ok 17 - days_between calculates correctly not ok 18 - holiday correctly recognized # Failed test 'holiday correctly recognized' # at t/Calendar.t line 119. # got: 14 # expected: 13 not ok 19 - multiple holidays correctly recognized # Failed test 'multiple holidays correctly recognized' # at t/Calendar.t line 124. # got: 14 # expected: 12 ok 20 - Single day add (Datedue, matches holiday, shift) ok 21 - Two days add, skips holiday (Datedue) not ok 22 - Add 7 days (Datedue) # Failed test 'Add 7 days (Datedue)' # at t/Calendar.t line 145. # got: '2012-07-30T10:53:00' # expected: '2012-07-30T11:53:00' ok 23 - Single day add (Calendar) not ok 24 - Add 7 days (Calendar) # Failed test 'Add 7 days (Calendar)' # at t/Calendar.t line 162. # got: '2012-07-31T10:53:00' # expected: '2012-07-31T11:53:00' ok 25 - Single day add (Days) not ok 26 - Add 7 days (Days) # Failed test 'Add 7 days (Days)' # at t/Calendar.t line 179. # got: '2012-07-30T10:53:00' # expected: '2012-07-30T11:53:00' # Looks like you failed 6 tests of 26. Could you have a look at this please. -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=9031 Chris Cormack <chris@bigballofwax.co.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |In Discussion -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org