Bug 8966 - Koha::Calendar::is_holiday truncates the date
Summary: Koha::Calendar::is_holiday truncates the date
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Circulation (show other bugs)
Version: 3.8
Hardware: All All
: P5 - low critical (vote)
Assignee: Maxime Pelletier
QA Contact: Paul Poulain
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-10-24 21:26 UTC by Maxime Pelletier
Modified: 2019-06-27 09:24 UTC (History)
6 users (show)

See Also:
Change sponsored?: Sponsored
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:


Attachments
patch to fix the bug and a test (1.88 KB, patch)
2012-10-24 21:31 UTC, Maxime Pelletier
Details | Diff | Splinter Review
Patch with a comment in the test to see this bug (1.89 KB, patch)
2012-10-24 21:35 UTC, Maxime Pelletier
Details | Diff | Splinter Review
Bug 8966 Koha::Calendar::is_holiday truncates the date (1.94 KB, patch)
2012-10-26 00:32 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Maxime Pelletier 2012-10-24 21:26:31 UTC
Calling the function is_holiday on a date truncate that dates because it does a truncate directly on it instead of using a local clone of the date.
Comment 1 Maxime Pelletier 2012-10-24 21:31:11 UTC Comment hidden (obsolete)
Comment 2 Maxime Pelletier 2012-10-24 21:35:28 UTC Comment hidden (obsolete)
Comment 3 Maxime Pelletier 2012-10-25 14:29:40 UTC
Forgot to mention that this is sponsored by CCSR.
Comment 4 Tomás Cohen Arazi 2012-10-26 00:32:04 UTC
Created attachment 13068 [details] [review]
Bug 8966 Koha::Calendar::is_holiday truncates the date

* Create a local copy of the date instead of calling truncate directly on the date
* Add a test to properly test that the issue is fixed

Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
Comment 5 Paul Poulain 2012-10-29 17:28:04 UTC
QA comment:
 * thanks for the updated Calendar.t, it helped me understand what the patch was fixing
 * small patch, add a test ++
 * passes koha-qa.pl
 * prove t/Calendar.t => All tests successful.

passed QA
Comment 6 Paul Poulain 2012-10-29 17:28:22 UTC
Patch pushed to master
Comment 7 Chris Cormack 2012-10-30 21:50:04 UTC
Pushed to 3.8.x, will be in 3.8.7
Comment 8 David Cook 2016-05-04 02:40:25 UTC
I just noticed that the Calendar.t uses a DateTime object with a timezone of Europe/London. 

If we change the test to include the following,

my $dt = DateTime->new(
    month => 9,
    year => 2015,
    day => 6,
    hour => 12,
    minute => 00,
    second => 00,
    time_zone => 'America/Santiago',
);
my $calendar = Koha::Calendar->new(branchcode => "MAIN");
$calendar->is_holiday($dt);

We'll get a fatal error with the following message:

Invalid local time for date in time zone: America/Santiago

See https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16376