AbstractDateFilter::filterRange() applies a WHERE condition separately for the start and end values in the case of DateRangeOperatorType::TYPE_NOT_BETWEEN

[tonyaxo]

Subject

AbstractDateFilter::filterRange() applies a WHERE condition separately for the start and end values in the case of DateRangeOperatorType::TYPE_NOT_BETWEEN.

I am targeting this branch, because it fixes bug in 4.x.

This fixes a bug that arises when one of this range borders is not set.

Closes #1766.

Changelog

### Fixed
- DateTimeRangeFilter exception occurs when either the `start` or `end` field is empty.

[VincentLanglet]

Thanks, sorry for the delay

[p1nGgeR]

Hi @VincentLanglet @tonyaxo

It seems to me that this fix has broken the filer

Query before the fix
SELECT * FROM table WHERE column < :start OR column > :end;

Query after the fix
SELECT * FROM table WHERE column < :start AND column > :end;

Am I missing something?

[VincentLanglet]

Indeed, can you make a fix ?

I assume we have to check

If both start and end are not null, do the previous query with OR elseif start id not null do the start query elseif end is not null do the end query.

[p1nGgeR]

@VincentLanglet
I'm just thinking, maybe it doesn't make sense to use "filterRange" if both "start" and "end" dates are not provided?
If only one (start or end) date is provided, then it is not a correct use case for a date range filter.
I ask because there is already an embedded "if" and adding another "if" will not make this method any easier.

What if this check

if (
    !$value['start'] instanceof \DateTimeInterface
    && !$value['end'] instanceof \DateTimeInterface
) {
    return;
}

will be changed to

if (
    !$value['start'] instanceof \DateTimeInterface
    || !$value['end'] instanceof \DateTimeInterface
) {
    return;
}

This change will make it possible to remove null checks for $value['start'] / $value['end'] and apply single where for both between and not between types

if (DateRangeOperatorType::TYPE_NOT_BETWEEN === $type) {
    $this->applyWhere($query, sprintf('%s.%s < :%s OR %s.%s > :%s', $alias, $field, $startDateParameterName, $alias, $field, $endDateParameterName));
} else {
    $this->applyWhere($query, sprintf('%s.%s >= :%s AND %s.%s <= :%s', $alias, $field, $startDateParameterName, $alias, $field, $endDateParameterName));
}

I'm not sure I'm considering every scenario, though.
What are your thoughts?

[VincentLanglet]

I agree that using the filter without both start and end is weird but if it just require an embedded if to support it I would prefer to keep the support rather than ignoring such case.

[VincentLanglet]

I'll make a fix #1807

[p1nGgeR]

I was preparing a PR, but okay, thanks