Fixed timezone and normalised ISO handling

A few observations:
- ISO says “T” is optional (https://en.wikipedia.org/wiki/ISO_8601#cite_note-21),
- WHATWG says in the HTML5 spec that it’s optional (https://html.spec.whatwg.org/multipage/infrastructure.html#local-dates-and-times)
- W3C says it’s reqiured in 1997 (https://www.w3.org/TR/NOTE-datetime), but then later says it’s optional in its HTML5 spec (https://www.w3.org/TR/html5/infrastructure.html#floating-dates-and-times).
- Chrome doesn’t parse values with whitespace separators (requires "T")
- DataObject DBDatetime values and database columns use whitespace separators (and will have many devs relying on this format)
- MySQL only supports whitespace separators (https://dev.mysql.com/doc/refman/5.7/en/datetime.html)
- SQLite can parse both ways (https://sqlite.org/lang_datefunc.html)

So the goal here is to retain ORM/database compatibility with 3.x (whitespace separator),
while exposing "T" separators to the browser in HTML5 mode.

Regarding timezones, this fixes a regression where setValue() would not actually
apply the timezone (last $value assignment is ineffective now that sub fields are removed).

docs/en/04_Changelogs/4.0.0.md

@@ -1400,10 +1400,6 @@ The below methods have been added or had their functionality updated to `DBDate`
 
 * Added `DBTime::FormatFromSettings`
 
-`DBDatetim` specific changes:
-
-* Changed `ISO_DATETIME`'s pattern from '`y-MM-dd HH:mm:ss`' to '`y-MM-dd\'T\'HH:mm:ss`'
-
 #### <a name="overview-orm-removed"></a>ORM Removed API
 
 * `DataObject::db` removed and replaced with `DataObjectSchema::fieldSpec` and `DataObjectSchema::fieldSpecs`
@@ -1677,8 +1673,12 @@ New `DatetimeField` methods replace `getConfig()` / `setConfig()`:
 
 The `DatetimeField` has changed behaviour:
 
-* It no longer relies on `DateField` and `TimeField`
+* It uses a combined input instead of a composite from `DateField` and `TimeField`
-* Anything related to `DateField` and `TimeField` have been removed, e.g. `DatetimeField::getDateField()`
+  Consequently, `getDateField()` and `getTimeField()` have been removed.
+* It returns [ISO 8601 normalised dates](https://html.spec.whatwg.org/multipage/infrastructure.html#local-dates-and-times)
+  by default in `Value()`, which include a "T" separator between date and time. 
+  This is required to allow HTML5 input. Either use `setHTML5(false)` to set your custom format,
+  or use `dataValue()` to retrieve a whitespace separated representation. 
 * Added `getHTML5()` / `setHTML5()`
 
 New `DateField` methods replace `getConfig()` / `setConfig()`:

src/Forms/DatetimeField.php

@@ -8,16 +8,10 @@ use SilverStripe\i18n\i18n;
 use SilverStripe\ORM\FieldType\DBDatetime;
 
 /**
- * Form field used for editing date time string
+ * Form field used for editing date time strings.
- *
+ * By default, the field handles strings in normalised ISO 8601 format,
- * # Localization
+ * for example 2017-04-26T23:59:59. The "T" separator can be replaced with a whitespace for value setting.
- *
+ * Data is passed on via {@link dataValue()} with whitespace separators.
- * See {@link DateField}
- *
- * # Configuration
- *
- * - "timezone": Set a different timezone for viewing. {@link dataValue()} will still save
- * the time in PHP's default timezone (date_default_timezone_get()), its only a view setting.
  */
 class DatetimeField extends TextField
 {
@@ -79,6 +73,9 @@ class DatetimeField extends TextField
      */
     protected $rawValue = null;
 
+    /**
+     * @inheritDoc
+     */
     protected $schemaDataType = FormField::SCHEMA_DATA_TYPE_DATETIME;
 
     /**
@@ -88,10 +85,12 @@ class DatetimeField extends TextField
      */
     protected $dateTimeOrder = '{date} {time}';
 
-    public function __construct($name, $title = null, $value = "")
+    /**
-    {
+     * Custom timezone
-        parent::__construct($name, $title, $value);
+     *
-    }
+     * @var string
+     */
+    protected $timezone = null;
 
     public function setForm($form)
     {
@@ -171,7 +170,7 @@ class DatetimeField extends TextField
     /**
      * Convert date localised in the current locale to ISO 8601 date
      *
-     * @param string $date
+     * @param string $datetime
      * @return string The formatted date, or null if not a valid date
      */
     public function localisedToISO8601($datetime)
@@ -181,6 +180,13 @@ class DatetimeField extends TextField
         }
         $fromFormatter = $this->getFormatter();
         $toFormatter = $this->getISO8601Formatter();
+
+        // Remove 'T' date and time separator before parsing (required by W3C HTML5 fields)
+        if ($this->getHTML5()) {
+            $datetime = str_replace('T', ' ', $datetime);
+        }
+
+        // Try to parse time with seconds
         $timestamp = $fromFormatter->parse($datetime);
 
         // Try to parse time without seconds, since that's a valid HTML5 submission format
@@ -204,7 +210,7 @@ class DatetimeField extends TextField
      */
     protected function getFormatter()
     {
-        if ($this->getHTML5() && $this->datetimeFormat && $this->datetimeFormat !== DBDatetime::ISO_DATETIME) {
+        if ($this->getHTML5() && $this->datetimeFormat && $this->datetimeFormat !== DBDatetime::ISO_DATETIME_NORMALISED) {
             throw new \LogicException(
                 'Please opt-out of HTML5 processing of ISO 8601 dates via setHTML5(false) if using setDatetimeFormat()'
             );
@@ -230,8 +236,9 @@ class DatetimeField extends TextField
         );
 
         if ($this->getHTML5()) {
-            // Browsers expect ISO 8601 dates, localisation is handled on the client
+            // Browsers expect ISO 8601 dates, localisation is handled on the client.
-            $formatter->setPattern(DBDatetime::ISO_DATETIME);
+            // Add 'T' date and time separator to create W3C compliant format
+            $formatter->setPattern(DBDatetime::ISO_DATETIME_NORMALISED);
         } elseif ($this->datetimeFormat) {
             // Don't invoke getDatetimeFormat() directly to avoid infinite loop
             $ok = $formatter->setPattern($this->datetimeFormat);
@@ -254,7 +261,7 @@ class DatetimeField extends TextField
     {
         if ($this->getHTML5()) {
             // Browsers expect ISO 8601 dates, localisation is handled on the client
-            $this->setDatetimeFormat(DBDatetime::ISO_DATETIME);
+            $this->setDatetimeFormat(DBDatetime::ISO_DATETIME_NORMALISED);
         }
 
         if ($this->datetimeFormat) {
@@ -295,6 +302,11 @@ class DatetimeField extends TextField
         // Build new formatter with the altered timezone
         $formatter = clone $this->getISO8601Formatter();
         $formatter->setTimeZone($timezone);
+
+        // ISO8601 date with a standard "T" separator (W3C standard).
+        // Note we omit timezone from this format, and we assume server TZ always.
+        $formatter->setPattern(DBDatetime::ISO_DATETIME_NORMALISED);
+
         return $formatter;
     }
 
@@ -312,9 +324,11 @@ class DatetimeField extends TextField
             date_default_timezone_get() // Default to server timezone
         );
         $formatter->setLenient(false);
-        // CLDR iso8601 date.
+
-        // Note we omit timezone from this format, and we assume server TZ always.
+        // Note we omit timezone from this format, and we always assume server TZ
-        $formatter->setPattern('y-MM-dd HH:mm:ss');
+        // ISO8601 date with a standard "T" separator (W3C standard).
+        $formatter->setPattern(DBDatetime::ISO_DATETIME_NORMALISED);
+
         return $formatter;
     }
 
@@ -343,6 +357,14 @@ class DatetimeField extends TextField
         // If invalid, assign for later validation failure
         $isoFormatter = $this->getISO8601Formatter();
         $timestamp = $isoFormatter->parse($value);
+
+        // Retry without "T" separator
+        if (!$timestamp) {
+            $isoFallbackFormatter = $this->getISO8601Formatter();
+            $isoFallbackFormatter->setPattern(DBDatetime::ISO_DATETIME);
+            $timestamp = $isoFallbackFormatter->parse($value);
+        }
+
         if ($timestamp === false) {
             return $this;
         }
@@ -353,11 +375,6 @@ class DatetimeField extends TextField
         // Save value
         $this->value = $value;
 
-        // Shift iso date into timezone before assignment to subfields
-        $timezoneFormatter = $this->getTimezoneFormatter();
-        if ($timezoneFormatter) {
-            $value = $timezoneFormatter->format($timestamp);
-        }
         return $this;
     }
 
@@ -366,12 +383,40 @@ class DatetimeField extends TextField
         return $this->iso8601ToLocalised($this->value);
     }
 
+    public function dataValue()
+    {
+        return $this->iso8601ToDataValue($this->value);
+    }
+
     /**
-     * Convert an ISO 8601 localised date into the format specified by the
+     * Convert an ISO 8601 localised datetime into a saveable representation.
-     * current date format.
      *
-     * @param string $date
+     * @param string $datetime
-     * @return string The formatted date, or null if not a valid date
+     * @return string The formatted date and time, or null if not a valid date and time
+     */
+    public function iso8601ToDataValue($datetime)
+    {
+        $fromFormatter = $this->getISO8601Formatter();
+        $toFormatter = $this->getFormatter();
+
+        // Set default timezone (avoid shifting data values into user timezone)
+        $toFormatter->setTimezone(date_default_timezone_get());
+
+        // Send to data object without "T" separator
+        $toFormatter->setPattern(DBDatetime::ISO_DATETIME);
+
+        $timestamp = $fromFormatter->parse($datetime);
+        if ($timestamp === false) {
+            return null;
+        }
+        return $toFormatter->format($timestamp) ?: null;
+    }
+
+    /**
+     * Convert an ISO 8601 localised datetime into the format specified by the current format.
+     *
+     * @param string $datetime
+     * @return string The formatted date and time, or null if not a valid date and time
      */
     public function iso8601ToLocalised($datetime)
     {
@@ -385,13 +430,14 @@ class DatetimeField extends TextField
         if ($timestamp === false) {
             return null;
         }
+
         return $toFormatter->format($timestamp) ?: null;
     }
 
     /**
      * Tidy up iso8601-ish date, or approximation
      *
-     * @param string $date Date in iso8601 or approximate form
+     * @param string $date Date in ISO 8601 or approximate form
      * @return string iso8601 date, or null if not valid
      */
     public function tidyISO8601($datetime)
@@ -523,7 +569,7 @@ class DatetimeField extends TextField
     }
 
     /**
-     * @param string $minDatetime
+     * @param string $minDatetime A string in ISO 8601 format
      * @return $this
      */
     public function setMinDatetime($minDatetime)
@@ -626,13 +672,6 @@ class DatetimeField extends TextField
         return $this->timezone;
     }
 
-    /**
-     * Custom timezone
-     *
-     * @var string
-     */
-    protected $timezone = null;
-
     /**
      * @param string $timezone
      * @return $this

src/ORM/FieldType/DBDatetime.php

@@ -35,9 +35,16 @@ use InvalidArgumentException;
 class DBDatetime extends DBDate implements TemplateGlobalProvider
 {
     /**
-     * Standard ISO format string for date and time in CLDR standard format
+     * Standard ISO format string for date and time in CLDR standard format,
+     * with a whitespace separating date and time (common database representation, e.g. in MySQL).
      */
-    const ISO_DATETIME = 'y-MM-dd\'T\'HH:mm:ss';
+    const ISO_DATETIME = 'y-MM-dd HH:mm:ss';
+
+    /**
+     * Standard ISO format string for date and time in CLDR standard format,
+     * with a "T" separator between date and time (W3C standard, e.g. for HTML5 datetime-local fields).
+     */
+    const ISO_DATETIME_NORMALISED = 'y-MM-dd\'T\'HH:mm:ss';
 
     /**
      * Returns the standard localised date

tests/php/Forms/DatetimeFieldTest.php

@@ -43,7 +43,7 @@ class DatetimeFieldTest extends SapphireTest
         $this->assertTrue($dateTimeField->validate($validator));
         $m = new Model();
         $form->saveInto($m);
-        $this->assertEquals('2003-03-29T23:59:38', $m->MyDatetime);
+        $this->assertEquals('2003-03-29 23:59:38', $m->MyDatetime);
     }
 
     public function testFormSaveIntoLocalised()
@@ -62,7 +62,7 @@ class DatetimeFieldTest extends SapphireTest
         $this->assertTrue($dateTimeField->validate($validator));
         $m = new Model();
         $form->saveInto($m);
-        $this->assertEquals('2003-03-29T23:59:38', $m->MyDatetime);
+        $this->assertEquals('2003-03-29 23:59:38', $m->MyDatetime);
     }
 
     public function testDataValue()
@@ -107,13 +107,21 @@ class DatetimeFieldTest extends SapphireTest
         $f = new DatetimeField('Datetime', 'Datetime');
         $f->setValue('2003-03-29 23:59:38');
         $this->assertEquals($f->dataValue(), '2003-03-29 23:59:38');
+
+        $f = new DatetimeField('Datetime', 'Datetime');
+        $f->setValue('2003-03-29T23:59:38');
+        $this->assertEquals($f->dataValue(), '2003-03-29 23:59:38', 'Normalised ISO');
     }
 
-    public function testSetValue()
+    public function testSubmittedValue()
     {
         $datetimeField = new DatetimeField('Datetime', 'Datetime');
         $datetimeField->setSubmittedValue('2003-03-29 23:00:00');
         $this->assertEquals($datetimeField->dataValue(), '2003-03-29 23:00:00');
+
+        $datetimeField = new DatetimeField('Datetime', 'Datetime');
+        $datetimeField->setSubmittedValue('2003-03-29T23:00:00');
+        $this->assertEquals($datetimeField->dataValue(), '2003-03-29 23:00:00', 'Normalised ISO');
     }
 
     public function testSetValueWithLocalised()
@@ -134,6 +142,9 @@ class DatetimeFieldTest extends SapphireTest
         $f = new DatetimeField('Datetime', 'Datetime', '2003-03-29 23:59:38');
         $this->assertTrue($f->validate(new RequiredFields()));
 
+        $f = new DatetimeField('Datetime', 'Datetime', '2003-03-29T23:59:38');
+        $this->assertTrue($f->validate(new RequiredFields()), 'Normalised ISO');
+
         $f = new DatetimeField('Datetime', 'Datetime', '2003-03-29 00:00:00');
         $this->assertTrue($f->validate(new RequiredFields()));
 
@@ -166,6 +177,11 @@ class DatetimeFieldTest extends SapphireTest
         $dateField->setMinDatetime('2009-03-31 23:00:00');
         $dateField->setValue('2008-03-31 23:00:00');
         $this->assertFalse($dateField->validate(new RequiredFields()), 'Date below min datetime');
+
+        $dateField = new DatetimeField('Datetime');
+        $dateField->setMinDatetime('2009-03-31T23:00:00');
+        $dateField->setValue('2009-03-31T23:00:01');
+        $this->assertTrue($dateField->validate(new RequiredFields()), 'Time above min datetime with normalised ISO');
     }
 
     public function testValidateMinDateStrtotime()
@@ -215,9 +231,14 @@ class DatetimeFieldTest extends SapphireTest
         $f->setMaxDatetime('2009-03-31 23:00:00');
         $f->setValue('2010-03-31 23:00:00');
         $this->assertFalse($f->validate(new RequiredFields()), 'Date above max datetime');
+
+        $f = new DatetimeField('Datetime');
+        $f->setMaxDatetime('2009-03-31T23:00:00');
+        $f->setValue('2009-03-31T22:00:00');
+        $this->assertTrue($f->validate(new RequiredFields()), 'Time below max datetime with normalised ISO');
     }
 
-    public function testTimezoneSetLocalised()
+    public function testTimezoneSetValueLocalised()
     {
         date_default_timezone_set('Europe/Berlin');
         // Berlin and Auckland have 12h time difference in northern hemisphere winter
@@ -230,7 +251,7 @@ class DatetimeFieldTest extends SapphireTest
         $datetimeField->setTimezone('Pacific/Auckland');
         $datetimeField->setValue('2003-12-24 23:59:59');
         $this->assertEquals(
-            '25/12/2003 11:59:59 AM',
+            '25/12/2003, 11:59:59 AM',
             $datetimeField->Value(),
             'User value is formatted, and in user timezone'
         );
@@ -238,11 +259,32 @@ class DatetimeFieldTest extends SapphireTest
         $this->assertEquals(
             '2003-12-24 23:59:59',
             $datetimeField->dataValue(),
-            'Data value is unformatted, and in server timezone'
+            'Data value is in ISO format, and in server timezone'
         );
     }
 
-    public function testTimezoneFromConfigLocalised()
+    public function testTimezoneSetValueWithHtml5()
+    {
+        date_default_timezone_set('Europe/Berlin');
+        // Berlin and Auckland have 12h time difference in northern hemisphere winter
+        $datetimeField = new DatetimeField('Datetime', 'Datetime');
+
+        $datetimeField->setTimezone('Pacific/Auckland');
+        $datetimeField->setValue('2003-12-24 23:59:59');
+        $this->assertEquals(
+            '2003-12-25T11:59:59',
+            $datetimeField->Value(),
+            'User value is in normalised ISO format and in user timezone'
+        );
+
+        $this->assertEquals(
+            '2003-12-24 23:59:59',
+            $datetimeField->dataValue(),
+            'Data value is in ISO format, and in server timezone'
+        );
+    }
+
+    public function testTimezoneSetSubmittedValueLocalised()
     {
         date_default_timezone_set('Europe/Berlin');
         // Berlin and Auckland have 12h time difference in northern hemisphere summer, but Berlin and Moscow only 2h.
@@ -285,4 +327,5 @@ class DatetimeFieldTest extends SapphireTest
             )
         );
     }
+
 }