Time ago handling

1. +++ b/src/Plugin/facets/processor/DateItemProcessor.php
   @@ -78,6 +85,30 @@ class DateItemProcessor extends ProcessorPluginBase implements BuildProcessorInt
   +    $options = [
   +      'older' => t('Older'),
   +      'l30d' => t('Last 30 days'),
   +      'l7d' => t('Last 7 days'),
   +      'yesterday' => t('Yesterday'),
   +      'today' => t('Today'),
   +      'tomorrow' => t('Tomorrow'),
   +      'c7d' => t('Coming 7 days'),
   +      'c30d' => t('Coming 30 days'),
   +      'newer' => t('Newer'),
   +    ];
   

   I can already see the feature request to be able to adjust these.

   "Please add Last 14 days to the options"
   "Please add Coming 9 days to the options"

   Instead of going with textual options, we could go with integer values and only use textual options for the older and newer options. People could then use the following format to define the available options:

   older|Older
   -30|Last 30 days
   -7|Last 7 days
   -1|Yesterday
   0|Today
   1|That day after today, also called tomorrow
   14|We choose to only go 14 days into the future
   newer|Newer
   

2. +++ b/src/Plugin/facets/query_type/SearchApiDate.php
   @@ -406,6 +429,138 @@ class SearchApiDate extends QueryTypeRangeBase {
   +    $now = new DrupalDateTime();
   +    $now->setTimestamp(REQUEST_TIME);
   

   We're only looking at days, so the exact request time doesn't matter, does it?

   Omitting this timestamp allows us to replace all the cloning of $now and then modifying it with just instantiating new DateTime objects.

3. +++ b/src/Plugin/facets/query_type/SearchApiDate.php
   @@ -406,6 +429,138 @@ class SearchApiDate extends QueryTypeRangeBase {
   +    if ($interval_days == 0 && isset($display_dates['today'])) {
   +      $lower = $date->format('Y-m-d\T00:00:00');
   +      $upper = $date->format('Y-m-d\T23:59:59');
   +      return [
   +        'display' => $this->t('Today'),
   +        'raw' => $lower . '__' . $upper,
   +      ];
   +    }
   +    if ($interval_days == 1 && !$future && isset($display_dates['yesterday'])) {
   +      $modify = clone $now;
   +      $modify->modify('- 1 day');
   +      return [
   +        'display' => $this->t('Yesterday'),
   +        'raw' => $modify
   +          ->format('Y-m-d\T00:00:00') . '__' . $now->format('Y-m-d\T23:59:59'),
   +      ];
   +    }
   +    if ($interval_days == 1 && $future && isset($display_dates['tomorrow'])) {
   +      $modify = clone $now;
   +      $modify->modify('+ 1 day');
   +      return [
   +        'display' => $this->t('Tomorrow'),
   +        'raw' => $modify
   +          ->format('Y-m-d\T00:00:00') . '__' . $modify->format('Y-m-d\T23:59:59'),
   +      ];
   +    }
   +
   +    if ($interval_days <= 7 && !$future && isset($display_dates['l7d'])) {
   +      $modify = clone $now;
   +      $modify->modify('- 7 day');
   +      return [
   +        'display' => $this->t('Last 7 days'),
   +        'raw' => $modify
   +          ->format('Y-m-d\T00:00:00') . '__' . $now->format('Y-m-d\T23:59:59'),
   +      ];
   +    }
   +    if ($interval_days <= 7 && $future && isset($display_dates['c7d'])) {
   +      $modify = clone $now;
   +      $modify->modify('+ 7 day');
   +      return [
   +        'display' => $this->t('Coming 7 days'),
   +        'raw' => $now
   +          ->format('Y-m-d\T00:00:00') . '__' . $modify->format('Y-m-d\T23:59:59'),
   +      ];
   +    }
   +
   +    if ($interval_days <= 30 && !$future && isset($display_dates['l30d'])) {
   +      $modify = clone $now;
   +      $modify->modify('- 30 day');
   +      return [
   +        'display' => $this->t('Last 30 days'),
   +        'raw' => $modify
   +          ->format('Y-m-d\T00:00:00') . '__' . $now->format('Y-m-d\T23:59:59'),
   +      ];
   +    }
   +    if ($interval_days <= 30 && $future && isset($display_dates['c30d'])) {
   +      $modify = clone $now;
   +      $modify->modify('+ 30 day');
   +      return [
   +        'display' => $this->t('Coming 30 days'),
   +        'raw' => $now
   +          ->format('Y-m-d\T00:00:00') . '__' . $modify->format('Y-m-d\T23:59:59'),
   +      ];
   +    }
   

   There is quite a lot of duplication going on here. We could probably reduce duplication by abstracting this out into something like an "IntervalOption" class.

   Edit: Even better, switching to numeric options would make this whole thing far simpler.

4. +++ b/src/Plugin/facets/query_type/SearchApiDate.php
   @@ -406,6 +429,138 @@ class SearchApiDate extends QueryTypeRangeBase {
   +      $far = clone $now;
   +      $far->modify('- 250 year');
   

   $far = new DrupalDateTime(- PHP_INT_MAX); would set the date to the oldest possible date.

5. +++ b/src/Plugin/facets/query_type/SearchApiDate.php
   @@ -406,6 +429,138 @@ class SearchApiDate extends QueryTypeRangeBase {
   +      $far = clone $now;
   +      $far->modify('- 250 year');
   

   So what if someone has indexed some historical things going back 2000 years?

   $far = new DrupalDateTime(- PHP_INT_MAX); would set the date to the oldest possible date.

6. +++ b/src/Plugin/facets/query_type/SearchApiDate.php
   @@ -406,6 +429,138 @@ class SearchApiDate extends QueryTypeRangeBase {
   +      $far = clone $now;
   +      $far->modify('+ 250 year');
   

   $far = new DrupalDateTime(PHP_INT_MAX); would set the date to the farthest possible date.

7. +++ b/src/Plugin/facets/query_type/SearchApiDate.php
   @@ -406,6 +429,138 @@ class SearchApiDate extends QueryTypeRangeBase {
   +      $far = clone $now;
   +      $far->modify('+ 250 year');
   

   What if someone has something indexed that describes things into the far future? Say the year 5423?

   $far = new DrupalDateTime(PHP_INT_MAX); would set the date to the oldest possible date.

1. +++ b/src/Plugin/facets/processor/DateItemProcessor.php
   @@ -78,9 +85,32 @@ class DateItemProcessor extends ProcessorPluginBase implements BuildProcessorInt
   +    $description = t("Enter the dates you wish to show in the facet. Enter these as a list. The older and newer keys are special, they are catch-alls for everything that did not fit in your other selected options.");
   

   $this->t()

2. +++ b/src/Plugin/facets/processor/DateItemProcessor.php
   @@ -78,9 +85,32 @@ class DateItemProcessor extends ProcessorPluginBase implements BuildProcessorInt
   +      '#title' => t('Configured dates'),
   

   $this->t()

3. +++ b/src/Plugin/facets/processor/DateItemProcessor.php
   @@ -78,9 +85,32 @@ class DateItemProcessor extends ProcessorPluginBase implements BuildProcessorInt
   +    $values = $form_state->getValues();
   +    $values['display_dates'] = explode("\n", $values['display_dates']);
   +    array_walk($values['display_dates'], 'trim');
   +    $this->setConfiguration($values);
   

   We should probably validate the display_dates to make sure each line is a key|value combination and that all keys are numeric except for the catch-alls

4. +++ b/src/Plugin/facets/query_type/SearchApiDate.php
   @@ -53,6 +53,20 @@ class SearchApiDate extends QueryTypeRangeBase {
   +  /**
   +   * The date the furtest in the future for config relative dates.
   +   *
   +   * @var int
   +   */
   +  protected $furthestFuture = 0;
   +
   +  /**
   +   * The date the furtest in the past for config relative dates.
   +   *
   +   * @var int
   +   */
   +  protected $furthestPast = 0;
   +
   

   This could (in theory) cause side effects if this class is re-used across multiple facets. Are we sure each facet gets a new instance of this class? Perhaps we should add tests to ensure that the furthest past/future does not bleed through to other facets on the same page.

5. +++ b/src/Plugin/facets/query_type/SearchApiDate.php
   @@ -66,6 +80,7 @@ class SearchApiDate extends QueryTypeRangeBase {
   @@ -76,9 +91,16 @@ class SearchApiDate extends QueryTypeRangeBase {
   
   @@ -76,9 +91,16 @@ class SearchApiDate extends QueryTypeRangeBase {
        if ($this->getDateDisplay() === 'relative_date') {
          return $this->calculateRangeRelative($value);
        }
   -    else {
   +    if ($this->getDateDisplay() === 'actual_date') {
          return $this->calculateRangeAbsolute($value);
        }
   +    if ($this->getDateDisplay() === 'config_relative') {
   +      $parts = explode('__', $value);
   +      return [
   +        'start' => date('U', strtotime($parts[0])),
   +        'stop' => date('U', strtotime($parts[1])),
   +      ];
   +    }
      }
    
      /**
   @@ -216,13 +238,22 @@ class SearchApiDate extends QueryTypeRangeBase {
   
   @@ -216,13 +238,22 @@ class SearchApiDate extends QueryTypeRangeBase {
        if ($this->getDateDisplay() === 'relative_date') {
          return $this->calculateResultFilterRelative($value);
        }
   -    else {
   +    if ($this->getDateDisplay() === 'actual_date') {
          return $this->calculateResultFilterAbsolute($value);
        }
   +    if ($this->getDateDisplay() === 'config_relative') {
   +      return $this->calculateResultFilterConfigRelative($value);
   +    }
      }
   

   Code smell, this class seems to violate the open-closed principle. I think we should refactor this class (in a follow up) and introduce the strategy pattern or the plugin pattern to allow for different types.

Thank you very much for working on this very useful feature! A few observations on top of #24 from my testing of the configured relative date range feature:

+ * Returns an array that can be passed trough the facet for grouping results.

Nit: I guess something like through to instead of trough?

+ // Unset $config so it does not bleed trough the next part of the method.

Nit: See above.

+          $this->furthestFuture = ($interval > $this->furthestFuture ? $interval : $this->furthestFuture);
+          $this->furthestPast = abs($interval) > $this->furthestPast ? abs($interval) : $this->furthestPast;

I think we need to calculate furthestFuture and furthestPast for each interval. Otherwise you may end up in a state, where you have an "older" date or a "newer" date and both properties are still 0, which will mess up the date ranges for "older" and "newer" dates. During my testing, I got two "older" categories with different date ranges for my index. I ended up moving those calculations directly to the start of the foreach loop using a slightly different condition for furthestPast

      $this->furthestFuture = ($interval > $this->furthestFuture ? $interval : $this->furthestFuture);
      $this->furthestPast = $interval < 0 && abs($interval) > $this->furthestPast ? abs($interval) : $this->furthestPast;

which seems to work as expected.

+ $modify->modify($this->furthestPast . ' days');

furthestPast is an absolute value, so I guess we should prepend a minus sign here.

Apart from that, there is another issue with the facet counts. Since everything except older and newer always starts at the current date, which I think is totally fine and expected, the counts are messed up a little. For example, if I have 30 and 365 configured, events within the next 30 days will be counted in the 30 days date range only, although I would expected them to be counted in the 365 days date range as well, since they will be listed in both categories. Less abstract example: Suppose today is June 3rd, 2019 and you have two dates: June 11, 2019 and August 02, 2019. I would expect a count of 1 to be shown for the next 30 days and 2 for the next 365 days, but although both dates are listed in the 365 days category as expected, the count for the 365 days category is only 1. I din't look into why this happens and how to best fix this, yet.

And here's something else I just noticed:

+    display_dates:
+      type: sequence
+      label: 'Display relative dates'
+      sequence:
+        type: string
+        label: 'Date type'

Since the date type contains the label to show, we should set the type to label instead of string, so that the setting will become available in config translation.

So I looked at this and went down the rabbit hole of why Search API Solr wasn't doing the existing date ranges as range queries. This needs another issue - and it's code to go into Search API Solr (SearchApiSolrBackend::setFacets()). It's quite important as these iterations over all items get more complicated.

What needs to be born in mind for this patch in relation to that is these will have to be facet.interval rather than facet.range.

Range allows passing the facet.range.gap as +1MONTH or whatever, and the start point can be absolute or NOW relative. I think we have that data to pass easily in the info.

Interval allows for multiple specified facets in the format:-

    (1,10) → will include values greater than 1 and lower than 10
    [1,10) → will include values greater or equal to 1 and lower than 10
    [1,10] → will include values greater or equal to 1 and lower or equal to 10

I've checked it will work with (2018-06-14T00:00:00Z,2018-07-14T00:00:00Z]. It won't with periods like -1YEAR.
It will work with periods, and relative periods like NOW/DAY is now rounded to today, and math can then be applied NOW/DAY-1DAY. If &TZ=full/timezone (like &TZ=Australia/Perth) is applied it will round to the day in the local timezone.

So the patch just needs to take into account that we'll want to send this data in the info to the backend in case they support this.

Update:

#3061897: Support range and interval facets for search_api_date facets for a description of what could happen in Search API Solr.

The next steps in this module would be to add the granularity or intervals array as appropriate to the info that gets passed to the backend. MySQL backend will ignore it (unless someone writes custom handling), Solr will also, until additional range and interval handling for dates is added. But when a backend implements the handling they will return just one item for each gap or interval - with count - so the present code will continue to work, but only iterate over one item (instead of potentially 10000sss).

Update 2:

For elastic this issue #2969137: Date range aggregation references the documentation that includes the data that it would need to provide the facets natively.

Looking into making some reusable code to get the boundaries to send to interval supporting backends I tightened the tests specifically in relation to timezone handling. The previous tests were looking at the far ends of a UTC day. This looks to half an hour into Australia/Sydney days. I'm pretty sure the logic I've used is sound, and it's created breaking tests - and hopefully do so 'consistently' throughout the day.

Created new path with some proposals from comment #25

I noticed that using the count option wasn't working correctly. e.g.:

Today is 2021-10-11.

You have an overview with 3 results with the following dates: 2021-10-11, 2021-10-12 and 2021-10-15.
And 3 filters: Today, Tomorrow and This week.

The filter will show the following result:

• Today (1)
• Today or tomorrow (1)
• This week (1)

When filtering on e.g. This week, the actual filter is something like 2021-10-11 00:00:00 - 2021-10-18 23:59:59 and 3 results are shown in the overview.

Imo the expected result should be:

• Today (1)
• Today or tomorrow (2)
• This week (3)

I added an extra check in the \Drupal\facets\Plugin\facets\query_type\SearchApiDate::build().
I sorted the filters on date loop over all filters. When the period of the previous filter is included in the current filter, the count value of the previous filter is added to the current filter.

Here a version of the patch to work wit v2.