Daterange handler #2

[jonathanjfshaw]

Second attempt at daterange driver, allowing for named properties.

[jonathanjfshaw]

Addresses #160

[pfrenssen]

Which use case do you have for passing the values as a numeric array?

[jonathanjfshaw]

The most natural syntax seemed to me to be something similar to what we do for links:

    Given event content:
      | title                     | dates                                                              | 
      | Sweet Palm         | 2005-12-30 00:00:00 - 2005-12-30 04:00:00 | 

[pfrenssen]

I'm not sure that this will work as expected in all use cases. For example this wouldn't work with time formats like "December 1, 1987" or "now +1 hour".

I'm not sure if it is even the scope of DrupalDriver to "fix" bad input. My gut feeling says that if the time format is not in the expected format we should throw an exception here. It seems more like the responsibility of the API consumer (e.g. DrupalExtension) to make sure the data is in the correct format.

But then again, DrupalDriver can realistically be expected to be able to deal with all kinds of date formats that are used by Drupal versions 6-8 (and future versions). So maybe it is a good idea to support multiple time formats here.

How about something like:

foreach (['value', 'end_value'] as $key) {
  if (!empty($value[$key])) {
    $time = strtotime($value[$key]);
    if ($time === FALSE) {
      throw new \LogicException();
    }
    $value[$key] = $time;
  }
}

[jonathanjfshaw]

this wouldn't work with time formats like "December 1, 1987"

that's a problem with the D8 datetime handler too;

or "now +1 hour".

relative datetime support was recently added to the D8 handler; I didn't add it to the daterange handler as I was being quick and dirty.

I'm not sure if it is even the scope of DrupalDriver to "fix" bad input.

Yes, there are fundamental questions about how liberal the driver should be with its inputs. It comes up with the entity reference handler where we allow reference by labels, and it comes up with the question of whether we should allow specifying labels as well as machine names in some other contexts.

The best context for discussing that is #155, the new plugin system. We need to decide if the Drupal driver provides basic plugins that are extended and overridden by more helpful human-friendly plugins from the Drupal extension, or if its just simpler to have only one set of human-friendly plugins that live in in the driver.

My suggestion is

1. to accept this PR as is, as it's not worse than the datetime handler and better than the nothing we currently have for people who are massaging their inputs in custom code.
2. answer the fundamental question of liberal inputs in Entity & Field plugins #155
3. start a new issue to discuss the ideal datetime logic we want to have that applies (conceptually and DRY) to both datetime and daterange fields.

[ericgsmith]

What about extending DateTimeHandler so that the relative date handling is shared?

E.g something like:

    foreach ($values as $value) {
      // Allow date ranges properties to be specified either explicitly,
      // or implicitly by array position.
      if (!isset($value['value']) && isset($value[0])) {
        $value['value'] = $value[0];
      }
      if (!isset($value['end_value'])) {
        if (isset($value[1])) {
          $value['end_value'] = $value[1];
        }
      }

      // Filter out NULL values to allow end value to be optional.
      $return[] = parent::expand(array_filter($value));
    }

[ericgsmith]

I've given that approach a quick test ericgsmith@f59eecb and works for me setting relative dates.

Appreciate this is a old MR - but given the suggested scope didn't get merge, would be keen to get feedback on that approach.

[jhedstrom]

@pfrenssen do the 3 steps @jonathanjfshaw outlined above work for you? If so, I (or you) can merge this.