Provide Views integration for datetime field

I'm not going to clobber the major queue by bumping this, but this is pretty darn important.

@karens

It feels like we should bring in the features of the date filter/sort to support datetime values as well,
so something similar to what you started on #241759: Better date filter and continued in the date module?

I'm going to claim this one for now. We'll see.

Haven't had the time :(

Fix the wrong related issue id

Let me try that.

Closed #2001036: Operators in views UI missing for date field as a duplicate

I think this is major as the views/datefield combination is not really useful currently. Pls, correct me if I am wrong.

Is it just me, or are your dates stored as strings? This has to be the worst data type to store a date in. A string has no sequence and is (most likely) bound to a timezone. Why don't we store it as a DATETIME or at least an INT? It seems pretty significant that you can't create a simple calendar in D8.

field.views.inc: field_views_field_default_views_data()

    // Identify likely filters and arguments for each column based on field type.
    switch ($attributes['type']) {
      case 'int':
      case 'mediumint':
      case 'tinyint':
      case 'bigint':
      case 'serial':
      case 'numeric':
      case 'float':
        $filter = 'numeric';
        $argument = 'numeric';
        $sort = 'standard';
        break;
      case 'text':
      case 'blob':
        // It does not make sense to sort by blob or text.
        $allow_sort = FALSE;
      default:
        $filter = 'string';
        $argument = 'string';
        $sort = 'standard';
        break;
    }

@judahtanthony I created an issue for this: #2366213: Date field doesn't use database-native date storage

HAHA

I wonder if this makes sense to do in core at this time. It seems to me there will still be need for a contrib 'Date' module, to provide things that have not made it into core (end dates immediately come to mind). As such, contrib views integration would most likely need to rework the core views integration.

I think that making date fields at all usable in Views (which they are really not right now) is still a major task that could be committed at any time.
Yes, date.module will need to exist in contrib to provide end dates and the views handlers for that, but we shouldn't give up on handlers for datetime.

Agreed on both.

I'll give this a shot.

This is a very rough start. It has a 'working' filter plugin. However, I don't think this approach will be cross-database compatible, and to truly get working date filters, we'll need to bring in more of the SQL logic from the Date module (ADDTIME, STR_TO_TIME for MySQL, and the PostgreSQL equivalents).

To add an argument plugin, it may be necessary to bring over some of the Date module SQL logic I mentioned above. From D7, adding an argument allowed one to pass in date parts (eg, 2015 for all dates in 2015, or 2015-10 for just October 2015 dates). The resulting SQL (when using MySQL):

