BC Break on SearchFilter with date as input

[Nek-]

API Platform version(s) affected: x.y.z

Description

If you use dates with the search filter, this was supported, it now triggers an exception. This is because previously the type was not always sent with the doctrine parameter, and doctrine was considering the date as string type (which is valid when you have a string like 2021-01-12 as input).

How to reproduce

If you have the following filter configured on an entity, it will break:

/**
 * ...
 * @ApiFilter(SearchFilter::class, properties={"something.id": "exact", "date": "exact"})
 */

Possible Solution

Add support for dates, which is a special case that is not that simple because it requires to manage date, datetime, datetime_immutable and so on.

[alanpoulain]

Related: #241
Maybe adding an exception when using a search filter in a date property would be a good thing?

[soyuka]

If you use dates with the search filter, this was supported, it now triggers an exception. This is because previously the type was not always sent with the doctrine parameter, and doctrine was considering the date as string type (which is valid when you have a string like 2021-01-12 as input).

This was never officialy supported as there is a DateFilter that should be used for dates no?

[Nek-]

It actually was working, see my example. (matching specific days was more intuitive and easier with the search filter than the date filter)

[bendavies]

The DateFilter does not support exact matching (date=2021-03-12), but only before and after filtering ('<=','<','>=', '>'), so we use SearchFilter instead, which worked previously.
Now the type is inferred, added in #3774
so this will be thrown:

Doctrine\DBAL\Types\ConversionException
Could not convert PHP value '2020-12-31' of type 'string' to type 'date_immutable'. Expected one of the following types: null, DateTimeImmutable

what is the way forward here? add = support to DateFilter?

[alanpoulain]

@bendavies you can combine the date filter like this to do so:
?date[before]=2021-03-12&date[after]=2021-03-12
Maybe we should document it?

[bendavies]

that is pretty terrible DX, don't you think?
I'm happy to PR a change to the datefilter.

[alanpoulain]

Not sure why there is no equal. Maybe @dunglas has an idea?
Personally, I'm OK of adding equal.

[bendavies]

hmm, unfortunately it would be date[equal]=2020-12-31, not matching the existing behavior of date=2020-12-31. would have to do some jiggery pokery to fix the BC break here.

[bendavies]

I think I'd like some guidance on what would be acceptable before working on a PR.

[alanpoulain]

Why wouldn't it match the behavior of the search filter?

[bendavies]

because API consumers would have to start using date[equal]=2020-12-31 instead of what they currently use, date=2020-12-31

[alanpoulain]

Oh yes sure, the format is not exactly the same.
I understand that it can be considered as a BC break, but using the search filter for dates is a twisted usage, not documented and not tested.
We really need to clarify this usage.

[bendavies]

I think we can make SearchFilter ignore dates, and let DateFilter handle both formats:
date[equal]=2020-12-31and date=2020-12-31.
this would mitigate the BC break.
WDYT?

[soyuka]

IMHO #3774 should not have break this anyways especially in a patch I'd be in favor of:

• reverting the part of the PR that breaks this
• add this to the next minor (future 2.7) but add an alternative

[alanpoulain]

I'm personally not OK of reverting it. If needed we can just handle the case of the date type (we use a string or null type instead).
And if there is no test covering a used but not documented feature, we cannot prevent BC breaks.

Also, I think having both formats would add more confusion with the search filter.