search

absinthe-graphql / absinthe_ecto

DEPRECATED: Use dataloader
MIT License
130 stars 36 forks source link

Call Ecto.assoc instead of Absinthe.Ecto.assoc #29

Closed sanderhahn closed 6 years ago

sanderhahn commented 6 years ago

Intends to fix resolution of has_many throughs. Not sure how to add a testcase for this.

sanderhahn commented 6 years ago

The conversation at https://github.com/absinthe-graphql/absinthe_ecto/pull/21 states: "we fall back to Ecto.assoc(parent, association) when no query function". However the code at line https://github.com/absinthe-graphql/absinthe_ecto/blob/master/lib/absinthe/ecto.ex#L178 seems to use the assoc macro from Absinthe.Ecto.

benwilson512 commented 6 years ago

Yup, this definitely seems to provide the intended functionality. What I'm trying to figure out is how this intended functionality would actually work.

The issue is that the query returned from resolve_query is used as part of the batch key https://github.com/sanderhahn/absinthe_ecto/blob/b0f04a25520b5e6b0ef286b1d7298e9c8af08f90/lib/absinthe/ecto.ex#L163

If resolve_query uses Ecto.assoc/2 then it's gonna be a different query for every parent, and so no actual batching will occur. @matthewlehner thoughts?