WHERE (( (DATE_FORMAT(ADDTIME(STR_TO_DATE(field_data_field_date.field_date_value, '%Y-%m-%dT%T'), SEC_TO_TIME(-25200)), '%Y-%m') >= '2015-10' AND DATE_FORMAT(ADDTIME(STR_TO_DATE(field_data_field_date.field_date_value, '%Y-%m-%dT%T'), SEC_TO_TIME(-25200)), '%Y-%m') <= '2015-10') )

Actually, looking into how numeric dates are handled, each part is a separate argument type (and core already has the date SQL methods mentioned above, just by different names).

I've started on this approach, and am posting for review now to see what folks think of this direction.

This adds tests for the new filter plugin.

This adds tests for the 3 argument types (year, month, and day). It also uses format_date() instead of just date() for timezone compatibility.

This replaces the simple string-comparison in the filter with db-native date functions. I've run the tests on MySQL and PostgreSQL.

MySQL is all green, PostgreSQL fails with:

function to_char(character varying, unknown) does not exist LINE 6: WHERE (( (TO_CHAR(node__field_date.field_date_value, 'YYYY')... ^ HINT: No function matches the given name and argument types. You might need to add explicit type casts.

since the current logic assumes the field being passed in is numeric. I think this can be addressed by overriding the method in the datetime filter handler.

I also added an initial beta-phase evaluation to the summary.

This gets things working for PostgreSQL, but isn't pretty. The Date module uses the 'EXTRACT' function to grab date partials, but that is a lot of logic to bring in at first glance.

I'm unsure as to why that one test is failing. It passes locally on both pgsql and mysql.

Setting this back to needs review since feedback on the technical approach is more important at this stage.

This fixes the fail, which was caused by an additional record with a January date being created due to the current date (this was copied from the filter test which needs relative test data).

Came up against this issue. Patch adds good support for filtering with custom date fields. One thing I noticed is that using the table style, dates were not able to be made sortable.

If that is out of scope for this issue +1 to RTBC.

One thing I noticed is that using the table style, dates were not able to be made sortable

This seems to be a larger issue than just dates. I tried adding a numeric field, and that wasn't click sortable either. I'm not able to find an open issue for this though.

This did get me thinking about adding a datetime sort handler though, which would add granularity for sorting.

This patch adds a sort handler and corresponding tests.

I created #2395763: Fields are not 'click sortable' in views for the click sort issue mentioned in #33.

In terms of functionality I vote +1 to RTBC, works as advertised with no issues.

+++ b/core/modules/datetime/datetime.views.inc
@@ -0,0 +1,47 @@
+  $data = field_views_field_default_views_data($field_storage);

This function has been renamed to views_field_default_views_data, this is what causes the tests to fail. Changing the function name turns tests back to green locally.

some nitpicking....

1. +++ b/core/modules/datetime/src/Plugin/views/filter/Date.php
   @@ -0,0 +1,71 @@
   + * Definition of Drupal\views\Plugin\views\filter\String.
   

   Change to Contains \ and set to the right class name

2. +++ b/core/modules/datetime/src/Tests/Views/ArgumentDateTimeTest.php
   @@ -0,0 +1,141 @@
   + * Contains of Drupal\datetime\Tests\Views\ArgumentDateTimeTest.
   

   'of' too many, lacks leading \

3. +++ b/core/modules/datetime/src/Tests/Views/FilterDateTimeTest.php
   @@ -0,0 +1,151 @@
   + * Contains of Drupal\datetime\Tests\Views\FilterDateTimeTest.
   

   of too many, missing leading \

Thanks for the feedback @Lendude!

This should fix the fatal error, and code comments as mentioned in #42.

It'd be great to get this in before Beta 5 if it isn't too late.

Patch for #43 works nicely, but what I miss is a fact that end user will see plain text fields for datetime values in views filter config forms and exposed filter form.

Patch #44 is trying to fix this, introducing datetime controls for views datetime filter config popup and exposed datetime filters.
Also one additional settings is addition of 'compare by date' options which alows user to specify does it wants to filter dates by date or also by time part. This is usefull when end user wants to filter all post in one specific day or couple of days and does not wants to select time part.

OK this form values elements works little strange when they are in config popup and later in views exposed form. New patch against #43, should work better now.

I think this direction makes sense to potentially simplify input (not requiring full date+time being entered).

+++ b/core/modules/datetime/src/Plugin/views/filter/Date.php
@@ -0,0 +1,127 @@
+        '#description' => $this->t('Should we compare just date part or also time part?'),

I think this UI text needs to be reworked, taking into account https://www.drupal.org/node/604342.

Yeah :)

I think the way it used to work in 7.x is that you can tick of the granularity for every part, from year to seconds. We should probably look at how that works and steal from there, instead of those two hardcoded "part" thingies ;)

Tested this patch, the "An offset from the current time such as "+1 day" or "-2 hours -30 minutes" feature doesn't seem to do anything right now?

@Benjy Using an offset is working for me. There's also a test that uses '- 1 day'.

@Benjy nevermind, I was still testing with the patch from #43. This is indeed not working in the patch from #46.

Thinking about this more, the UI changes from #46 should probably be split into a separate issue which would also address the numeric date plugin UIs provided by Views. The UI from the patch in #43 is identical to that provided by Views.

This is a reroll of #43. As I mentioned in #52, let's improve the UI of both the numeric date plugins as well as these new plugins in a follow-up issue so they can have a unified look and feel.

Needed to account for #2394883: Language setup for entity and field based rendering in views is independent, confusing UI, lacking test coverage.

More schema fixes.

+++ b/core/modules/datetime/src/Plugin/views/argument/Date.php
@@ -0,0 +1,46 @@
+class Date extends NumericDate {

Since this extends the numeric Date argument handler it suffers from the same problems as that handler when trying to add default date arguments, see #2325899: UI fatal caused by views argument handlers no longer can provide their own default argument handling. The fix proposed there would probably work for this implementation too, but haven't tested with both patches applied. But if either issue gets in, it would influence the other, so added it as related.

I'd love to see #2325899: UI fatal caused by views argument handlers no longer can provide their own default argument handling get in first, otherwise we'd just be introducing more known issues.

Other then that, this looks good to me.

The tests needed to account for #2429447: Use data table as views base table, if available..

Sorry if it's not the right place to tell about it. I can create a separate issue if it's better.

I just needed views datetime integration for an internal D8 project, and wanted to be able to set something like "last day of this month" in the date filter. Here's a patch. Drupal\views\Plugin\views\filter\Date might need the same kind of update too?

Saying we have existing Views integration for datetime is just patently false. I've manually tested this patch, and @jhedstrom has apparently been using it for months. There is extensive test coverage added, and it matches very closely with the D7 date.module integration.

Therefore, I consider this RTBC.

I'm sorry

1. +++ b/core/modules/datetime/datetime.views.inc
   @@ -0,0 +1,47 @@
   +function datetime_field_views_data(FieldStorageConfigInterface $field_storage) {
   +  $data = views_field_default_views_data($field_storage);
   +  foreach ($data as $table_name => $table_data) {
   

   Its a wrong assumption that we don't store that field on base tables.

2. +++ b/core/modules/datetime/src/Plugin/views/argument/Date.php
   @@ -0,0 +1,46 @@
   + */
   +namespace Drupal\datetime\Plugin\views\Argument;
   

   here should be a new line in between.

3. +++ b/core/modules/datetime/src/Plugin/views/argument/Date.php
   @@ -0,0 +1,46 @@
   +  /**
   +   * Override to account for dates stored as strings.
   +   */
   +  public function getDateField() {
   ...
   +  /**
   +   * Override to account for dates stored as strings.
   +   *
   +   * {@inheritdoc}
   +   */
   +  public function getDateFormat($format) {
   

   Should we synchronize those comments a bit?

4. +++ b/core/modules/datetime/src/Plugin/views/filter/Date.php
   @@ -0,0 +1,67 @@
   +    $a = $this->query->getDateFormat("'" . format_date($a, 'custom', 'c') . "'", static::$dateFormat, TRUE);
   +    $b = $this->query->getDateFormat("'" . format_date($b, 'custom', 'c') . "'", static::$dateFormat, TRUE);
   ...
   +    $value = $this->query->getDateFormat("'" . format_date($value, 'custom', 'c') . "'", static::$dateFormat, TRUE);
   

   Can we use \Drupal::service('date.formatter')->format()

Let's skip the first part of the complain and open a follow up.

Opened #2489476: [PP-1] Base fields miss out on Views data from hook_field_views_data() per discussion at drupalcon.
Fixed the other points.

Thank you tim!

1. +++ b/core/modules/datetime/src/Plugin/views/argument/DayDate.php
   @@ -0,0 +1,22 @@
   + * Argument handler for a day (DD)
   ...
   +  protected $argFormat = 'd';
   

   Missing fullstop and I'm confused as to why (DD) when the argFormat is 'd'. Perhaps the best thing to do would be to remove (DD).

2. +++ b/core/modules/datetime/src/Plugin/views/argument/MonthDate.php
   @@ -0,0 +1,22 @@
   + * Argument handler for a month (MM)
   ...
   +  protected $argFormat = 'm';
   
   +++ b/core/modules/datetime/src/Plugin/views/argument/YearDate.php
   @@ -0,0 +1,22 @@
   + * Argument handler for a year (CCYY)
   ...
   +  protected $argFormat = 'Y';
   

   As above

3. +++ b/core/modules/datetime/src/Plugin/views/filter/Date.php
   @@ -0,0 +1,106 @@
   +use Drupal\Core\Form\FormStateInterface;
   

   Not used.

This should address #72.

#73 is the wrong patch. Disregard.

This is the proper patch with fixes from #72.

Back to RTBC (assuming the above goes green) since the changes were just cosmetic.

Oops, missed one item from #72.

Tentatively back to RTBC assuming green.

Random testbot fail--back to RTBC.

This issue addresses a major bug and is allowed per https://www.drupal.org/core/beta-changes. Committed 26b7df5 and pushed to 8.0.x. Thanks!

WOOHOO! Thanks for sticking with this one, jhedstrom!!

I've reverted due to multiple failures seen in Drupal\datetime\Tests\Views\FilterDateTimeTest.

Specifically:

* Drupal\datetime\Tests\Views\FilterDateTimeTest (10 pass(es), 4 fail(s), and 0 exception(s))
   - [fail] [Other] "Actual result 
array (
)
is  identical to expected 
array (
  0 => 
  array (
    'nid' => '4',
  ),
)
" in FilterDateTimeTest.php on line 72 of Drupal\datetime\Tests\Views\FilterDateTimeTest->_testOffset().
   - [fail] [Other] "Actual result 
array (
)
is  identical to expected 
array (
  0 => 
  array (
    'nid' => '4',
  ),
)
" in FilterDateTimeTest.php on line 86 of Drupal\datetime\Tests\Views\FilterDateTimeTest->_testOffset().
   - [fail] [Other] "Actual result 
array (
)
is  identical to expected 
array (
  0 => 
  array (
    'nid' => '4',
  ),
)
" in FilterDateTimeTest.php on line 72 of Drupal\datetime\Tests\Views\FilterDateTimeTest->_testOffset().
   - [fail] [Other] "Actual result 
array (
)
is  identical to expected 
array (
  0 => 
  array (
    'nid' => '4',
  ),
)
" in FilterDateTimeTest.php on line 86 of Drupal\datetime\Tests\Views\FilterDateTimeTest->_testOffset().

Darn.

These fails are in the offset test, and while I can't reproduce them locally, I think I can fix the test so it isn't so fragile (I think on really slow test runs, the filter fails because the nodes are too old for the filter to work).

Actually, the test is already setting a date 1 day in the future, so unless 24 hours are passing between when the setUp method is called, and when the test runs, I don't think that's the issue.

I checked how the offset test in the views numeric date filter works, and it is identical except it uses time() instead of REQUEST_TIME, so I've made that change here.

After thinking about this more, I've realized the intermittent failures were due to timezones! The test data was only storing date, not date and time, so under certain circumstances, the offset query would fail to detect a future date. This patch adds time storage to the test date, which should resolve the seemingly-random failures.

Timezones aren't the actual issue--rather as the previous versions of the patch were run near midnight on the testbot, since the time part wasn't stored, the queries would fail to find any data.

For example, imagine the node is created with field_date = 2015-05-26 when the actual time is 2015-05-25T23:59:59--the query is then run at 2015-05-26T00:00:01, where a +1 day offset would look for values of 2015-05-27, so our test node was not found.

Oh interesting research on the failures.
Ideally I would like to see @mpdonadio to have a look at them, he did some really good work on similar stuff recently.

Confirmed that #89 failed at 23:32, #90 passed at 23:33. Difference is that #89 set only date, #90 sets date and time.

I tried to test the filter manually and I think there's a problem with timezones.

1. Add field_date to article
2. Created an article and set the date to 22:30
3. Added field_date to admin/content as a field and exposed filter with "between" operator
4. Went to admin/content and date was displayed correctly
5. Typed 22:00 and 23:00 for values in the filter, and got no results
6. Typed 19:00 and 20:00, and now I see my article

In table node__field_date the value is set as 2015-05-30T19:30:00.

Is this only me or something with the patch?

+++ b/core/modules/datetime/src/Plugin/views/filter/Date.php
@@ -0,0 +1,105 @@
+  /**
+   * Override parent method, which deals with dates as integers.
+   */
+  protected function opBetween($field) {
+    $origin = ($this->value['type'] == 'offset') ? REQUEST_TIME : 0;
+    $a = intval(strtotime($this->value['min'], $origin));
+    $b = intval(strtotime($this->value['max'], $origin));
+

The request stack should be injected, and the request time should be retrieved from the current request.

Regarding spurious fails, I pulled my hair out over them in #2399211: Support all options from views fields in DateTime formatters. Take a look at that. The tests should be setting up a timezone in the setUp, so TestBot and local tests behave the same. Also, I am not seeing any calls to datetime_date_default_time() anywhere. Date-only objects should be created in UTC have this called on them, so the time portion gets set to noon. This eliminates timezone problems when you use them (noon UTC is the same day everywhere). That issue also has links out to a bug I found last night and to a discussion to add timezone setup to WebTestBase.

+++ b/core/modules/datetime/src/Tests/Views/FilterDateTimeTest.php
@@ -0,0 +1,153 @@
+      // For the offset test, put a date 24 hours in the future. The `time()`
...
+      \Drupal::service('date.formatter')->format(time() + 86400, 'custom', DATETIME_DATETIME_STORAGE_FORMAT),
+    );

I think if you do the datetime_date_default_time() thing on the dates, that you can go back to REQUEST_TIME or the injected request object. This just seems like a ticking time bomb.

The request stack should be injected, and the request time should be retrieved from the current request.

I don't think we need to do that here--the REQUEST_TIME is also set using $_SERVER['REQUEST_TIME'], which is the same as that in the request object.

Regarding the timezone issues, that was a red herring as to the failures here, so let's follow that up in a separate issue (it would be more broadly applicable to the date field, etc).

This removes the un-needed switch to time() as mentioned in #98.

+++ b/core/modules/datetime/src/Plugin/views/filter/Date.php
@@ -0,0 +1,105 @@
+    $a = $this->query->getDateFormat("'" . $this->dateFormatter->format($a, 'custom', 'c') . "'", static::$dateFormat, TRUE);
+    $b = $this->query->getDateFormat("'" . $this->dateFormatter->format($b, 'custom', 'c') . "'", static::$dateFormat, TRUE);
...
+    $value = $this->query->getDateFormat("'" . $this->dateFormatter->format($value, 'custom', 'c') . "'", static::$dateFormat, TRUE);

Thinking more about the TZ issue in #95, I think actually in this code we want to pass in the timezone not of the site, but of the server, since that's how the dates are stored in the db, and the formatted date used here is part of the db query.

This should resolve the issue Olli noticed in #96. (Essentially the changes I outlined in #100, plus a test). The test only patch is against the previous patch, but without the fix included.

Manually tested that the latest patch fixes #96. Great work @jhedstrom!

The test that was supposed to fail didn't because I think the testrunners are on UTC. This change hard-codes a non-UTC timezone for the test.

Ok, previous test didn't actually set the timezone. This time!

Test-only patch is still against #99.

Only minor changes to the test since #102, tentatively bouncing back to RTBC.

To summarize the changes since this was previously committed:

• Random failures were due to storing date only, rather than date and time. This caused tests that spanned midnight on the testbot to fail. See example in #91
• Olli discovered timezone issues in #96. These are now resolved (and have tests to boot)

Thanks for working out what caused the random fails - and we;ve another bug whilst we're at it - awesome!

Let's try again... Committed 4a4ee05 and pushed to 8.0.x. Thanks!

Just back from my holiday I noticed that this causes continuous test fails in the datetime test group for SQLite (see SQLite bot).

Drupal\datetime\Tests\Views\FilterDateTimeTest                 1 passes   7 fails                            
Drupal\datetime\Tests\Views\SortDateTimeTest                   2 passes   2 fails                            
Drupal\datetime\Tests\Views\ArgumentDateTimeTest          4 passes   8 fails

Thanks for finding this @bzrudi71. Let's open a new issue to fix this.

This patch introduced the $string_date to getDateFormat.

+++ b/core/modules/views/src/Plugin/views/query/QueryPluginBase.php
@@ -237,12 +237,15 @@ public function setupTimezone() {
    *   An appropriate query expression pointing to the date field.
    * @param string $format
    *   A format string for the result, like 'Y-m-d H:i:s'.
+   * @param boolean $string_date
+   *   For certain databases, date format functions vary depending on string or
+   *   numeric storage.
    *
    * @return string
    *   A string representing the field formatted as a date in the format
    *   specified by $format.
    */
-  public function getDateFormat($field, $format) {
+  public function getDateFormat($field, $format, $string_date = FALSE) {
     return $field;
   }

+++ b/core/modules/views/src/Plugin/views/query/Sql.php
@@ -1748,9 +1748,9 @@ public function setupTimezone() {
   /**
-   * Overrides \Drupal\views\Plugin\views\query\QueryPluginBase::getDateFormat().
+   * {@inheritdoc}
    */
-  public function getDateFormat($field, $format) {
+  public function getDateFormat($field, $format, $string_date = FALSE) {

Unfortunately this is not reliable as there are several calls to getDateFormat where it is not provided. For example in Drupal\views\Plugin\views\HandlerBase::getDateFormat(). In order for Postgres and SQLite to work as expected we need this information :(. Prior to this patch SQLite was at zero fails and the only reason it is not a supported test environment is because new DrupalCI has not yet been delivered.

Regardless of whether SQLite or Postgres is supported $string_date not being correct is a critical bug introduced by this patch therefore I've decided to revert this patch after doing investigative work in #2502343: New SQLite failures in vewis due to data handling.

Can/should $string_format be added to \Drupal\Core\Database\Connection as a static $dateStringFormat (I can't remember if PHP support abstract constants), and then each implementation can set it, which then can be used where needed?

Interestingly enough, the $string_date parameter was introduced for postgres rather than MySQL (but not retested once the sort handler was added). This patch gets things working again on postgres and SQLite. I've tested MySQL. SQLite, and Postgres locally and all are passing.

Re #117 Views already has special handling for dates in Drupal\views\Plugin\views\query\Sql, so probably no need to propagate that up the the connection level at this time.

Unfortunately I think this is more complex that that... since date plugins are being used on field which are a timestamp.

For example:

    $data['node_field_data']['created_fulldate'] = array(
      'title' => t('Created date'),
      'help' => t('Date in the form of CCYYMMDD.'),
      'argument' => array(
        'field' => 'created',
        'id' => 'date_fulldate',
      ),
    );

in \Drupal\node\NodeViewsData

And it's definition is defined as:

    $fields['created'] = BaseFieldDefinition::create('created')
      ->setLabel(t('Authored on'))
      ->setDescription(t('The time that the node was created.'))
      ->setRevisionable(TRUE)
      ->setTranslatable(TRUE)
      ->setDisplayOptions('view', array(
        'label' => 'hidden',
        'type' => 'timestamp',
        'weight' => 0,
      ))
      ->setDisplayOptions('form', array(
        'type' => 'datetime_timestamp',
        'weight' => 10,
      ))
      ->setDisplayConfigurable('form', TRUE);

In Drupal\node\Entity\Node - which uses Drupal\Core\Entity\Plugin\Field\FieldType\CreatedItem which is an integer. I think whether or not we're dealing with a string date or a numeric date needs to be pushed into the plugin - not sure when it could work this out. So that all date plugins have a chance of working on all date fields.

Reminds me of #1145976: Make date handlers easily identifiable (not actually going to help here, I just remember it being similar)

re #120 this doesn't touch the display plugin though, and id = date_fulldate will use the existing views numeric plugin, rather than these?

+++ b/core/modules/datetime/src/Plugin/views/argument/MonthDate.php
@@ -0,0 +1,22 @@
+<?php
+
+/**
+ * @file
+ * Contains \Drupal\datetime\Plugin\views\argument\MonthDate.
+ */
+
+namespace Drupal\datetime\Plugin\views\argument;
+
+/**
+ * Argument handler for a month.
+ *
+ * @ViewsArgument("datetime_month")
+ */
+class MonthDate extends Date {
+
+  /**
+   * {@inheritdoc}
+   */
+  protected $argFormat = 'm';
+
+}

re #122 I think that FullDate plugin is a argument plugin, same as this. Will this work with SQLite?

+++ b/core/modules/datetime/src/Plugin/views/argument/Date.php
@@ -0,0 +1,45 @@
+  /**
+   * {@inheritdoc}
+   */
+  public function getDateFormat($format) {
+    // Pass in the string-field option.
+    return $this->query->getDateFormat($this->getDateField(), $format, TRUE);
+  }

This is what sets the string date parameter in the argument handler (the day/month/year handlers for datetime extend this one), so with the changes to views' Sql class, this does now work with SQLite (there's tests that cover the new argument handler too).

Perhaps if the string date argument were converted to a class property it would better-document the behavior?

Er, scratch that... The query object is used across all fields, which could involve both string dates and numeric dates.

I think that FullDate plugin is a argument plugin, same as this. Will this work with SQLite?

Yes, the $argFormat = 'm' maps directly to the SQLite format in \Drupal\views\Plugin\views\query\Sql::getDateFormat():

...
     case 'sqlite':
        $replace = array(
          'Y' => '%Y',
          // No format for 2 digit year number.
          'y' => '%Y',
          // No format for 3 letter month name.
          'M' => '%m',
          'm' => '%m',

and then with the new parameter <$string_date> which defaults to false, numeric dates continue to function as they do now, and string dates in sqlite skip the unixepoch bit which broke tests in sqlite above.

As mentioned above, since the new Drupal\datetime\Plugin\views\argument\Date is the only plugin that will call getDateFormat() with $string_date set to TRUE, the new parameter will only impact string dates.

Patch 118 applied cleanly to the drupal 8.0.0-beta11

After the patch I am able to filter my view by an end date field. With filter of greater than "now".

Perfect for creating Upcoming or Past Events. Can we get some more People to Review?

Doesn't work completely.
For fields that are date only (no time), it doesn't seem to work properly, because it also takes a time (which can't be entered then) into account.
There is also a small formatting mistake it seems, because in the filter on view edit it shows as e.g. (> now).

@daften can you clarify what isn't working with just date fields? There are tests that use dates, and tests that use datetime.

Also, the UI is identical to numeric dates for Views, so if there are formatting issues, those most likely need to be in a follow-up issue that addresses both.

This may be related, #2510348: Datetime select list widget has options for time interval on date only mode.

I've been out of the loop for a long time, but I keep an eye on much of the 'date' stuff.

Regarding #130, I'm pretty sure ALL date fields include time information, even if the data field granularity does not include time (it is date only).
Thus IIRC, date fields without time, actually have time set to 00:00 (12am midnight) in the stored date field value, which is not displayed, but may be used in some date manipulations.

I've tried it out quickly again on simplytest (Simplytest direct url for patch #118).

• First adapt the content type Page so it has a date field with date type "Date only", Default "Current Date".
• Then create content of type Page one with date on yesterday, one with date today, one with date tomorrow
• Create a view for content type page with an extra filter with date field greater than or equal to now
• The view should show all content for today and tomorrow, it only shows content for today

So granularity is not taken into account it seems. Like @arlinsandbulte says, all date fields include time information, but that shouldn't be taken into account for the comparison in the view IMO.

Date-only fields in the database are stored at 00:00:00+00:00 (midnight UTC).

I suspect (but can't be certain) that there may be a timezone problem here. TestBot runs as UTC, which can mask some problems. A worthwhile check would be to set an explicit timezone in the test and see if there are failures.

Date-only fields are stored simply as date:

/**
 * Defines the format that date and time should be stored in.
 */
const DATETIME_DATETIME_STORAGE_FORMAT = 'Y-m-d\TH:i:s';

/**
 * Defines the format that dates should be stored in.
 */
const DATETIME_DATE_STORAGE_FORMAT = 'Y-m-d';

This should address the issue detailed in #135. Thanks for digging into this folks. Reviews much appreciated!

Sorry, yes, @jhedstrom is correct about the storage; I got something confused in my head. When it comes out as a DrupalDateTime object, the time gets set to midnight UTC, until you call datetime_date_default_time() on it. This is one of the quirks in date handling...

I want to look at the tests more closely. We should probably have a case that would have caught #135, or am I missing something?

1. +++ b/core/modules/datetime/src/Plugin/views/filter/Date.php
   @@ -0,0 +1,119 @@
   +/**
   + * @file
   + * Contains \Drupal\datetime\Plugin\views\filter\String.
   + */
   +
   

   Should be \Drupal\datetime\Plugin\views\filter\Date

2. +++ b/core/modules/datetime/src/Plugin/views/filter/Date.php
   @@ -0,0 +1,119 @@
   +  protected $dateFormat = 'Y-m-d H:i:s';
   

   It's a shame we can't use the constant from the module here.

3. +++ b/core/modules/datetime/src/Plugin/views/filter/Date.php
   @@ -0,0 +1,119 @@
   +      $this->dateFormat = 'Y-m-d';
   

   Torn on this. We can't use the module constant above, so using it here would be weird.

4. +++ b/core/modules/datetime/src/Plugin/views/filter/Date.php
   @@ -0,0 +1,119 @@
   +  protected function opBetween($field) {
   +    $origin = ($this->value['type'] == 'offset') ? REQUEST_TIME : 0;
   +    $a = intval(strtotime($this->value['min'], $origin));
   

   Do we want to inject the RequestStack so we can get this from the current request instead? I would kinda prefer this, and it would also help with unit tests (if we decide to add them).

5. +++ b/core/modules/datetime/src/Plugin/views/filter/Date.php
   @@ -0,0 +1,119 @@
   +    // This is safe because we are manually scrubbing the value.
   +    $field = $this->query->getDateFormat($field, $this->dateFormat, TRUE);
   +    $this->query->addWhereExpression($this->options['group'], "$field $this->operator $value");
   

   Just want to point out that a similar thing is done with the timestamp filters in the views module itself, and I don't see a problem with it.

6. +++ b/core/modules/datetime/src/Tests/Views/ArgumentDateTimeTest.php
   @@ -0,0 +1,141 @@
   +class ArgumentDateTimeTest extends DateTimeHandlerTestBase {
   
   +++ b/core/modules/views/src/Plugin/views/query/Sql.php
   @@ -1797,7 +1797,13 @@ public function getDateFormat($field, $format) {
   +        // In order to allow for partials (eg, only the year), transform to a
   +        // date, back to a string again.
   +        // @todo this is very messy, and EXTRACT should probably be used.
   +        return "TO_CHAR(TO_TIMESTAMP($field, 'YYYY-MM-DD HH24:MI:SS'), '$format')";
   

   I think there is a policy in place to not commit anything with a @todo that doesn't correspond to a followup. Not totally sure this is worth a followup? Can we fix it here?

@jhedstrom thanks for your persistence with this issue.

I tested the patch from #138 as instructed in #135. The filter works as expected if you use >= "TODAY" but not >= "NOW". One might expect a date of the current day to be equal to "NOW" but at least there is a workaround.

I want to add that I think that a numeric date argument plugin is essential. Is the plan to work that out after the filter seems solid enough to commit?

@Jaesin the input for relative time offsets uses these allowed formats: https://secure.php.net/manual/en/datetime.formats.relative.php What's really odd is that 'now' is allowed, but according to those docs "is simply ignored"...

Since the numeric filters have the same input form, rather than change the help text here, we could do that in a follow-up that addressed both numeric and string date handlers.

Er, actually, it makes sense that 'now' is ignored, because strtotime('now', $now) should always just return whatever $now is set to.

I'm writing tests now that use 'now' for offset views of date-only fields.

This should address #140.

@Jaesin in both manual testing, and in the new automated tests added here, both 'today' and 'now' are working as expected for me.

And for the sake of completeness, here's the test runs from sqlite and pgsql.

The new test coverage looks good, but ...

With manual testing, <= today and <= now both work. >= today works, but >= now doesn't (only pulls up future date). Oddly enough, the timestamp plugins do the exact same thing.

And even odder, if I change FilterDateTest to use 'today' instead of 'now', then it fails on lines 85 and 98.

Could this be because `new DateTime('now')` uses the current time and `new DateTime('today')` uses midnight? I was going to suggest mapping 'now' to 'today' for date-only fields, but that doesn't describe the fails when I hacked the test.

Not changing the status since I am confused on what is really happening here.

I can not reproduce the manual test results @mpdonadio reports above. For date-only fields, >= now and >= today both work the same. The test performs this same query too. For date+time fields, today always sets the time to midnight of the current date (00:00:00), while now uses the current date and time, thus they will behave differently.

I re-ran my tests and think this is good to go. I am pretty sure my observations in #146 were because I misconfigured the field to be date+time.

array() is dead, usage of the short array syntax is highly recommended (not sure if we mandate it yet) for all new code.

Here's a reroll to address that.

My interdiff got swallowed.

@bojanz actually atm we have no coding standard around which array syntax to use - only that we should try to be consistent within a method / function.

Third time lucky?

Committed 24219fe and pushed to 8.0.x. Thanks!

Arguments don't allow operators so I've added a followup issue here #2544832: Views argument for the datetime module should use maths..

+++ b/core/modules/datetime/src/Plugin/views/sort/Date.php
@@ -0,0 +1,56 @@
+   * Override query to provide 'second' granularity.
+   */
+  public function query() {
+    $this->ensureMyTable();
+    switch ($this->options['granularity']) {
+      case 'second':
+        $formula = $this->getDateFormat('YmdHis');
+        $this->query->addOrderBy(NULL, $formula, $this->options['order'], $this->tableAlias . '_' . $this->field . '_' . $this->options['granularity']);
+        return;
+    }
+
+    // All other granularities are handled by the numeric sort handler.
+    parent::query();
+  }
+

What's the reason for this? As far as I can see, it behaves the same without that conversion, and this seems to make it very slow in my case, since you you can't have an index on this.

And when I say slow I mean 2s+, compared to 0.00s before.

Created followup, #2631958: Remove unnecessary query method in \Drupal\datetime\Plugin\views\sort\Date

Hi, I'm not really sure if this is related but how can I filter my view to only show content where the end date has not passed ?

Thank you!

Best regards
Adam