[upstream] Datetime field shown with wrong timezone offset

For what it's worth, if I patch Drupal core with #2926508-115: Add DateTimeNormalizer+TimestampNormalizer, deprecate TimestampItemNormalizer: @DataType-level normalizers are reusable by JSON:API to enhance the core Rest API so that it includes datetime + timezone offset strings, the same issue occurs.

/entity/visit/1?_format=json
date.value = "2018-09-16T12:00:00-05:00"
expected = "2018-09-16T07:00:00-05:00"

Thank you for the timely follow-up! I appreciate it. :)

Yes, I'm using Drupal 8.6.1, JSON API 2.0-beta1 (haven't tried with 2.x-dev), not using JSON API Extras.

Another FWIW: I created a basic "Event" content type with a datetime field, this is actually easier to reproduce. When I visit the JSON API endpoint for that event node, the same issue occurs. Is this happening for anyone else?

/jsonapi/node/event/39a6caf7-6981-4de5-9cbb-f45a27a92737
date.value = "2018-09-16T12:00:00-05:00"
expected = "2018-09-16T07:00:00-05:00"

I'm digging deeper on this and what I'm currently seeing is that in DateTimeIso8601Normalizer and DateTimeNormalizer we're getting the DateTime object via $datetime->getDateTime(), which returns a DrupalDateTime object. The problem is that only the datetime string value from the database (without any timezone information) is sent to DrupalDateTime::__construct() ("2018-09-16T12:00:00"), and thus DrupalDateTime (actually DateTimePlus) doesn't have any timezone information and so it assumes the local timezone.

Somehow we need to inform this whole thing that we have a database value which is in the UTC timezone...

Okay, so yeah, the normalizers (DateTimeIso8601Normalizer and DateTimeNormalizer) are asking DateTimeIso8601 to give them a DrupalDateTime object in the correct timezone, but DateTimeIso8601 is not informing DrupalDateTime of the source timezone (in this case at least, it's "UTC", because that's how Drupal store's datetime fields).

Here's a simple patch which helps fix this in the ForwardCompatibility classes with JSON API. I'm also posting the corresponding patch which would affect #2926508: Add DateTimeNormalizer+TimestampNormalizer, deprecate TimestampItemNormalizer: @DataType-level normalizers are reusable by JSON:API, but keeping it here for now to isolate the conversation at this point.

We "could" simply patch DateTimeIso8601::getDateTime() to create a DrupalDateTime from a UTC string. That would solve this issue, but may introduce other problems... I don't have a broad enough knowledge of how everything works to feel confident about this solution.

I'm going to use these patches for a bit and stop further troubleshooting until somebody else weighs in.

Oops, I had one small typo in the last patches.

More work is needed here. It should change anything other than datetime_iso8601 fields (not any other datetime fields). Because as it's written in my patches, it breaks timestamp fields. Whoops.

I found some other issues which may be at play here.

#2716891: DateTimeIso8601::setDateTime() vs. DateTimeItem::schema(): (at least) one of them is broken: this issue works changes the storage format for datetime fields to 25 characters to allow for the timezone offset in storage, which is a really good idea, because then when it's loaded, we know something about the date (instead of assuming incorrectly that it's a local date).

#2632040: [PP-1] Add ability to select a timezone for datetime field: adds a separate column for datetime tables to store the timezone. Looks like a lot of work.

I believe the simplest solution, currently, is to append "+00:00" to the datetime value when it's loaded from the database. Is this possible somehow?

Wim, thanks for the kind words. This is a super complicated issue, because anything having to do with datetime, timezones, db storage, normalization and denormalization is a mess. Technical soup!

Here's what I came up with for the project we're working on, just so we could move forward. I'm pasting it here in case it's useful for this and the Drupal core issues. It's just a custom normalizer in a custom module which fills in the gaps for datetime field handling.

Note: there are probably issues with the date-only handling, which I didn't test, but this works pretty well for datetime fields.

Also note: in my denormalize function I return a string formatted according to how the DateTimeItem fields are stored in the database. When I returned a \DateTime object, all sorts of problems ensued. This approached seems to work better (for me, at least).

Thanks for everything!

mymodule.services.yml:

mymodule.normalizer.datetimeiso8601:
    class: \Drupal\mymodule\Normalizer\DateTimeIso8601Normalizer
    tags:
      # Higher priority than serializer.normalizer.datetimeiso8601.jsonapi.
      - { name: normalizer, priority: 21 }

mymodule/src/Normalizer/DateTimeIso8601Normalizer.php:

namespace Drupal\mymodule\Normalizer;

use Drupal\Core\Datetime\DrupalDateTime;
use Drupal\Core\TypedData\Plugin\DataType\DateTimeIso8601;
use Drupal\datetime\Plugin\Field\FieldType\DateTimeItem;
use Drupal\datetime\Plugin\Field\FieldType\DateTimeItemInterface;
use Drupal\jsonapi\ForwardCompatibility\Normalizer\DateTimeNormalizer;

/**
 * There are several problems with Drupal's datetime-field handling, how it's
 * stored, how it's normalized, how it's denormalized, etc.
 *
 * This class borrows from the similar JSON API class, which itself is a
 * forward-port of #2926508. Still, there are other issues (notably #2716891).
 *
 * I don't like the idea of a moving target when it comes to API. So I have
 * this simple class to normalize outgoing dates to UTC format (working around
 * issues with the DateTimeIso8601 class), and denormalize incoming dates so
 * they can be stored without timezone data in the database. Sigh.
 *
 * @see https://www.drupal.org/project/drupal/issues/2926508
 * @see https://www.drupal.org/project/drupal/issues/2716891
 * @see https://www.drupal.org/project/jsonapi/issues/2999438
 * @see https://www.drupal.org/project/drupal/issues/2632040
 */
class DateTimeIso8601Normalizer extends DateTimeNormalizer {

  /**
   * {@inheritdoc}
   */
  protected $supportedInterfaceOrClass = DateTimeIso8601::class;

  /**
   * {@inheritdoc}
   */
  public function normalize($datetime, $format = NULL, array $context = []) {
    // Always normalize in UTC timezone.
    $drupal_date_time = new DrupalDateTime($datetime->getCastedValue(), 'UTC');

    // Date-only fields.
    $field_item = $datetime->getParent();
    if ($field_item instanceof DateTimeItem && $field_item->getFieldDefinition()->getFieldStorageDefinition()->getSetting('datetime_type') === DateTimeItem::DATETIME_TYPE_DATE) {
      return $drupal_date_time->format('Y-m-d');
    }

    // Datetime fields.
    return $drupal_date_time->format(\DateTime::RFC3339);
  }

  /**
   * {@inheritdoc}
   */
  public function denormalize($data, $class, $format = NULL, array $context = []) {
    // Date-only fields.
    if ($context['field_type'] === 'datetime' && $context['field_definition']->getFieldStorageDefinition()->getSetting('datetime_type') === DateTimeItem::DATETIME_TYPE_DATE) {
      $this->allowedFormats = ['date-only' => 'Y-m-d'];
      return parent::denormalize($data, $class, $format, $context)
        ->setTimezone(new \DateTimeZone('UTC'))
        ->format(DateTimeItemInterface::DATE_STORAGE_FORMAT);
    }

    // Datetime fields.
    return parent::denormalize($data, $class, $format, $context)
      ->setTimezone(new \DateTimeZone('UTC'))
      ->format(DateTimeItemInterface::DATETIME_STORAGE_FORMAT);
  }

}

Great! Thanks so much, Wim!