neos-ui icon indicating copy to clipboard operation
neos-ui copied to clipboard

BUGFIX: Never fall back to UTC when picking date

Open gerhard-boden opened this issue 5 years ago • 15 comments

Since our default date format for DateTime properties is Y-m-d H:i:s.u and we also persist the time we must not fall back to ``UTC` within the date picker per default.

Example: If the persisted time is 00:00:00.000000 and convert to UTC this will result in yesterday's date at 22:00:00.000000 (depending on timezone) being selected in the date picker.

When choosing a new date however, the problem is the other way around. Now tomorrow's date would be chosen if a date is selected.

This issue can be hard to reproduce, since per default time for a selected day could be 02:00:00.000000 (depending on timezone and UTC). So even if the conversion to UTC happens in the frontend date picker the problem would not occur.

fixes #2467

gerhard-boden avatar Oct 02 '19 14:10 gerhard-boden

So this basically reverts a lot of stuff done in: https://github.com/neos/neos-ui/pull/2180 (fixed https://github.com/neos/neos-ui/issues/2147)

I couldn't reproduce the old issue, maybe some changes on the server side happened in the mean time when it comes to persisting / converting dates?

Would find it really important if @hlubek could do a review on this one, since he has a better understanding of the initial issue he fixed about a year ago.

The last thing I want is to introduce a regression that re-introduces old issues.

gerhard-boden avatar Oct 02 '19 16:10 gerhard-boden

If we want to use the server timezone and not the users local time zone, this could be done with the displayTimeZone but might end up in more changes in various places and is rather a behaviour breaking feature than a hotfix like this one.

gerhard-boden avatar Oct 02 '19 16:10 gerhard-boden

Maybe something in the conversion chain to the stored date changed. The issue sounds like the one we had a year ago. For example a date chosen at 23:00 in CET ended up on the wrong day after being persisted. IMHO UTC is still the way to go (timezones don’t matter for a date only value) and the server-side has to respect the conversion. It could still be wrong to „cast“ the value to a display timezone, since the date part could be shifted. So we basically miss a type that discards time information here, which makes things complicated.

hlubek avatar Oct 02 '19 17:10 hlubek

@hlubek do you have any ideas of what we could do about it?

dimaip avatar Oct 03 '19 15:10 dimaip

IMO the user's locale should have absolutely nothing to do with dates editing, it should only be defined by server's locale. E.g. we have editors working from different timezones, but the website is always rendered just in one timezone. The thing that the editor inputs should be exactly what is rendered on the site. So I would expect the UI to send something like 2018-10-17T00:01:23.000000 to the server, and it would figure out in which timezone to persist it.

dimaip avatar Oct 14 '19 14:10 dimaip

For the protocol: We had some lengthy discussions about this topic at the sprint with the outcome, that there are still many things we do not consider currently when it comes to time zone handling. And in most cases the current behavior is not self explaining.

One proposal I personally liked was to show the active time zone underneath all sorts of date / time related inspectors. We could not decide on any immediate next steps, but the new features are out of scope for a bugfix.

This PR mitigates the symptoms of the current problem, but will not tackle all the issues where problems like this come from.

gerhard-boden avatar Oct 18 '19 15:10 gerhard-boden

@gerhard-boden Thanks for the summary. What means the discussion to the PR. Go for it and improve later, or wait?

markusguenther avatar Nov 25 '19 23:11 markusguenther

@gerhard-boden Thanks for the summary. What means the discussion to the PR. Go for it and improve later, or wait?

Hi @markusguenther 😊 I rather see this PR as a hotfix, it could fix the immediate issue in many cases, but will not take care of many general shortcomings that Neos has in combination with the complex topic of time zone handling.

In case you have a related issue, you could test this PR and give feedback if it fixed the problem. Since I didn't receive any approvals for this (quite the opposite really) I did not go any further with this solution.

gerhard-boden avatar Nov 30 '19 14:11 gerhard-boden

As it is a bugfix we could luckily care for that after the feature freeze ;)

markusguenther avatar Nov 30 '19 17:11 markusguenther

@kitsunet @drillsergeant Might this be useful for "your timezone problem"?

kdambekalns avatar Nov 11 '20 07:11 kdambekalns

@kdambekalns , I think this could help.

But I also just got the budget for the implementation of the "timezone-aware datepicker". I am in contact with @kitsunet about the implementation of that feature. I think the final solution should fix the problem - mabye in the way like in this PR - but also offer to see/modify the timezone of dates.

DrillSergeant avatar Nov 11 '20 13:11 DrillSergeant

Could maybe the Intl.DateTimeFormat solve part of the issue? The API has a pretty good browser support right now. Was just a thought I had today :D

markusguenther avatar Feb 12 '21 09:02 markusguenther

Getting back to this… @kitsunet and @DrillSergeant might have solved this by now? Is this still an issue that needs to be fixed?

kdambekalns avatar Oct 13 '21 07:10 kdambekalns

As this is pretty old, I will rebase this to 5.3

markusguenther avatar Oct 13 '21 08:10 markusguenther

@kitsunet developed for our usecase the package https://github.com/flownative/neos-extendedtimeeditor I am not sure if this PR has an influence on that.

DrillSergeant avatar Oct 14 '21 09:10 DrillSergeant