[Bug 16376] New: Koha::Calendar->is_holiday date truncation creates fatal errors for TZ America/Santiago
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16376 Bug ID: 16376 Summary: Koha::Calendar->is_holiday date truncation creates fatal errors for TZ America/Santiago Change sponsored?: --- Product: Koha Version: 3.20 Hardware: All OS: All Status: NEW Severity: critical Priority: P5 - low Component: Circulation Assignee: koha-bugs@lists.koha-community.org Reporter: dcook@prosentient.com.au QA Contact: testopia@bugs.koha-community.org CC: gmcharlt@gmail.com, kyle.m.hall@gmail.com Koha::Calendar->is_holiday calls the following DateTime truncate method: $localdt->truncate( to => 'day' ); This will truncate 2015-09-06 12:00:00 down to 2015-09-06 00:00:00. This causes major problems in Santiago, Chile, because 2015-09-06 is an invalid local time due to DST. I can't explain why it's a problem in 2015 because DST is weird in Chile in 2015, but it's obviously a problem for every other year as well: http://www.timeanddate.com/time/change/chile/santiago?year=2015 Consider 2014: 2014 Sunday, 27 April, 12:00 Midnight Sunday, 7 September, 12:00 Midnight You shouldn't have a problem on 27 April, because when 27 April 12:00 midnight hits (relative to UTC), local time is changed to 27 April 11:00pm. While UTC marches on, local time has been magically moved back an hour. You will have a problem on 7 September 12:00 midnight though, because that time doesn't exist. As soon as we roll over from 6 September 11:59pm to the next minute... we're actually on to 7 September 01:00:00am. Midnight doesn't exist on that day, which creates the following error in DateTime.pm: "Invalid local time for date in time zone: America/Santiago" This is a problem that the DateTime writer predicted. You can see their comments here: http://cpansearch.perl.org/src/DROLSKY/DateTime-1.26/lib/DateTime.pm https://rt.cpan.org/Public/Bug/Display.html?id=93347 We should probably do something like: eval { $dt->truncate(to => 'day'); } || warn "Eval failed with '$@'\n"; Why are we even doing this day truncation? -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16376 --- Comment #1 from David Cook <dcook@prosentient.com.au> --- You can easily verify this with the following Perl code: use DateTime; my $dt = DateTime->new( month => 9, year => 2015, day => 6, hour => 12, minute => 00, second => 00, time_zone => 'America/Santiago', ); print "DateTime:\n"; print $dt."\n"; print "Timezone:\n"; print $dt->time_zone()->name."\n"; print "Let's truncate...\n"; $dt->truncate( to => 'day'); TZ=America/Santiago perl issues.pl DateTime: 2015-09-06T12:00:00 Timezone: America/Santiago Let's truncate... Invalid local time for date in time zone: America/Santiago -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16376 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |larry@opengeek.cl --- Comment #2 from David Cook <dcook@prosentient.com.au> --- This problem was brought to me by Larry Letelier who found that he couldn't use overdue_notices.pl because of this error. You can replicate it by creating a Koha::Calendar, and then running the following: use DateTime; use Koha::DateUtils; use Koha::Calendar; my $dt = DateTime->new( month => 9, year => 2015, day => 6, hour => 12, minute => 00, second => 00, time_zone => 'America/Santiago', ); my $now = DateTime->now(); my $calendar = Koha::Calendar->new(branchcode => "MAIN"); $calendar->days_between($dt, $now); Run that using the America/Santiago TZ environmental variable: TZ=America/Santiago perl issues.pl -- You should get the "Invalid local time for date in time zone: America/Santiago" message and the script should die. -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16376 --- Comment #3 from David Cook <dcook@prosentient.com.au> --- Ahh, I bet it's truncating it to compare it to "single_holidays" and "exception_holidays"... right... that makes sense. In that case, before truncating it, I think you have to turn it into a "floating" DateTime. Except that we're using the Timezone in single_holidays() and exception_holidays()... that's great... I think we might need to re-think Koha::Calendar in light of https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12669 -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16376 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |ernestobwh@hotmail.com --- Comment #4 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- *** Bug 13079 has been marked as a duplicate of this bug. *** -- 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=16376 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=8966 -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16376 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=12669 -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16376 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |tomascohen@gmail.com --- Comment #5 from Tomás Cohen Arazi <tomascohen@gmail.com> --- (In reply to David Cook from comment #3)
Ahh, I bet it's truncating it to compare it to "single_holidays" and "exception_holidays"... right... that makes sense.
In that case, before truncating it, I think you have to turn it into a "floating" DateTime.
Except that we're using the Timezone in single_holidays() and exception_holidays()... that's great...
I think we might need to re-think Koha::Calendar in light of https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12669
Hey, where's the patch? -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16376 --- Comment #6 from David Cook <dcook@prosentient.com.au> --- (In reply to Tomás Cohen Arazi from comment #5)
Hey, where's the patch?
I've been too busy working on other projects :/. While I might have time to write a patch, I don't have anywhere near the time to test it or write a comprehensive test plan for others to use... :/. -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16376 --- Comment #7 from Tomás Cohen Arazi <tomascohen@gmail.com> --- (In reply to David Cook from comment #6)
(In reply to Tomás Cohen Arazi from comment #5)
Hey, where's the patch?
I've been too busy working on other projects :/.
While I might have time to write a patch, I don't have anywhere near the time to test it or write a comprehensive test plan for others to use... :/.
If you write a patch, I write regression tests for it :-) -- 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=16376 --- Comment #8 from David Cook <dcook@prosentient.com.au> --- (In reply to Tomás Cohen Arazi from comment #7)
If you write a patch, I write regression tests for it :-)
Thanks, Tomas :). Used 'grep -l -R "Koha::Calendar" * --exclude-dir="blib" | xargs grep "exception_holidays"' to try to find everything using Koha::Calendar and exception_holidays... looks like it's only Koha::Calendar. Looks like it's only Koha::Calendar::exeption_holidays() and Koha::Calendar::is_holiday() as well. I'm sure this issue pervades Koha, but perhaps we can resolve the problem in this particular piece of it... -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16376 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff -- 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=16376 --- Comment #9 from David Cook <dcook@prosentient.com.au> --- Created attachment 51702 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=51702&action=edit Bug 16376 - Koha::Calendar->is_holiday date truncation creates fatal errors for TZ America/Santiago Using a DateTime object with a timezone of America/Santiago was causing fatal errors for Koha::Calendar->is_holiday and Koha::Calendar->exception_holidays, when the objects were truncated to an invalid local time. Using a floating zone allows us to use the same day, month, year for comparison purposes without running into the possibility of creating an invalid local time and thus a fatal software error. -- 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=16376 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jonathan.druart@bugs.koha-c | |ommunity.org -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16376 M. Tompsett <mtompset@hotmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |mtompset@hotmail.com --- Comment #10 from M. Tompsett <mtompset@hotmail.com> --- (In reply to Tomás Cohen Arazi from comment #7)
If you write a patch, I write regression tests for it :-)
There's a patch. Where are the regression tests, Tomás? ;) -- 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=16376 --- Comment #11 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- (In reply to David Cook from comment #9)
Using a floating zone allows us to use the same day, month, year for comparison purposes without running into the possibility of creating an invalid local time and thus a fatal software error.
Ok you will remove the software error, but I am not sure it's the way to go. Currently the dates are stored in local time (so with the tz), if we want to switch to floating time zones (what we should do at some point!) I think we should recalculate all dates present in DB. An easy workaround would be to use Koha::DateUtils, which deals (at least try to) with invalid dates due du DST. -- 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=16376 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Version|3.20 |unspecified -- 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=16376 --- Comment #12 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Moreover `grep time_zone Koha/Calendar.pm` will return another occurrence where the fix should be applied too. -- 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=16376 --- Comment #13 from David Cook <dcook@prosentient.com.au> --- (In reply to Jonathan Druart from comment #11)
Ok you will remove the software error, but I am not sure it's the way to go.
I would certainly prefer a more thorough overhaul of Koha's DateTime handling, but I think this is good enough for now. Chile can't use the overdue functionality in Koha at all at this stage without this fix.
Currently the dates are stored in local time (so with the tz), if we want to switch to floating time zones (what we should do at some point!) I think we should recalculate all dates present in DB. An easy workaround would be to use Koha::DateUtils, which deals (at least try to) with invalid dates due du DST.
Well, we calculate and interpret the dates using local time, but they're stored without timezone in the database. So I'm not sure what you mean by "recalculate all dates present in DB". The only difference between "local" and "floating" is the timezone. They're the same numbers, so there would be no recalculation to do. Or do you mean we should recalculate into UTC? Even if it were stored in UTC, it would need to be converted to local, and then to floating for the actual date handling, as we'd need the local numbers before doing any datetime math/truncation for the holiday checks. I have my reservations about Koha::DateUtils... I don't think enough thought has been put into its implementation. Besides, it wouldn't work in this case. Jonathan, the main problem at the moment is that Koha::DateUtils produces a valid local time, so it uses the local timezone. During the $calendar->days_between(), a day is added to this local datetime to produce another valid local time. However, the act of truncating that valid local time creates an invalid local time. So Koha::DateUtils isn't going to know there's a problem until it's too late. You could implement your own wrapper around DateTime, but I think that's getting excessive. We just need to be thoughtful and careful in our handling of dates and time. -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16376 --- Comment #14 from David Cook <dcook@prosentient.com.au> --- (In reply to Jonathan Druart from comment #12)
Moreover `grep time_zone Koha/Calendar.pm` will return another occurrence where the fix should be applied too.
Interesting. I think I avoided that the first time because $calendar->is_holiday() uses the following: my $ymd = $localdt->ymd('') ; if ($self->single_holidays( $ymd ) == 1 ) { return 1; } However, you are correct. sub single_holidays is weird... but definitely prone to this problem too. -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16376 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #51702|0 |1 is obsolete| | --- Comment #15 from David Cook <dcook@prosentient.com.au> --- Created attachment 51906 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=51906&action=edit Bug 16376 - Koha::Calendar->is_holiday date truncation creates fatal errors for TZ America/Santiago Using a DateTime object with a timezone of America/Santiago was causing fatal errors for Koha::Calendar->is_holiday and Koha::Calendar->exception_holidays, when the objects were truncated to an invalid local time. Using a floating zone allows us to use the same day, month, year for comparison purposes without running into the possibility of creating an invalid local time and thus a fatal software error. -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16376 Chris Cormack <chris@bigballofwax.co.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- 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=16376 Chris Cormack <chris@bigballofwax.co.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #51906|0 |1 is obsolete| | --- Comment #16 from Chris Cormack <chris@bigballofwax.co.nz> --- Created attachment 51968 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=51968&action=edit Bug 16376 - Koha::Calendar->is_holiday date truncation creates fatal errors for TZ America/Santiago Using a DateTime object with a timezone of America/Santiago was causing fatal errors for Koha::Calendar->is_holiday and Koha::Calendar->exception_holidays, when the objects were truncated to an invalid local time. Using a floating zone allows us to use the same day, month, year for comparison purposes without running into the possibility of creating an invalid local time and thus a fatal software error. Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz> -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16376 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Failed QA --- Comment #17 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- (In reply to David Cook from comment #13)
(In reply to Jonathan Druart from comment #11)
Ok you will remove the software error, but I am not sure it's the way to go.
I would certainly prefer a more thorough overhaul of Koha's DateTime handling, but I think this is good enough for now. Chile can't use the overdue functionality in Koha at all at this stage without this fix.
Currently the dates are stored in local time (so with the tz), if we want to switch to floating time zones (what we should do at some point!) I think we should recalculate all dates present in DB. An easy workaround would be to use Koha::DateUtils, which deals (at least try to) with invalid dates due du DST.
Well, we calculate and interpret the dates using local time, but they're stored without timezone in the database.
So I'm not sure what you mean by "recalculate all dates present in DB". The only difference between "local" and "floating" is the timezone. They're the same numbers, so there would be no recalculation to do. Or do you mean we should recalculate into UTC?
If we plan to support several timezone, I think we will need to recalculate the existing dates into UTC, yes.
Even if it were stored in UTC, it would need to be converted to local, and then to floating for the actual date handling, as we'd need the local numbers before doing any datetime math/truncation for the holiday checks.
Yes, floating times for calculation and tz to display/store the values. But let's have this discussion somewhere else :)
I have my reservations about Koha::DateUtils... I don't think enough thought has been put into its implementation. Besides, it wouldn't work in this case.
Well, look at the code before and after, we you will understand how it has been useful :) Looking at your patch again, it looks good. Please provide tests to highlight the issue you are trying to fix and to make sure this patch won't introduce regressions. -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16376 Mason James <mtj@kohaaloha.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |mtj@kohaaloha.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16376 OpenGeek <larry@opengeek.cl> changed: What |Removed |Added ---------------------------------------------------------------------------- Resolution|--- |FIXED Status|Failed QA |RESOLVED --- Comment #18 from OpenGeek <larry@opengeek.cl> --- This patch solve the repodted issue with overdue_process.pl using TZ=America/Santiago In fact, overdue_process.pl calculated until 1985 without errors. (Do not ask me why they have issues with that date :P ) Many Thanks David, Chris, John, and all involved !! -- 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=16376 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Resolution|FIXED |--- Status|RESOLVED |REOPENED --- Comment #19 from Tomás Cohen Arazi <tomascohen@gmail.com> --- (In reply to OpenGeek from comment #18)
This patch solve the repodted issue with overdue_process.pl using TZ=America/Santiago
In fact, overdue_process.pl calculated until 1985 without errors. (Do not ask me why they have issues with that date :P )
Many Thanks David, Chris, John, and all involved !!
Hey, this one is failed-qa, I promised a regression test which I haven't provided and Jonathan failed it! -- 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=16376 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|REOPENED |Needs Signoff -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16376 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Failed QA -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16376 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=9031 -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16376 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |kyle@bywatersolutions.com --- Comment #20 from Kyle M Hall <kyle@bywatersolutions.com> --- (In reply to Tomás Cohen Arazi from comment #19)
(In reply to OpenGeek from comment #18)
This patch solve the repodted issue with overdue_process.pl using TZ=America/Santiago
In fact, overdue_process.pl calculated until 1985 without errors. (Do not ask me why they have issues with that date :P )
Many Thanks David, Chris, John, and all involved !!
Hey, this one is failed-qa, I promised a regression test which I haven't provided and Jonathan failed it!
Ping! Any chance we can get those unit tests ; ) -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16376 --- Comment #21 from Tomás Cohen Arazi <tomascohen@gmail.com> --- I spent a couple hours on this. And now have to leave for Manuel's birthday :-D So, for the record: - I narrowed the problem down to the sub days_between. Which is called in the failing scripts. The only way to reproduce this (in order to create the unit tests) was: $ENV{TZ} = $timezone; use POSIX qw(tzset); tzset; my $tz = C4::Context->tz; my $chile_dt = DateTime->new( day => 6, month => 9, year => 2015, time_zone => $tz ); my $now_dt = DateTime->now; my $branch = $builder->build({ source => 'Branch' })->{branchcode}; my $calendar = Koha::Calendar->new( branchcode => $branch ); my $diff = $calendar->days_between($now_dt,$chile_dt); Using the other suspected functions didn't trigger the problem (exception_holidays, etc)[1]. So the problem was correctly highlighted by David, but I'm not sure what's the best place to fix it. I vote for days_between. I'll try to resume tomorrow, and will be glad to hear people's comments, specially David, that created the patch. [1] Using truncate didn't raise the problem to me, either. -- 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=16376 --- Comment #22 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 58001 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=58001&action=edit Bug 16376: (regression tests) This patch introduces a regression test for exception_holidays. This routine returns a list of datetimes to be used in date comparison and some datetimes don't exist in some timezones, so floating timezones should be used instead. To test: - Apply the patch on master - Run: $ prove t/db_dependent/Holidays.t => FAIL: The new test fails Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> -- 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=16376 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #51968|0 |1 is obsolete| | --- Comment #23 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 58002 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=58002&action=edit Bug 16376 - Koha::Calendar->is_holiday date truncation creates fatal errors for TZ America/Santiago Using a DateTime object with a timezone of America/Santiago was causing fatal errors for Koha::Calendar->is_holiday and Koha::Calendar->exception_holidays, when the objects were truncated to an invalid local time. Using a floating zone allows us to use the same day, month, year for comparison purposes without running into the possibility of creating an invalid local time and thus a fatal software error. Edit: While the changes to is_holiday and single_holiday make sense (Jonathan agrees too) I didn't manage to have them fail, because truncate is not failing in my trials, but days_between. So to me, it narrows down to have exception_holiday return floating tz datetime objects so it doesn't break days_between. Anyway, it is ok to push this patch, and the regression test I provide covers this scenario I'm describing. To test: - Apply the regression tests patch - Run: $ prove t/db_dependent/Holidays.t => FAIL: Unexpected error due to bad timezone/date combination - Apply this patch - Run: $ prove t/db_dependent/Holidays.t => SUCCESS: Tests pass Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16376 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Passed QA -- 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=16376 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Version|unspecified |master -- 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=16376 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to Master --- Comment #24 from Kyle M Hall <kyle@bywatersolutions.com> --- Pushed to master for 17.05, thanks David, Tomas! -- 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=16376 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to Master |Pushed to Stable CC| |katrin.fischer@bsz-bw.de --- Comment #25 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- These patches have been pushed to 16.11.x, will be in 16.11.01. -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16376 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Version|master |16.05 -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16376 Julian Maurice <julian.maurice@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |julian.maurice@biblibre.com --- Comment #26 from Julian Maurice <julian.maurice@biblibre.com> --- Pushed to 3.22.x for 3.22.14 -- 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=16376 --- Comment #27 from David Cook <dcook@prosentient.com.au> --- Hurray! Take that invalid local time for date in time zone... -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16376 --- Comment #28 from Mason James <mtj@kohaaloha.com> --- Pushed to 16.05.x, for 16.05.06 release -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org