Filtering / Sorting a Parent Resource based off its associations value

[Reizar]

I couldn't quite work out if this can be done by reading the docs / playing around with some requests.

I'm looking for a way to filter / sort a parent resource based off a child association's field.

I think an example might be the easiest way to illustrate what I'm wanting to do.

Say you have 2 graphiti resources:

class ParentResource < ApplicationResource
  self.model = Parent
  attribute :something, :string

  has_one :child, resource: ChildResource
end

class ChildResource < ApplicationResource
  self.model = Child
  attribute :other_thing, :string

  attribute :parent_id, :integer_id

  belongs_to :parent, resource: ParentResource
end

Now on the frontend, using Spraypaint to query.
What I would like to do, is something like this:

Parent.includes('child').where({'child.other_thing': 'some_value'}).all()

This will return Parents, but set the child as undefined if the other_thing value doesn't match. However what I want, is some way to exclude the parent if it can't find that child.

I understand that the easiest way to achieve this would be to flip how I query the resources and query for the child. Eg:

Child.includes('parent').where({'other_thing': 'some_value'}).all()

The reason that I want to have some way of filtering / sorting the parent based off the child, is because I have a dropdown with a whole bunch of fields that a user can choose to have their results sorted by. This includes fields from the parent and the child.

For sorting I guess I could change the resource thats queried based off the sort field. Although it does complicate the UI having to accept different models when rendering certain components.
However when it comes to filtering, there are situations where I want to stack multiple filters and that wouldn't work.

[Reizar]

Aha, ok I think I worked this one out myself.

I just defined attributes on the parent resource and used a join. eg:

class ParentResource < ApplicationResource
  # .... Other code
  sort :other_thing, :string do |scope, direction|
    scope.joins(:child).order("child.other_thing" => direction).select("parent.*", "child.other_thing")
  end
end

Same idea should work with filters I believe.

Man Graphiti is awesome!

I would close this issue, but I just have one last question with this approach. The select seems to be required as I'm referencing the child value in the order clause. I had to have the parent.* added in to make sure it pulls the parent fields. However this ignores the fieldset requested by the client if the client wants sparse fields.
Is there anyway to pull the select fields within the sort block?

[carlobeltrame]

I have run into the same limitation with filtering multiple times by multiple independent users now.
When listing /events?include=participants and filtering by participant id, people expect to only get events with matching participants, instead of all events with only the included participants filtered. Further, people want to additionally include and filter by transitive sideloads, e.g. only fetch events in which user x has participation role y, via /events?include=participants.roles&filter[participants.id]=x&filter[participants.roles.type]=y.

In other words, people expect an INNER JOIN-like behaviour, while Graphiti implements a LEFT OUTER JOIN-like behaviour.

Flipping the relation and start querying at /participants or /roles is not a solution if we at the same time want to filter e.g. by only upcoming events: /events?filter[only_upcoming]=true&include=participants&filter[participants.id]=x.
I could manually add filters and sort clauses like suggested above, but doing this for every single filterable/sortable attribute in every (even transitive) combination seems like way too much work for the default expectation.
Related issue with the exact same problem and workaround example for filters: #459

Of course, changing this in Graphiti would be a major breaking change. But maybe we could add some way to make this configurable? Or maybe we could add a parameter like where_exists=participants,participants.roles or any other parameter name..?

What are your thoughts on this @jkeen, is this something that you could see Graphiti support? I could try to muster up a PR if requested.

[jkeen]

@carlobeltrame I think the concept is great, and I think you're right that the current behavior is surprising. Thanks for explaining this clearly. I haven't run into this issue personally (I think I just created some custom filters and went on my way) but in trying to figure out how this would best slot into the architecture, realizing that a deep filter without an include like /events?filter[participants.id]=x is basically just ignored entirely (!!) but a filter on an attribute is honored (like /events?filter[id]=x) definitely doesn't feel intuitive. Woof.

I've been really going to town on the beta branch tackling these sorts of sticking points and inconsistencies that IMO make graphiti harder to use than it could/should be, so I think the 2.0 release might be the right time for a behavior change like this.

I've been wrestling with the ergonomics and naming the past few days, and this is kinda where I ended up.

