New DateRangeHandler is unreliable, causing failures

[pfrenssen]

In #161 a DateRangeHandler was introduced, but it is not working as expected because it assumes that the start date will have the key 0 and the end date will have the key 1. We are using Behat DrupalExtension for testing our fields, and it is passing the correct compound field property keys value and end_value.

Ref. RawDrupalContext::parseEntityFields().

[pfrenssen]

diff --git a/src/Drupal/Driver/Fields/Drupal8/DaterangeHandler.php b/src/Drupal/Driver/Fields/Drupal8/DaterangeHandler.php
index 8285d2a..6920fa3 100644
--- a/src/Drupal/Driver/Fields/Drupal8/DaterangeHandler.php
+++ b/src/Drupal/Driver/Fields/Drupal8/DaterangeHandler.php
@@ -13,8 +13,8 @@ class DaterangeHandler extends AbstractHandler {
   public function expand($values) {
     $return = [];
     foreach ($values as $value) {
-      $start = str_replace(' ', 'T', $value[0]);
-      $end = str_replace(' ', 'T', $value[1]);
+      $start = str_replace(' ', 'T', $value['value']);
+      $end = str_replace(' ', 'T', $value['end_value']);
       $return[] = [
         'value' => $start,
         'end_value' => $end,

This fixes it for us, needs work.

[brummbar]

The code introduced with said PR breaks backwards compatibility with any test that looks like the following:

When I am viewing a "post" content:
      | title                | Post title          |
      | body                 | PLACEHOLDER BODY    |
      | field_date:value     | 2015-02-10 17:45:00 |
      | field_date:end_value | 2015-03-10 17:45:00 |

[jhedstrom]

Sorry about that. Even the simplest looking things probably need corresponding tests in the Drupal Extension to prevent this.

[jonathanjfshaw]

Sorry about that. New PR #167.

I'm a bit surprised because I thought no one out there could be already successfully using a daterange field if there was not a handler, as the "T" substitution in the date wouldn't be happening.

[pfrenssen]

I'm sorry for the lack of context in the previous messages, I was in a big rush to get a release out and didn't have much time for providing more detail.

We are using date range fields successfully in DrupalExtension through the existing support for compound field properties. Since we are using "client-friendly" language in our Behat scenarios (e.g. "January 1, 2017" or "now +2 days") we are not using the T substitution. I had a quick look at how this is implemented in Drupal and it seems to me that any valid PHP time string should work: DateRangeFieldItemList::processDefaultValue() uses DrupalDateTime which accepts two arguments: the $time and the $timezone. The documentation mentions that it is possible to pass the timezone as a "T" value in the $time argument:

  /**
   * @param string $time
   *   A date/input_time_adjusted string. Defaults to 'now'.
   * @param mixed $timezone
   *   PHP DateTimeZone object, string or NULL allowed.
   *   Defaults to NULL. Note that the $timezone parameter and the current
   *   timezone are ignored when the $time parameter either is a UNIX timestamp
   *   (e.g. @946684800) or specifies a timezone
   *   (e.g. 2010-01-28T15:00:00+02:00).
   *   @see http://php.net/manual/en/datetime.construct.php
   */

Here is an example of how we are using the date ranges currently in DrupalExtension:

We create an "event" (which is a node type) that has a daterange field with a start date and end date (ref. https://github.com/ec-europa/joinup-dev/blob/develop/tests/features/collection/collection.event.feature#L14):

    And event content:
      | title                     | collection | start date   | end date            | created    | state     | author         |
      | Sweet Palm                | Fairy Tail | now -1 years | now -1 years +1 day | now -4 day | validated | katerpillar    |

We are using the aliases start date and end date instead of field_event_date:value and field_event_date:end_value because in Behat using a client friendly ubiquitous language is encouraged. We are using a @BeforeNodeCreate hook to replace the client friendly aliases with the actual field names (ref. https://github.com/ec-europa/joinup-dev/blob/develop/web/profiles/joinup/joinup.behat.inc#L551).

[jonathanjfshaw]

Datetime fields look similar to DateRange fields in this regard. If Daterange's worked OOTB with FieldHandlerBase without a specific handler, I wonder why we need a handler for datetime fields. Maybe we don't, maybe it was just copy-paste from D7. Until recently (when relative date support was added) the datetime handler simply replaced the space between date and time with a "T".

…

On 12 February 2018 at 14:01, Pieter Frenssen ***@***.***> wrote: I'm sorry for the lack of context in the previous messages, I was in a big rush to get a release out and didn't have much time for providing more detail. We are using date range fields successfully in DrupalExtension through the existing support for compound field properties. Since we are using "client-friendly" language in our Behat scenarios (e.g. "January 1, 2017" or "now +2 days") we are not using the T substitution. I had a quick look at how this is implemented in Drupal and it seems to me that any valid PHP time string should work: DateRangeFieldItemList::processDefaultValue() uses DrupalDateTime which accepts two arguments: the $time and the $timezone. The documentation mentions that it is possible to pass the timezone as a "T" value in the $time argument: /** * @param string $time * A date/input_time_adjusted string. Defaults to 'now'. * @param mixed $timezone * PHP DateTimeZone object, string or NULL allowed. * Defaults to NULL. Note that the $timezone parameter and the current * timezone are ignored when the $time parameter either is a UNIX timestamp * (e.g. @946684800) or specifies a timezone * (e.g. 2010-01-28T15:00:00+02:00). * @see http://php.net/manual/en/datetime.construct.php */ Here is an example of how we are using the date ranges currently in DrupalExtension: We create an "event" (which is a node type) that has a daterange field with a start date and end date (ref. https://github.com/ec-europa/ joinup-dev/blob/develop/tests/features/collection/ collection.event.feature#L14): And event content: | title | collection | start date | end date | created | state | author | | Sweet Palm | Fairy Tail | now -1 years | now -1 years +1 day | now -4 day | validated | katerpillar | We are using the aliases start date and end date instead of field_event_date:value and field_event_date:end_value because in Behat using a client friendly ubiquitous language is encouraged. We are using a @BeforeNodeCreate hook to replace the client friendly aliases with the actual field names (ref. https://github.com/ec-europa/ joinup-dev/blob/develop/web/profiles/joinup/joinup.behat.inc#L551). — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#165 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ADWYQTBRwp95KMRIOFbL882slRSEkbasks5tUERJgaJpZM4R8WTY> .

[pfrenssen]

I just discovered a missing piece of the puzzle, we have this code that transforms the human readable date format into a UNIX timestamp (ref https://github.com/ec-europa/joinup-dev/blob/develop/web/modules/custom/joinup_event/joinup_event.behat.inc#L47):

  public static function massageEventFieldsBeforeNodeCreate(BeforeNodeCreateScope $scope) {
    $node = $scope->getEntity();

    if ($node->type !== 'event') {
      return;
    }

    foreach (['start date', 'end date'] as $field) {
      if (isset($node->$field)) {
        $date = strtotime($node->$field);

        if ($date === FALSE) {
          throw new \Exception(sprintf('Invalid format for date specified: %s', $node->$field));
        }

        $node->$field = $date;
      }
    }
  }

So in fact we are not passing the human readable data to the field API, but UNIX timestamps.

[jonathanjfshaw]

That explains a lot :)

…

On 12 February 2018 at 21:34, Pieter Frenssen ***@***.***> wrote: I just discovered a missing piece of the puzzle, we have this code that transforms the human readable date format into a UNIX timestamp (ref https://github.com/ec-europa/joinup-dev/blob/develop/web/ modules/custom/joinup_event/joinup_event.behat.inc#L47): public static function massageEventFieldsBeforeNodeCreate(BeforeNodeCreateScope $scope) { $node = $scope->getEntity(); if ($node->type !== 'event') { return; } foreach (['start date', 'end date'] as $field) { if (isset($node->$field)) { $date = strtotime($node->$field); if ($date === FALSE) { throw new \Exception(sprintf('Invalid format for date specified: %s', $node->$field)); } $node->$field = $date; } } } So in fact we are not passing the human readable data to the field API, but UNIX timestamps. — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#165 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ADWYQSGvjcP0fdZjoz5P7SjPMZLiynQWks5tUK5pgaJpZM4R8WTY> .