Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Wrong calculation of local date/time in CakeTime functions #2032

Closed
ghost opened this issue Oct 11, 2013 · 6 comments
Closed

Wrong calculation of local date/time in CakeTime functions #2032

ghost opened this issue Oct 11, 2013 · 6 comments
Assignees
Milestone

Comments

@ghost
Copy link

ghost commented Oct 11, 2013

Created by Za-el, 8th Jan 2013. (originally Lighthouse ticket #3514):


What I did

Check a given date e.g. 2013-01-08 with (TimeHelper) $this->Time->isToday('08.01.2013', '+10');
For example the server has 2013-01-08 18:00:00 (GMT) and the given TimeZoneOffset is +10 (Eastern Australia).

What happened

The result is "true". The methods returns me true for the check if 2013-01-08 is today for servertime 2013-01-08 18:00:00 (GMT) + 10 hours.

What I expected to happen

The result should be "false". Because in Eastern Australia it is 2013-01-09 at this time.

My opinion

The methods has a big logical fail. Not the given date should be used for conversion with $userOffset. The correct way is to use the current timestamp "time()" for conversion with $userOffset.
This problem exists for all those check methods like isThisWeek, isThisMonth, isThisYear.

@ghost ghost assigned lorenzo Oct 11, 2013
@ghost
Copy link
Author

ghost commented Oct 11, 2013

8th Jan 2013, Jose Lorenzo Rodríguez said:


The user offset expressed as an integer is deprecated and only there to support migrating from older versions. It should be now expressed as a timezone name like 'America/New_York' or as a real DateTimeZone object.

@ghost
Copy link
Author

ghost commented Oct 11, 2013

9th Jan 2013, Za-el said:


The problem with the user offset beeing an integer should be a new ticket.
I have my doubts that you understand what I tried to describe. Not the user offset is the (only) problem with these mehtods.
The wrong parameter is used for conversion with user offset. Not the given $dateString (which I want to be compared) should be converted, but the current timestamp should be converted to local time.

This problem is present in all older versions too (from 1.2 to 2.2).

For example a quick an dirty fix:

function isToday($dateString, $userOffset = null) {
    $date = $this->fromString($dateString); // removed $userOffset
    $actualdate = $this->convert(time(), $userOffset); // use $userOffset to convert current server time to local time
    return date('Y-m-d', $date) == date('Y-m-d', $actualdate);
}

@lorenzo
Copy link
Member

lorenzo commented Oct 14, 2013

This needs to be fixed in 3.0, including a major refactor of all the Time class.

@markstory
Copy link
Member

I think switching everything to DateTime and timezones would be a good way to help address this and many other issues in timehelper.

@lorenzo
Copy link
Member

lorenzo commented Oct 15, 2013

I totally agree

@dereuromark
Copy link
Member

Looks like this is pretty much covered with the work done in 3.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants