The way we are dealing with dates & their different formats currently needs be looked at and improved upon.

Following comments were made by Wim Leers at #2995140: External Entities 2.x:

  1. RE: // TODO: What about dates and their original format? I would strongly recommend to look at the date format choices that were made in CR JSON API now (de)normalizes Timestamp fields to/from ISO timestamps, not UNIX timestamps, which were based on those in #2926508: Add DateTimeNormalizer+TimestampNormalizer, deprecate TimestampItemNormalizer: @DataType-level normalizers are reusable by JSON:API. Basically: the formats that are used most widely in internet standards!
  2. Adding to the previous point: you can continue to send the "as stored by Drupal" data. But your TODO points to the underlying problem doing that: integers are integers, strings are strings, but booleans can be represented in multiple ways, and there are even more possible representations with date/time! What you actually want, is the ability to configure transformations. The only module that does this to some extent as far as I know is https://www.drupal.org/project/jsonapi_extras. If you want to consider doing this too, then \Drupal\jsonapi_extras\Plugin\DateTimeEnhancerBase is the best starting point for inspiration.
  3. Also adding to this: instead of points 1/2, wouldn't it be simpler to require the storage client to perform the necessary transformations? Then you could demand that timestamps are always represented in RFC3339 format.

Comments

rp7 created an issue. See original summary.

rp7’s picture

Title: Date handling » Improve date handling
Schoonzie’s picture

Attached is a patch that fixes some timezone issues for us. Basically the display of dates was off by the timezone of the site, since the field value is assumed to be in UTC. I don't think this is a solution that will work for all use cases but might help someone else.

hanno’s picture

Status: Active » Needs review
hanno’s picture

Issue summary: View changes
hanno’s picture

Patch should fix UTC. Still open is RFC3339 vs ISO 8601 vs other date formats

hanno’s picture

Issue summary: View changes
clemens.tolboom’s picture

Issue summary: View changes
Status: Needs review » Needs work

Fixed link to CR https://www.drupal.org/node/2982678

And needs work

clemens.tolboom’s picture

I set to Needs work because of

Patch should fix UTC. Still open is RFC3339 vs ISO 8601 vs other date formats

But we @Hanno and @clemens.tolboom decided to commit current patch and keep issue open.

@Schoonzie how/what did you test against?

Schoonzie’s picture

I tested using a custom StorageClient plugin for Elasticsearch. Elasticsearch returns dates in ISO 8601 format.

The problem that my patch fixes is that the timezone offset of the site is applied twice. Once when it's converted from the source and then once again when the field is rendered (since Drupal assumes the data is UTC).

So if my site has a timezone of +10 and I had a date like this in Elasticsearch response "2019-05-05T14:00:00.000+00:00" it would end up being shown as "2019-05-06 10:00am", instead of "2019-05-06 12:00am" (midnight) because it had applied the +10 twice.

guignonv’s picture

I'm digging up an old thread but maybe it could be marked as "closed (work as designed)".
Whatever date format that come from the storage client will need to be turned into something Drupal manages, that's a point. But I don't think we really should do something special to manage what format comes from the storage client from the storage client perspective itself. I believe the current philosophy of External Entities is to have a storage client that manages to extract raw data from a source and then, use a field mapper to adapt that raw data into something Drupal can handle. Therefore, managing date and time format should be the job of a field mapper. And that's it for that issue.

However, this discussion brings be to another one that should fit into a different topic: being able to combine field mappers depending on the nature of the fields we need to map (ie. having a dedicated date field mapper that could be used in combination with other field mappers).

UPDATE: see v3 #3455362: Using multiple field mappers at once