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

Bug: Render Dataset datetime field adds incorrect timezone #434

Closed
fastbike opened this issue Oct 9, 2020 · 7 comments
Closed

Bug: Render Dataset datetime field adds incorrect timezone #434

fastbike opened this issue Oct 9, 2020 · 7 comments
Assignees
Labels
accepted Issue has been accepted and inserted in a future milestone bug
Milestone

Comments

@fastbike
Copy link
Contributor

fastbike commented Oct 9, 2020

I have a dataset with a DateTime field, which contains local server times. When this is rendered it has the "Z" (Zulu) timezone added to the json data.
So a value that is 2020/07/31@14:30:12.250 is rendered as 2020-07-31T14:30:12.250Z rather than the 2020-07-31T14:30:12.250+12:00 we would expect to see from the server time zone.
This means the time as rendered is actually incorrect by 12 hours !

The code that does the rendering is in the TMVCJsonDataObjectsSerializer.DataSetToJsonObject method. For DateTime fields it calls DateTimeToISOTimeStamp (in the MVCFramework.Serializer.Commons.pas unit) which calls DateToISO8601 passing through a hardcoded value of True for the second param of "InputIsUTC"
Because of this hard coded param all dates are assumed to be encoded to UTC with zero offset, so local dates are not handled correctly.

I can see a couple of ways to fix this:

  • change the storage of the dates in the database from local to UTC. Unfortunately we do not control the database.
  • change the call to DateToISO8601 to pass through False as the second param. This may break existing applications.
  • add an additional param to DateTimeToISOTimeStamp to allow InputIsUTC to be specified, with a default value of True.
    This could be specified with a set of options that can be passed through to the Serialiser call (I can see other options that could be added over time for formatting).

Let me know what you think and I can mock up some potential changes with test cases.

@danieleteti danieleteti self-assigned this Oct 9, 2020
@danieleteti danieleteti added the accepted Issue has been accepted and inserted in a future milestone label Oct 9, 2020
@danieleteti danieleteti added this to the 3.2.1-carbon milestone Oct 19, 2020
@danieleteti
Copy link
Owner

I think that the 2nd option is the more correct one. Application which don't use timezone, will not break. Those who use timezoe will be "more" correct. I think that we should test for this solution. Can you do some checks on your side?

@danieleteti
Copy link
Owner

I'd like to fix this bug before the 3.2.1-carbon release.

@fastbike
Copy link
Contributor Author

OK, I will make the fix in my app and test (this week)

@danieleteti
Copy link
Owner

News?

@danieleteti
Copy link
Owner

Please, check the proposed solution in the repo. Is works for all unit tests considering all the internal date representation as local but serialization and deserialization accept a valid UTC format. Following the specs a datetime string without UTC specification is considered as "local to the server".

@fastbike
Copy link
Contributor Author

fastbike commented Nov 5, 2020 via email

@danieleteti
Copy link
Owner

It should be solved in the repo. Thank you for your support.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
accepted Issue has been accepted and inserted in a future milestone bug
Projects
None yet
Development

No branches or pull requests

2 participants