Skip to content

Fix issue in Required filter validator with dot notation#4221

Merged
alanpoulain merged 1 commit into
api-platform:2.6from
jdeniau:jd-fix-requiredFilterDotNotation
Apr 16, 2021
Merged

Fix issue in Required filter validator with dot notation#4221
alanpoulain merged 1 commit into
api-platform:2.6from
jdeniau:jd-fix-requiredFilterDotNotation

Conversation

@jdeniau

@jdeniau jdeniau commented Apr 15, 2021

Copy link
Copy Markdown
Contributor

Fixes #4214

@alanpoulain

Copy link
Copy Markdown
Member

Thank you! Don't you think it's a bugfix? It should target 2.6, isn't it?

@jdeniau

jdeniau commented Apr 15, 2021

Copy link
Copy Markdown
Contributor Author

@alanpoulain you are totally right 👍

I updated the target.

@jdeniau jdeniau changed the base branch from main to 2.6 April 15, 2021 18:58
@jdeniau jdeniau force-pushed the jd-fix-requiredFilterDotNotation branch from 64bd8d0 to 3c404ee Compare April 15, 2021 19:01
@norkunas

Copy link
Copy Markdown
Contributor

In Symfony there is HeaderUtils::parseQuery, maybe apip could use this in RequestParser::parseRequestParams ?

@jdeniau jdeniau force-pushed the jd-fix-requiredFilterDotNotation branch from 3c404ee to 0b65adc Compare April 16, 2021 06:11
@guilliamxavier

Copy link
Copy Markdown
Contributor

@norkunas: Symfony's HeaderUtils::parseQuery (which was actually partly inspired from API Platform's RequestParser::parseRequestParams) isn't available in Symfony < 5.2 (symfony/symfony#37272)

@norkunas

Copy link
Copy Markdown
Contributor

I know, but maybe it could fallback to it if available and later could be dropped from apip itself

@alanpoulain

Copy link
Copy Markdown
Member

Yes it's possible to create a wrapper (and it's a good idea!), like we do for the Doctrine inflector: https://github.com/api-platform/core/blob/main/src/Util/Inflector.php.

@alanpoulain

Copy link
Copy Markdown
Member

Anyway it can be done in another PR. Let's merge this one first. Could you add a changelog entry @jdeniau?

@jdeniau jdeniau force-pushed the jd-fix-requiredFilterDotNotation branch from 0b65adc to dd5ea7e Compare April 16, 2021 09:19
@jdeniau

jdeniau commented Apr 16, 2021

Copy link
Copy Markdown
Contributor Author

@alanpoulain done.

The CI fails for a reason that seems unrelated to my PR though. (some cache issue apparently)

@alanpoulain alanpoulain merged commit 8d4891a into api-platform:2.6 Apr 16, 2021
@alanpoulain

Copy link
Copy Markdown
Member

Thank you @jdeniau.

@jdeniau jdeniau deleted the jd-fix-requiredFilterDotNotation branch April 16, 2021 12:52
@TheKassaK

Copy link
Copy Markdown

Hi, do you know when/how can I have this fix ?

I'm on apiplatform 2.6.3

@ahmed-bhs

Copy link
Copy Markdown
Contributor

@alanpoulain @jdeniau Guys small question, why we are using a custom RequestParser util to parse the request params, I think we could just call $request->query->all() instead of using the Request Parser, like this:

  $queryString = RequestParser::getQueryString($request);
  $queryParameters = $queryString ? RequestParser::parseRequestParams($queryString) : [];  

It's safer !

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.

Filter\Validator\Require fails on nested property

6 participants