Computed field value for DateTime field causes start time validation error

Failing test

PR with fix: https://github.com/drupalcommerce/commerce/pull/741

+++ b/modules/promotion/tests/src/Kernel/Entity/PromotionTest.php
@@ -205,10 +205,21 @@ class PromotionTest extends CommerceKernelTestBase {
+    $fake_time->getRequestTime()->willReturn(mktime(0, 0, 0, '02', '15', '2016'));

That meant to me 2017-02-15. And when I change that locally it passes.

I'm really confused. Because when we run #2792653: Checkout coupon pane implementation there's this bug.

Okay, now this fails and makes the issue clear. The promotion which has a start date manually set works fine. However, when a start date is not specified it seems to fail.

So confused.

gmdate, running kernel test where promotion uses generated date time.

Start time: 1495886400
Request time: 1495855424
Difference: 0.35851851851852

changed to just date

Start time: 1495886400
Request time: 1495856189
Difference: 0.34966435185185

So I think the problem is in core?

So confused. I added the following debugging in the entity class for Promotion.

  public static function getDefaultStartDate() {
    $timestamp = \Drupal::time()->getRequestTime();
    print 'Default start time: ' . (new \DateTime())->setTimestamp($timestamp)->format('Y-m-d h:i:s A') . PHP_EOL;
    return date(DATETIME_DATE_STORAGE_FORMAT, $timestamp);
  }

public function available(OrderInterface $order) {
..
    $time = \Drupal::time()->getRequestTime();
    if ($this->getStartDate()->format('U') > $time) {
      print 'Start time: ' . (new \DateTime())->setTimestamp($this->getStartDate()->format('U'))->format('Y-m-d h:i:s A') . PHP_EOL;
      print 'Request time: ' . (new \DateTime())->setTimestamp($time)->format('Y-m-d h:i:s A') . PHP_EOL;
      print 'Difference: ' . ($this->getStartDate()->format('U') - $time) / 3600 . PHP_EOL;
      return FALSE;
    }

The result:

Default start time: 2017-05-27 02:19:04 PM
Start time: 2017-05-27 10:00:00 PM
Request time: 2017-05-27 02:19:04 PM

So, for some reason once the time is passed to TypedData/Field API it jumps. I've had a nearly constant different of almost 8 hours. And actually, at looking at changes, it decreases over time.

more, better testing.

Improved debugging when getting default value

    print 'Default start time: ' . (new \DateTime(gmdate(DATETIME_DATE_STORAGE_FORMAT, $timestamp)))->format('Y-m-d h:i:s A e') . PHP_EOL;
    return gmdate(DATETIME_DATE_STORAGE_FORMAT, $timestamp);

This gets us the following output in test:

Default start time: 2017-05-27 12:00:00 AM Australia/Sydney
Start time: 2017-05-27 10:00:00 PM Australia/Sydney
Request time: 2017-05-27 02:42:39 PM Australia/Sydne

Somewhere core screws our date. The only core test coverage for this TypedData plugin is `\Drupal\KernelTests\Core\TypedData\TypedDataTest::testGetAndSet` and barely is testing.

The issue is the \Drupal\datetime\DateTimeComputed class.

with adjustment

  /**
   * {@inheritdoc}
   */
  public function getStartDate() {
    print 'WTFDRUPAL: ' . (new \DateTime($this->get('start_date')->value))->format('Y-m-d h:i:s A e') . PHP_EOL;

    return $this->get('start_date')->date;
  }

I get this output

Default start time: 2017-05-27 12:00:00 AM Australia/Sydney
WTFDRUPAL: 2017-05-27 12:00:00 AM Australia/Sydney
WTFDRUPAL: 2017-05-27 12:00:00 AM Australia/Sydney
Start time: 2017-05-27 10:00:00 PM Australia/Sydney
Request time: 2017-05-27 02:59:16 PM Australia/Sydney
WTFDRUPAL: 2017-05-27 12:00:00 AM Australia/Sydney

When the value is via DateTimeComputed the timed difference is way off.

It seems the problem is #2632040: [PP-1] Add ability to select a timezone for datetime field.

$date = DrupalDateTime::createFromFormat($storage_format, $value, DATETIME_STORAGE_TIMEZONE);

It defaults timezone to UTC. Working on a fix.

RE: #10. True, we assume midnight.

The test failed last night: https://travis-ci.org/drupalcommerce/commerce/jobs/236614056. Just passed locally. So it's all about the hours.

Based on the linked core issue, I do think we should at least consider the following changes

   public function getStartDate() {
 -    return $this->get('start_date')->date;
 +    // @todo use `date` property once https://www.drupal.org/node/2632040 fixed.
 +    return DrupalDateTime::createFromFormat(DATETIME_DATE_STORAGE_FORMAT, $this->get('start_date')->value);
    }
...
-    return $this->get('end_date')->date;
 +    // @todo use `date` property once https://www.drupal.org/node/2632040 fixed.
 +    if (!$this->get('end_date')->isEmpty()) {
 +      return DrupalDateTime::createFromFormat(DATETIME_DATE_STORAGE_FORMAT, $this->get('end_date')->value);
 +    }

Because the calculated date time does not take into consideration the timezone. Or we should change it. Or not.

Either way, we've had plenty of people having invalid coupons due to start times. Functional tests pass versus kernel tests with our logic.

I found #2632040: [PP-1] Add ability to select a timezone for datetime field. Our errors were using the computed date field value and not the raw timestamp and generating our own date time object. This bug only realized itself around evening time.

The PR here has tests and now tests this bug specifically, and has the fix: https://github.com/drupalcommerce/commerce/pull/741

Patch for those who need it