Implement consistent filter arg across connections

[seagyn]

This is a post v1 discussion which started on Slack: https://wp-graphql.slack.com/archives/C3NM1M291/p1594650031121400

The idea is #1384 is nice to have in core but it's very WP like which makes it harder for none WP developers to gain the correct context and the different ways to filter. The long term plan would be to add a filtering system into core that is consistent no matter you're querying against. There are plenty of edge cases so it's worth having a deep and mostly long discussion about it before any implementation takes place.

Initial ideas

Filtering by taxonomy

Array of category slugs:

{
  posts({ 
    filter: {
     categoryIn: [{ id: "category-b", idType: SLUG }, { id: "category-b", idType: SLUG }]
    }
  })
}

Single category:

{
  posts({ 
    filter: {
      categoryId: { id: "category-b", idType: SLUG }
    }
  })
}

Complex category query:

{
  posts({ 
    filter: {
      AND: [
        categoryIn: [{ id: "category-b", idType: SLUG }],
        categoryNotIn: [{ id: "category-a", idType: SLUG }]
      ]
    }
  })
}

By post information

A query you would probably never make but could:

{
  posts({ 
    filter: {
      AND: [
        idIn: [{id: 21, idType: DATABASE_ID}],
        titleNotIn: [ "some title" ],
        titleLike: "something",
        titleNotLike: "something else",
        contentContains: "some string",
      ]
    }
  })
}

Mixed queries with mixed relations

{
  posts({ 
    filter: {
      OR: [
        titleLike: "something",
        AND: [
          categoryIn: [{ id: "category-b", idType: SLUG }],
          idIn: [{id: 21, idType: DATABASE_ID}, {id: "/path", idType: URI}]
        ],
      ]
    }
  })
}

The idea here is that you can quickly build up filter queries without requiring knowledge of how WP_Query works.

[seagyn]

Here's the thread from slack:

Seagyn Davis 4 hours ago
@jason Bahl I see that Tax Query filter will go into core soon, is there an ETA on this? (edited)

Seagyn Davis 4 hours ago
Just moved the code into my project (from the repo) because it isn't on packagist. Happy to jump on to getting this in core if you'd like.

Jason Bahl 3 hours ago
It's less about moving the code into the core plugins and more about deciding on proper shape for arguments for filters. Filtering in GraphQL is largely still an un-standardized thing.
I'm not a big fan of how I did it in the Tax Query and Meta Query plugins. They try too hard to mirror WordPress internals and don't focus on declarative ways to filter data.
I like how Graph.Cool handles filters. You can compose filters using AND and OR for example.
Like here's an example query from their docs:

query($published: Boolean) {
  allPosts(filter: {
    OR: [{
      AND: [{
        title_in: ["My biggest Adventure", "My latest Hobbies"]
      }, {
        published: $published
      }]
    }, {
      id: "cixnen24p33lo0143bexvr52n"
    }]
  }) {
    id
    title
    published
  }
}

I suppose we could get the Tax Query in now though, and re-visit filters for 2.0 :thinking_face:

Seagyn Davis 3 hours ago
What would you prefer?

Jason Bahl 3 hours ago
I don't know yet 😭
I was hoping if I held off a bit, the wider GraphQL ecosystem would have some sort of sense of standards for filtering connections, but I've not seen anything like that arise. Everyone has vastly different approaches to them.
I'm a fan of the Schema being declarative and not being tied too closely to the backend. Like, you should be able to use the same query and it could work if your data was in WP and SQL database, or in some other data store. I don't like when things mirror internal WordPress decisions too much. You shouldn't have to know anything about WordPress to interact with a WPGraphQL Schema.
Like in an ideal world, a Native iOS engineer that's never touched WP should be able to use a WPGraphQL Schema without looking at even being told the underlying system is WordPress. Shouldn't have to know what "tax_query" even is. To me, you still have to "know WordPress" to even consider using tax_query arguments in WPGraphQL. That feels "wrong" to me for what we're trying to accomplish.

Jason Bahl 2 hours ago
But, I do think the value of "something" working sooner than later is better than the "pretty much nothing" we have for this today

Seagyn Davis 1 hour ago
:thinking_face: I guess it would be a matter of picking a pattern that makes sense. I do like the idea of making it easier to consume especially if the idea is that non-WP type folk would be interacting with it.

query($published: Boolean) {
  allPosts(filter: {
    category_in: ["main-category", "another-category"]
  }) {
    id
    title
    published
  }
}

could work from a tax perspective where category could be replaced by the taxonomy field name for custom taxes.

Seagyn Davis 1 hour ago
You'd probably need to make it a bit more dynamic than that though

Jason Bahl 1 hour ago
ya, for sure. There's a lot to think about, and I'm not quite in the right spot to dive deep on this mentally.

Jason Bahl 1 hour ago
but totally happy to have folks like you consider various options for discussion

Seagyn Davis 1 hour ago
category_in: [{term: "category", termBy: SLUG}]

Seagyn Davis 1 hour ago
This is me cowboy coding though.

Jason Bahl 1 hour ago
ya, now that we have id and idType fields elsewhere possibly we use that convention

Jason Bahl 1 hour ago
category_in: [{ id: "category" idType: SLUG }]

