-
Notifications
You must be signed in to change notification settings - Fork 241
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
EZP-29545 Values of DateTime Field Type should be handled in UTC only #1405
base: 2017.12
Are you sure you want to change the base?
Conversation
Do we miss a db update script ? |
ffed79b
to
b35b2b5
Compare
@gggeek currently yes, but it is almost completed. |
I also do not see any phpdoc paragraph mentioning what the timestamp is supposed to be |
@gggeek can you please point me to the part of the code you are talking about? |
No specific part. I just think that the fact that we are now storing the timestamp of midnight-in-utc-timezone should be documented clearly. Ideally both in the online docs and in the source code. To give some context: of all the ez5 api, one of the parts that I found most lacking in documentation was what are the accepted input/output values for Fields. |
@gggeek the "midnight" part was a bad case of copy&paste from Date FieldType PR's description - sorry for that. In legacy dates/datetimes were always stored as timestamps - this PR changes that those timestamps are stored with UTC to be more reliable and compatible with Platform (which does that from the start) - for example when using new stack with Legacy Bridge. I don't think that such low-level things (as what database value lays behind ezdatetime) have to be better documented and actually are out of this PR scope. |
Let me disagree with you.
|
@gggeek I get your point, but still - it is out of the scope of this PR. Nevertheless, I'm taking it into the account and do my best to improve the documentation. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@glye about docs: yes I agree. I got confused because those are from the "root PR" and I couldn't pinpoint where I could put some doc blocks here 😉 |
Script for updating DB: |
MERGE NOTE: Needs to be merged after #1401 |
JIRA issue: EZP-29545
Currently, in Legacy, the timestamp stored in the database is in the server's timezone, while in Platform it is always in UTC.
This PR modifies
eZDateTimeType
class so timestamps are converted to UTC before being stored and are converted back to Local Timezone timestamp on retrieval.This PR depends on #1401 and should be merged only after it.