-
Notifications
You must be signed in to change notification settings - Fork 23
Feat: Filtering wildcard and multiple value #125
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
base: master
Are you sure you want to change the base?
Conversation
@spike83 This is a pretty big change. I'll try to find some time to look at it this weekend. One thing I will say now is that because filtering syntax is not defined in the JSONAPI spec (beyond saying you must use the |
@csantero I added some documentation on filtering. |
@spike83 I've thought about this some more. Because your implementation (which looks very good, btw) uses its own non-standardized micro-syntax, I don't want to commit all users to it without an opt-in. It would make it harder to adopt other syntaxes in the future, such as some of the ones discussed in this forum thread. I want to leave the decision of what filtering syntax to use up to the user, but I also don't want to waste the work you've done in this PR. I propose that you move these changes into a new implementation of |
@csantero Thanks for your review. I think most of the addition should be default behavior like multiple values and automatic transformation to DateTime ranges. I agree that the wildcard filter syntax is not standard and would extract this to a separate location. I've seen this forum thread and wonder about how to integrate such things. As you could see in #124 there are other needs to evaluate query parameters and manipulate the predicate or the queryable as well. To make the system as flexible as possible I would propose to make a chain of "consumers" of the url parameters wich can be configured by the customer. So if anyone would implement any specific syntax he can intercept the chain and "consume" the parameters which belong to his syntax. All parameters wich are not consumed will be passed to the next class in chain. The last Instance in chain is always the If you agree, I would do this:
What do you think about that? |
I know I haven't given you feedback on #124 yet, and I apologize. Preliminarily I'll say that your concept of the resolver seems to differ from what I had in mind, and I'd like to get that sorted out over in that PR separately from this discussion. The big problem with filtering I've always had in my JSONAPI.NET consuming project, is that I need pluggability on a per-resource type basis. Not all attributes on every resource type are filterable, but the current implementation based around I'd like to see filtering changed to be something that is configured on a per-endpoint basis. For example, you might have a |
@spike83 The smallest possible change would to be enable overriding the global filtering transformer at configuration-time, which would allow us to the work you've done in this PR sooner. We can then discuss adding support for per-resource filtering in a separate PR. |
This change introduces
@csantero would be great if you could have a look and merge if ok. Thanks.