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

CRM-21422 - Fix handling of 0 timezone offset #11273

Merged
merged 2 commits into from
Nov 14, 2017

Conversation

aydun
Copy link
Contributor

@aydun aydun commented Nov 13, 2017

Overview

Fix 'Timestamp Mismatch' warnings for servers with 0 timezone offset for UTC - eg Europe/London (when no daylight savings in effect), AND where the server timezone is different.

Before

Timestamp Mismatch warning

After

No Timestamp Mismatch warning

Technical Details

If the PR introduces noteworthy technical changes, please describe them here. Provide code snippets if necessary

Comments

Anything else you would like the reviewer to note


@eileenmcnaughton
Copy link
Contributor

eileenmcnaughton commented Nov 14, 2017

So I understand you are saying '0' is a legit offset & should be treated as such. 2 things

  1. it would be better to use === since this is a specific value we accept.
  2. your pr comment says 'AND where the server timezone is different.' - but we want warnings to show if the server timezone differs.... is the pr comment correct?

@aydun
Copy link
Contributor Author

aydun commented Nov 14, 2017

Thanks @eileenmcnaughton. Yes, a '0' offset is legit.

The specific scenario is:

  • system timezone is 'Europe/Sofia' and cannot be changed (shared hosting)
  • php.ini and Drupal are configured to 'Europe/London'
  • The offset at this time of year from 'Europe/London' to UTC is 0, but getTimeZoneOffset() returns false instead of '+00:00' causing the MySQL 'SET time_zone = ...' to be skipped in setMySQLTimeZone() so the SQL results default to 'Europe/Sofia' triggering the 'Timestamp mismatch' warning.

So:

  1. code changed
  2. I'm meaning that the change should not affect the results were the system timezone is the same as the php/Drupal one. If the system timezone is the same, and the offset is 0, it still skips setting the MySQL timezone but the times are returned correctly as a result of the system timezone and the 'Timestamp mismatch' warning is not triggered. Any better ways of expressing that welcome!

@eileenmcnaughton
Copy link
Contributor

Ok that makes sense & I think this is a logical change which should not have adverse consequences. Merging

@eileenmcnaughton eileenmcnaughton merged commit ee933b1 into civicrm:master Nov 14, 2017
sluc23 pushed a commit to ixiam/civicrm-core that referenced this pull request Jan 10, 2018
CRM-21422 - Fix handling of 0 timezone offset
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants