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

Build a "datetime + time zone" field type (LocalDateTime?) #2938

Closed
molomby opened this issue May 11, 2020 · 7 comments
Closed

Build a "datetime + time zone" field type (LocalDateTime?) #2938

molomby opened this issue May 11, 2020 · 7 comments

Comments

@molomby
Copy link
Member

molomby commented May 11, 2020

2022 April update – References here to the "current" implementation are talking about Keystone 5 DateTime field type, not the Keystone 6 timestamp field type which is much more correct. Regardless, a field that stores a point in time (timestamp) and the time zone in which it should be rendered would still be useful. See #7327 for a more up to date discussion.


As far as I can tell, the current DateTime type was built to solve the "always render as entered" problem:

If an event is planned for May 2nd 2020 at 6pm in Sydney Australia, it should always display as 6pm May 2nd, 2020; no matter where the viewer is located around the world / what time zone they're in, what offset they have.

Where DateTime falls down is that it relies on the user knowing the offset of the time zone the event will be in, not just now but, at the time the event occurs. Sure, if I want to create the this event for May, I can enter May 2nd at 6pm. But, if I'm planning ahead, I can't just create another event for Oct 5th at 6pm; Oct 5th is the day after daylight savings kicks in and, if I don't realise that, the UTC date stored (once #2936 is fixed) will be for 7pm not 6pm.

This is the same problem you'd have if you were creating an event from another time zone overseas. Arguably, the OS case is not as insidious though since you'd hopefully already realise you had to set correct the offset explicitly. (You'd still need to know what the correct offset was though.)

Interestingly, the way the DateTime type is built, this behaviour often doesn't cause problems. Even though entering Oct 5th at 6pm in Sydney now will store the wrong UTC value (2020-10-05T08:00:00z) it'll also store the wrong offset. When read back out by the field these errors cancel out.

A better approach to the original problem is to store the time zone, not an offset. Eg. Australia/Sydney rather than +10 or +11. This would allow Keystone to store correct UTC values while letting the user select from a friendly list, rather than know the correct offsets for their target time zones on the specific date.

Implementation

I think, for this to work, the field type would expose 3 field:

  • The timestamp in UTC; equivalent to the JS date object (eg. 2020-10-05T07:00:00z)
  • The name of the time zone in which the implicit event is occurring (eg. Australia/Sydney)
  • The date time of the event, in the time zone, without offset (eg. 2020-10-05T18:00:00)
{
	myDate_utc: String
	myDate_timezone: String
	myDate_local: String
}

Note that all field could be read but, when creating or updating the value, _local and _utc couldn't be set at the same time. I imagine, usually, the _timezone and _local values would be set (rather than _utc) as they wouldn't require any time zone manipulation on the client.

In terms of storage, only the _utc and _timezone values would be needed. Sorting and comparison filters (ie. myDate_lt, myDate_gte, etc.) would be applied against the stored UTC value. I suppose the comparison filters could accept an ISO date string either with or without an offset (in which case they be assumed to be in the time zone stored).

I'm hoping that, internally, this field type would leverage the TimeZone (#2937) and DateTimeUtc field types. They're both useful on their own but packaging them together like this will allow a nicer experience for the Admin UI.

I think that, once LocalDateTime type replaces DateTime as the "always render as entered" solution, the current DateTime should be deprecated and DateTimeUtc moved to core.

@molomby
Copy link
Member Author

molomby commented May 12, 2020

I'm hoping that, internally, this field type would leverage the TimeZone (#2937) and DateTimeUtc field types. They're both useful on their own but packaging them together like this will allow a nicer experience for the Admin UI.

Just to expanding on this -- given that we'd (probably?) only want to store the date time (in UTC) and the time zone name, it would be possible to just have two separate fields (the TimeZone (#2937) and DateTimeUtc) and get somewhat similar functionality. Actually editing or consuming the values in this scenario puts a much greater onus on the user though. It removes the ability to have the _local field -- a date time without the time zone.

Eg. if you're in San Fran and setting up an event in Sydney for Monday, Oct 5th at 6pm, you'd need to:

  • Select the Australia/Sydney time zone
  • Enter the date time with a correct offset, so either the event time...
    • In Sydney time: 2020-10-05T18:00:00+11:00
    • In San Fran time: 2020-10-05T00:00:00-07:00 or
    • In UTC time: 2020-10-05T07:00:00z

None of these are "easy to know".

If we create a specialised field for this use case the user can enter the values in a much more intuitive way:

  • Select the Australia/Sydney time zone
  • Enter the date time without an offset: 2020-10-05T18:00:00, or literally just Oct 5th at 6pm

Easy! Likewise, when reading the value, having the _local time pre-calculated may be useful in some cases.

@VinayaSathyanarayana
Copy link
Contributor

To add to what @molomby mentioned

  • Date Time Data Entry : Capture the DateTime+TimeZone (the default should be user / browser time zone)

  • Storage in DB: as UTC

  • Display: Convert the UTC Date TimeStamp to User Timezone

Notes:

  • We should capture the User TimeZone of the User as part of UserProfile and use that to display

  • If the UserTime zone is not set, use the timezone from the browser

  • If UserTimeZone is set, then it should over-ride the browser timezone.

@molomby
Copy link
Member Author

molomby commented May 22, 2020

@VinayaSathyanarayana, as far as I'm aware, Keystone doesn't make any assumptions about users so there's not really a built in concept of a UserProfile that we can add to. Am I missing something?

@VinayaSathyanarayana
Copy link
Contributor

@molomby While KeystoneJs does not current have the concept of userProfile, we should consider having it and adding userTimeZone to facilitate displaying the date+time correctly based on the timezone.

@molomby
Copy link
Member Author

molomby commented Jul 13, 2020

The Temporal proposal is relevant to this.

Date has been a long-standing pain point in ECMAScript. This is a proposal for Temporal, a global Object that acts as a top-level namespace [..], that brings a modern date/time API to the ECMAScript language.

@molomby
Copy link
Member Author

molomby commented Sep 23, 2020

See also the ECMAScript Internationalization API (Node.js docs).

@stale
Copy link

stale bot commented Jan 23, 2021

It looks like there hasn't been any activity here in over 6 months. Sorry about that! We've flagged this issue for special attention. It wil be manually reviewed by maintainers, not automatically closed. If you have any additional information please leave us a comment. It really helps! Thank you for you contribution. :)

@stale stale bot added the needs-review label Jan 23, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants