Support comma-separated and plus-separated multi-value filters

There is some talk about implementing a shorthand filtering syntax for simple cases (like operator equals and conjunction AND). The relevant class should have an expand() method ready to do this.

Thanks for the contribution!

Any progress on this @jcnventura? Did you get any chance to implement this?

I'm quite interested in getting this done. Development sugar, sweet! Let's try to get it further.

Attaching the patch just for the initial review. If everyone is happy (especially d.org test bot), I'll supply the tests. Also, the docs will have to be enhanced.

Oh right, the patch :)

This is looking good. You are moving in the right direction. I agree that we want to treat the , as an OR. We need to make sure that all the scenarios are covered for filter[field_a]=x,y,z:

• Field is single value: The filter matches if the single value is either x, y or x.
• Field is multiple value: The filter matches if one of the multiple values is either x, y or x.

We should also implement filter[field_a]=x+y+z (I'll update the IS):

• Field is single value: The filter never matches since the only value cannot be x and y and z.
• Field is multiple value: The filter matches if it contains all the 3 values x and y and z. Note that field_a can contain other values and still match.

1. +++ b/src/Routing/Param/Filter.php
   @@ -83,20 +83,34 @@ class Filter extends JsonApiParamBase {
   +      $filter_items = explode(',', $filter_item);
   

   Let's create a constant for that ','. Maybe something like static::MULTI_OR_SEPARATOR.

2. +++ b/src/Routing/Param/Filter.php
   @@ -83,20 +83,34 @@ class Filter extends JsonApiParamBase {
   +      $filter_item = [
   +        static::VALUE_KEY => $filter_items,
   +        static::OPERATOR_KEY => 'IN',
   +      ];
   

   Let's create the full form (add the path) right away and return early, instead of letting the next step enhance it.

We should also implement filter[field_a]=x+y+z

Once again you beat me - I was thinking about the same, but as a follow-up :) Glad we're thinking the same way.

I'd say that it makes sense to deliver #2852259: Add comprehensive test coverage to \Drupal\jsonapi\Routing\Param\Filter ahead of this story to make sure we're not breaking anything. So going to focus on tests coverage first, then get back to this one.

The issue mentioned above was fixed, but we got another one which ideally should be done ahead of this one: #2878654: [FEATURE] Implement validation of filter params. I'm going to work on the mentioned one and then get back to this story.

This feature is now unblocked and can be taken into the development.

Made some progress, would like to share the results and concerns:
1. There are no issues with OR condition. ?filter[field]=a,b,c works as expected for both single and multiple value fields.
2. There is a big issue with AND condition. To get it working for a field with multiple values as described in #7, we need to change the way query is built, which will likely bring an effort which doesn't worth it's value. This stackoverflow issue describes the issue and the first answer explains my concerns. I'll tell you even more: currently it's not possible to build this query properly even with full canonical condition form (unless I'm missing anything obvious).

Having this said, my proposal would be to get back to the original issue idea to handle only short form for OR condition, which is what most of developers are looking for.

How does it sound to you guys?

@Spleshka can we just use whatever solution EntityQuery provides? I like the idea of JSON API just being a syntax to write EntityQueries.

Let's have:

/jsonapi/foo/bar?filter[field]=a+b+c

Become (I believe that is the syntax):

/jsonapi/foo/bar?filter[field][condition][path]=field&filter[field][condition][conjunction]=AND&filter[field][condition][value][]=a&filter[field][condition][value][]=b&filter[field][condition][value][]=c&filter[field][condition][operator]=%3D"

We should leave the querying to core's EntityQuery.

What do you think?

I like the idea of JSON API just being a syntax to write EntityQueries.

So am I, that's exactly what I've mentioned the other day in slack channel. However, the resuling sql query doesn't make sense, because it will always return null result, no matter if we query a single or a multiple value field. This issue is described in details here.

#12 says the OR case already works. The AND case does not yet work.

IMHO it's fine to only do the OR case here. Especially because http://jsonapi.org/recommendations/#filtering does not even mention the AND case. In principle, we should only support AND by using + if we can get the JSON API spec updated to say exactly that.
For now, an AND can be simulated like #13.

What do you think, @e0ipso?

@Spleshka: in any case, this is going to need test coverage. #2874601: refactor(QueryBuilder): Improve testability/maintainability most likely made this easier to test now :)

This actually seems pretty similar to #2968891: Allow extreme shorthand filtering: ?filter[promote]=1.

I'm changing my mind here, and I'm having second thoughts on the shorthand syntaxes. They seem to go in detriment of using a client library to construct the URLs, which I believe is the best practice. So we have 2 options:

1️⃣ Manual URL construction in the consumer + support for multiple different syntax to do the same thing.
2️⃣ Library verbose URL construction in the consumer + support for a single syntax.

Nowadays I'm leaning towards 2️⃣ although in the past I introduced the concept of the shorthand syntax.

Thoughts?

• If you're talking in general (i.e. "we should also remove any shorthand notations we've introduced in the past, like ?filter[field][value]=2 and ?filter[field]=2), I think 2️⃣ destroys DX. Because even simple filtering then is made complex.
• If you're talking about this issue in specific, I feel less strongly about this; I can see the argument for not supporting this in JSON API itself, but shifting the burden to the client for the complex filtering cases, then I think that's defensible.

I see your point. I somehow wished that people didn't construct URLs manually, but followed links instead and/or filters manually but use a library instead.

Perhaps my wishes are not realistic and making things more difficult is not the way to incentivize those best practices. So I may agree with your point.

Either way, this seems like a nice-to-have feature we can do in the 8.x-2.x branch.