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

Unexpected behavior of Piwik\Date and Piwik\Period\Range when setting timezone #9745

Open
adaqus opened this issue Feb 10, 2016 · 2 comments
Labels
Bug For errors / faults / flaws / inconsistencies etc.

Comments

@adaqus
Copy link

adaqus commented Feb 10, 2016

Piwik\Date

Let's say we want to create new Date instance with custom timezone:

$date = Date::factory('2016-01-26', 'America/New_York');

In this factory it is assumed that input date is always in UTC, so I would expect following behavior:

  • $date->getDatetime() should return value from custom timezone '2016-01-25 19:00:00' and it works this way,
  • $date->getDateStartUTC() should return original UTC value '2016-01-26 00:00:00', it returns '2016-01-25 00:00:00' instead.

Piwik\Period\Range

Let's create new Range period instance:

$period = new Range('day', '2016-01-26,2016-01-27', 'America/New_York');

This is how this period behaves:

  • $period->getDateStart()->getDatetime() should return '2016-01-25 19:00:00', it returns '2016-01-26 00:00:00',
  • $period->getDateStart()->getDateStartUTC() should return '2016-01-26 00:00:00' and it does.
@tsteur
Copy link
Member

tsteur commented Feb 10, 2016

That looks like a bug indeed and it could cause problems as it seems to be used around Archiving etc. In this line https://github.com/piwik/piwik/blob/2.16.0/core/Date.php#L190 it applies the timezone so it doesn't return the start of the day in UTC but in the given timezone.

The description of the method says:

Returns the start of the day of the current timestamp in UTC.

but it looks like it definitely doesn't. However, I wonder, if maybe only the name of the method and the description is wrong so far and the behaviour is maybe expected? Really surprised that the timezone is applied there when it specifically says UTC. @mattab any ideas?

BTW:
We should make Period\Factory public API and remove API from __constructor maybe to force creation of Periods via the factory? Not sure!

@mattab mattab added this to the 2.16.x (LTS) milestone Mar 31, 2016
@mattab mattab added the Bug For errors / faults / flaws / inconsistencies etc. label Mar 31, 2016
@mattab mattab modified the milestones: 2.16.x (LTS), Mid term Apr 1, 2016
@piymis
Copy link

piymis commented Dec 21, 2017

Hi, if this issue still exists I would like to work on it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug For errors / faults / flaws / inconsistencies etc.
Projects
None yet
Development

No branches or pull requests

4 participants