1. The default be changed so that all deep query filters narrow the results.
2. We add a self.deep_filters_narrow_results = true default flag at the ApplicationResource level that could be changed to false to let someone go back to the old behavior. (Although it really does feel like an oversight having it do what it does now, and I'm having a hard time imagining someone actually relying on the current behavior). Probably have a deprecation warning to remove the flag in 3.0
3. To restore the previous behavior in a per-query way, we could:
   a. add a new @ flag to the query parser like /events?filter[only_upcoming]=true&include=participants&filter[participants.id@included]=x (meaning only run the participants.id filter on the includes, not the primary query).
   b. add a prefix param to the filter like /events?filter[only_upcoming]=true&include=participants&filter[included][participants.id]=x to mean the same thing
   c. add filter_included and filter_primary as top level query params to specify an individual preference, with the new filter default applying to both included and primary by default. filter[only_upcoming]=true&include=participants&filter_included[participants.id]=x
   d. only ship the resource flag and behavior change for 2.0 until the requirements become a little clearer

Right now I'm kinda leaning 1, 2, and 3C?

[carlobeltrame]

Oh wow, thanks for the detailed response! TBH I wasn't expecting this much approval of the idea due to this being a breaking change and very long standing behaviour. But with a new major version on the horizon, I see there is no better time than now to change something like this.

Change 1 would be nice, since in my experience people expect this to be the default.
Change 2 is probably a good idea for backwards compatibility, although then we maybe also need a way to opt in per request so people have the option to migrate one API call at a time instead of a big bang?
Change 3a feels to me like it is quite hard to understand.
Changes 3b and 3c have a limitation with multi-layer nested includes: filter filters the top level resource and filter[included] or filter_included filters the deepest resource, but if there are intermediate resources these still can't be filtered. See next paragraph for an example of this problem.

Incidentally, in the past week, I played around on our application side with providing the expected behaviour through a where_exists parameter. It turned out to be not too hard to implement, but I also found some complications involving multi-layer nested relations. The basic problem goes like this: Suppose I have a Library resource which has_many :books and every book has_many :authors. Fetching /libraries?include=books.authors&filter[books.authors.birth_date][gt]=2020-01-01 will return all libraries and all books, which is probably not what I want - I probably want to get only libraries with books from young authors. With a hypotheical where_exists parameter, it could look like /libraries?include=books.authors&filter[books.authors.birth_date][gt]=2020-01-01&where_exists=books.authors. But then I could also want to fetch all libraries and include only books from young authors, and my where_exists just like your options 3b and 3c don't support this.

If we really wanted to solve this, we could require filter to have a first argument specifying which resource to filter, then a second argument specifying the (possibly nested) field to filter, and then the operator in a third argument:

• /libraries?include=books.authors&filter[books.authors][birth_date][gt]=2020-01-01 is the current behaviour of only filtering authors and returning all books and all libraries
• /libraries?include=books.authors&filter[books][authors.birth_date][gt]=2020-01-01 returns all libraries, includes only books which have any young author, and then includes all authors (young and old) of these books
• /libraries?include=books.authors&filter[something][books.authors.birth_date][gt]=2020-01-01 returns only libraries with books from young authors, and includes all books and all authors in these matching libraries
• Combinations of the above filter calls are possible to reduce each included resource. But the basic rule "if a book isn't included in the response, then this book's authors are also not included" should be respected IMO.

What do you think about this format? It's of course an even deeper breaking change, requiring all clients to change how they filter resources, but then again, technically so is your option 3c if clients want to preserve the current behaviour during the graphiti 2.0 upgrade. I don't have a solution for the something (the name of the top level resource) yet. Maybe filter[.][books.authors.birth_date][gt] or if you just omit the first arg filter[books.authors.birth_date][gt] (could lead to conflicts if an operator looks similar to a field).

[jkeen]

That's interesting, I like that syntax you outlined! That feels pretty intuitive.

What I've learned since I posted my first response proposal is that I can see now why this wasn't implemented from the jump: it has some really tricky parts and kinda goes against the grain of how Graphiti is structured, since every sideload/include has its own query that processes independently and having that affect the parent's results really cuts right across the architecture.

You're right about the intermediate levels, though. In my version ?include=books.authors&filter[books.authors.birth_date][gt]=... would narrow the libraries and filter the authors, but leaves books alone, so you get books back with no authors attached.

I tried to take a swing at the format I proposed and ran into a bunch of tricky spots even without realizing that intermediate layer miss, like trying to still honor sideload_allowlist and default_filter, realizing that adapters would have to add a method in order to support this behavior, and eventually realizing that the bigger project that would make all of this querying logic easier is restructuring the query builder to produce more of an AST-type thing that the adapters could then translate down to whatever they target, kinda like what Arel does for ActiveRecord, which I think would have additional benefits for Graphiti (debugging, allowing adapter testing against a shared contract, and separating filter syntax from semantics)

Now knowing what I know… I'd prefer to target this after 2.0, because my feeling is that this is going to get complicated, potentially break everyone's custom adapters, and will be really tricky to get right. 2.0 is already doing a lot, so I'd like to get that out first.

[carlobeltrame]

Yes, that's more in line with what I expected. It's not as small a change as it seems at first, and I assumed there were some internal reasons in graphiti details which I don't know which led to the current behaviour being the default.

I don't suppose you could make a prediction how this would most likely be implemented, if at all, in a future graphiti version? I could add the functionality on our application side and test it out for a while (months / years), but if at all possible I'd like to avoid manually introducing such a breaking change now and then again later when graphiti starts supporting something like this in a completely different way.

If you can't make any prediction, I might just add my relatively simple where_exists extension for now, since that is not a breaking change, relatively simple to understand and limited in the number of supported edge cases (only supports filtering ALL resources at once, not opt-in / opt-out for individual includes). We don't currently use default_filter or sideload_allowlist to my knowledge. Once graphiti starts introducing a feature like this, I can then deprecate the where_exists param and advise clients how to migrate to the official graphiti feature.

[jkeen]

@carlobeltrame I would if I could! The AST thing sounds like the right thing to do long term, but it also seems like a real science project. 😅

The one prediction I'll make: whatever lands will most likely be an extension of filter[...] (maybe like you outlined?) rather than a new top-level param. So where_exists would be most likely be a deprecate-and-remove for you down the line. But knowing that, and given you don't need sideload_allowlist or default_filter I don't see a downside for you to use your extension until the bigger changes land.