Jason Bahl 1 hour ago
that way you could use slug, database id, or global ID

Jason Bahl 1 hour ago
or whatever other unique identifiers a particular entity allowed for identification

Jason Bahl 1 hour ago
:thinking_face:

Jason Bahl 1 hour ago
but then handling composition is important too.
And doing things like or queries.
Like:
Query posts in Category A or Category B

Jason Bahl 1 hour ago
like do you do:

category_in: {
  OR: [
    { id: "category-a" idType: SLUG }, 
    { id: "category-b" idType: SLUG }
  ]
}

(edited)

Seagyn Davis 1 hour ago
That was my initial thought

Jason Bahl 1 hour ago
Query posts in Category A and Category B

category_in: {
  AND: [
    { id: "category-a" idType: SLUG }, 
    { id: "category-b" idType: SLUG }
  ]
}

Jason Bahl 1 hour ago
But what about Query Posts in Category B and NOT IN Category A

Jason Bahl 1 hour ago

category: {
  AND: {
    in: [{ id: "category-b", idType: SLUG }],
    notIn: [{ id: "category-a", idType: SLUG }]
  }
}

Jason Bahl 1 hour ago
:thinking_face: (edited)

Jason Bahl 1 hour ago
or is it:
{
posts({
filter: {
AND: [
categoryIn: [{ id: "category-b", idType: SLUG }],
categoryNotIn: [{ id: "category-a", idType: SLUG }],
someOtherTaxIn: [{ id: "other-tax-a", idType: SLUG }],
someOtherTaxNotIn: [{ id: "other-tax-b", idType: SLUG }]
]
}
})
}

Jason Bahl 1 hour ago
for posts:
In category B
AND
Not in category A
AND
in Other Tax A
AND
not in Other Tax B

Jason Bahl 1 hour ago
:thinking_face:

Jason Bahl 1 hour ago
gets tricky quick

Seagyn Davis 1 hour ago
Having a look at tax_query now and that's about as complex as it gets though

Seagyn Davis 1 hour ago
I think making the relation the outer most property makes sense from a query building POV.

Seagyn Davis 1 hour ago

{
  posts({ 
    filter: {
      AND: [
        categoryIn: [{ id: "category-b", idType: SLUG }],
        idIn: [{id: 21, idType: DATABASE_ID}]
      ]
    }
  })
}

Jason Bahl 34 minutes ago
and then consider filters in addition to taxonomies

Jason Bahl 32 minutes ago
like:

{
  posts({ 
    filter: {
      AND: [
        categoryIn: [{ id: "category-b", idType: SLUG }],
        idIn: [{id: 21, idType: DATABASE_ID}],
        titleNotIn: [ "some title" ],
        titleLike: "something",
        titleNotLike: "something else",
        contentContains: "some string",
      ]
    }
  })
}

Jason Bahl 32 minutes ago
etc

Jason Bahl 32 minutes ago
then mapping that to WP_Query or underlying SQL

Jason Bahl 32 minutes ago
lots to do to make it work well

Jason Bahl 32 minutes ago
naming conventions are hard enough to settle on

Jason Bahl 32 minutes ago
haha

Jason Bahl 31 minutes ago
then actually writing the code to map everything

Jason Bahl 31 minutes ago
gotta have a pretty good plan first before investing, I think

Jason Bahl 31 minutes ago
:thinking_face:

Jason Bahl 31 minutes ago
but I think we're at least brainstorming some good ideas here

Seagyn Davis 30 minutes ago
So I assume v1 is soon soon (south africanism for soon but I can't give a definite answer) (edited)

Jason Bahl 30 minutes ago
ya, and in my head these filters will be part of post-1.0

Jason Bahl 30 minutes ago
I see a backward compat way of modifying filters

Jason Bahl 29 minutes ago
we'll deprecate where args, and introduce filters and order as new top-level args on connections

Seagyn Davis 29 minutes ago
So can I do a PR to get taxQuery in (short term) and then add an issue for the greater filter idea? (edited)

Jason Bahl 29 minutes ago
ya, I think that works

Jason Bahl 29 minutes ago
we'll need some decent tests around taxQuery to merge to 1.0

Jason Bahl 28 minutes ago
if we have some decent tests so we can help ensure there aren't regressions, then I think the value of having it (even if not "perfect") outweights the value of not having it for now

Jason Bahl 28 minutes ago
and then we can work on improving filters with conventions we're discussing for post 1.0

Seagyn Davis 27 minutes ago
Cool, let me get cracking on it. Might ask for a slot to do some code pairing if I get stuck on something?

Jason Bahl 26 minutes ago
for sure

Jason Bahl 26 minutes ago
that sounds good indeed

Jason Bahl 26 minutes ago
open an issue to track it as well please 🙏

Jason Bahl 7 minutes ago
we might want to copy/paste this conversation in the issue instead of reference it. Since we're on the free Slack plan, this conversation could be archived by time we work on it

[stale]

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.

[stale]

This issue has been automatically closed because it has not had recent activity. If you believe this issue is still valid, please open a new issue and mark this as a related issue.

[stale]

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.

[stale]

This issue has been automatically closed because it has not had recent activity. If you believe this issue is still valid, please open a new issue and mark this as a related issue.