Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for name converter in filters #2751

Merged

Conversation

antograssiot
Copy link
Contributor

@antograssiot antograssiot commented Apr 22, 2019

Q A
Bug fix? no
New feature? yes
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets #2090
License MIT
Doc PR

Alternative to #2377, which I think is more complete and more accurate as some filters were not properly covered.

  • Boolean Filter
  • Date Filter
  • Exists Filter
  • Numeric Filter
  • Order Filter
  • Property Filter
  • Range Filter
  • Search Filter

The Group Filter is not supposed to be updated I think

I've added Behat tests against the schema as we do usually but also plain JSON content. I've made sure that the schemas are actually really validating what I'm looking for but at the end it is way more verbose, and I'm not sure it will be easier to maintain. I'll of course leave only a single version of the test, let me know what's your prefered one.

I've been checking and it works correctly with Swagger UI/ReDoc

TODO

  • Add tests to check that it is working with GraphQL ⚠️

Thanks for your feedback

cc @bendavies @soullivaneuh

@antograssiot antograssiot marked this pull request as ready for review April 22, 2019 08:55
@bendavies
Copy link
Contributor

awesome work!

Copy link
Member

@soyuka soyuka left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice!

@antograssiot
Copy link
Contributor Author

Do not merge that yet, I've just remembered that there are no tests for GraphQL yet.
Are name converters already supported @alanpoulain for property names ?

@soyuka
Copy link
Member

soyuka commented May 7, 2019

@antograssiot could we merge (pls rebase :D) this and do the graphql support in another pr?

@antograssiot
Copy link
Contributor Author

antograssiot commented May 7, 2019

Sure I'll rebase and merge tonight or tomorrow. GraphQl support is already in another PR anyway

@antograssiot antograssiot merged commit 1d5f020 into api-platform:master May 7, 2019
@antograssiot antograssiot deleted the filter-name-converter branch May 7, 2019 17:20
@antograssiot
Copy link
Contributor Author

GraphQL support will land with #2765

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.

None yet

4 participants