-
-
Notifications
You must be signed in to change notification settings - Fork 956
WhereFilter - Allow to build complex search query #2055
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
Conversation
|
Good ideas here also: https://postgrest.readthedocs.io/en/v5.0/api.html# |
dunglas
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can't wait to merge this feature. Some general feedback:
- We use
finalandprivateby default - Is it possible to share some code between the new inner filters and the existing filters?
src/Bridge/Doctrine/Orm/Filter/WhereFilter/Condition/AndCondition.php
Outdated
Show resolved
Hide resolved
src/Bridge/Doctrine/Orm/Filter/WhereFilter/Condition/AndCondition.php
Outdated
Show resolved
Hide resolved
src/Bridge/Doctrine/Orm/Filter/WhereFilter/Condition/ConditionInterface.php
Outdated
Show resolved
Hide resolved
|
Actually, do you think it's possible to embed any existing or future filter in the |
|
@dunglas, thinks it's a good idea, will try to use existing filters logic in this WhereFilter |
|
Strongly -1 for a SQL-like Frankenfilter. |
|
I like the name « FrankenFilter », but +1 to include it because:
However, we must write in bold in the documentation that this filter can be dangerous (DDOS vector) and should not be used for public APIs |
|
The fact that it's a free-form filter is already a big no-no for me. But now that you mention the security risks, that makes it even worse. But if we really want to do something like this, I wish we would stick to a format that is at least standardized, in other words OData. |
9a63903 to
d8f0a1c
Compare
|
@amenophis is this pull request still valid? |
|
Let's keep this as-is while we wait to reach a consensus over #2400 |
d8f0a1c to
39ee809
Compare
|
Just rebased it and close outdated comments. |
|
Hi, I was hoping to better understand #2400 composable issue and how it relates to this PR? These features seem to have been stale for just over a year. Are there issues in this update which could not be resolved or have you just not had the time needed for this feature? I only ask as the issue was brought to my attention by @alanpoulain while beginning another PR here #3924 |
|
see also api-platform/api-platform#1724 imo we need to pick or write a filter RFC and then work on it |
|
I built a WhereFilter (FilterLogic) that reuses the existing ORM Filter classes and combines them with compound logic. If you don't like the way it manipulates the QueryBuilder (i don't): it can be combined with QueryExpressionGeneratorInterface (see my comment on RFC Filter composition), resulting in this branch. |
|
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
|
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
Hi,
I open this PR to start discuss about implementing a filter like provided in Loopback (http://loopback.io/doc/en/lb3/Where-filter.html).
The idea is to allow building complex query from client side like following examples:
TO-DO:
I provide a first shoot implementation, wait for your comments !