Skip to content

[RFR] Fixing date filter to not consider empty string as a current date#382

Merged
dunglas merged 1 commit into
api-platform:1.xfrom
salahm:fix-empty-strings-for-date-filter
Jan 11, 2016
Merged

[RFR] Fixing date filter to not consider empty string as a current date#382
dunglas merged 1 commit into
api-platform:1.xfrom
salahm:fix-empty-strings-for-date-filter

Conversation

@salahm

@salahm salahm commented Jan 11, 2016

Copy link
Copy Markdown
Q A
Bug fix? may be
New feature? no
BC breaks? may be
Deprecations? no
Tests pass? yes
Fixed tickets N/A
License MIT

Actually when we use DateFilter, if we send an empty parameter property[before]=, doctrine will convert it to current date (new \Datetime('')). This is annoying for a filtering behaviour, empty params should be ignored.

If we want use currentDate in DateFilter, we can use now string.

@salahm salahm force-pushed the fix-empty-strings-for-date-filter branch from 501c781 to 253652a Compare January 11, 2016 09:15
@salahm salahm changed the title [RFR] Fixing date filter to not considering empty string as a current date [RFR] Fixing date filter to not consider empty string as a current date Jan 11, 2016
@sstok

sstok commented Jan 11, 2016

Copy link
Copy Markdown

👍 but you should add some tests to prevent future regressions (and proof it's working).

@sroze

sroze commented Jan 11, 2016

Copy link
Copy Markdown
Contributor

I definitely agree with the idea. Can you add Behat tests? Then that would be good 👍

@dunglas

dunglas commented Jan 11, 2016

Copy link
Copy Markdown
Member

👍

@salahm salahm force-pushed the fix-empty-strings-for-date-filter branch from 253652a to fd0d73e Compare January 11, 2016 14:58
@salahm

salahm commented Jan 11, 2016

Copy link
Copy Markdown
Author

Tests added.

dunglas added a commit that referenced this pull request Jan 11, 2016
[RFR] Fixing date filter to not consider empty string as a current date
@dunglas dunglas merged commit a0ec5bb into api-platform:1.x Jan 11, 2016
@dunglas

dunglas commented Jan 11, 2016

Copy link
Copy Markdown
Member

Thanks @salahm

@mbrodala

mbrodala commented Aug 3, 2023

Copy link
Copy Markdown
Contributor

Is there a reason this change is not present in newer APIP versions? It seems this only went into the 1.x branch without forward-porting to newer branches.

@mbrodala

mbrodala commented Jul 9, 2025

Copy link
Copy Markdown
Contributor

@dunglas Can you check this? It is missing in API Platform 2.x, 3.x and 4.x.

@soyuka

soyuka commented Jul 15, 2025

Copy link
Copy Markdown
Member

Feel free to cherry-pick that and open a PR

@mbrodala

Copy link
Copy Markdown
Contributor

Started in #7291 now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants