search

Vincit / objection-graphql

GraphQL schema generator for objection.js
MIT License
307 stars 34 forks source link

Columns for virtual attributes not selected #9

Closed mattleff closed 7 years ago

mattleff commented 7 years ago

When a model has virtual attributes that depend on columns that aren't being selected the virtual attributes cause the resolver to return null. This can be "fixed" by adding return null here. Would it be possible to make _filterForSelects() optional?

koskimas commented 7 years ago

Sorry, I have totally missed your pull requests. Thank you for them! I'll take a look at them as soon as I can. I'm on a vacation so it may take a day or five 😀

mattleff commented 7 years ago

@koskimas (from here)

An option like selectFiltering sounds like a reasonable solution to this.

I just saw your edit now. I'll work on adding this to this pull.

mattleff commented 7 years ago

From here:

I don't like the idea that users need to remember things like "if I use fragments, I need to call this arbitrary selectFiltering(false) method".

^ I totally agree. Is there a way that we could avoid a similar What?! for virtual attributes? Would it be better if the method was named like allowVirtualAttributes(true)? How can we communicate that there's a connection between selectFiltering and virtual attributes that reference columns? Wdyt about logging a warning if a model has virtual attributes and select filtering is not disabled?

mattleff commented 7 years ago

@koskimas I think this is ready for review now.

koskimas commented 7 years ago

Awesome! Thank you so much! I'll release a new version with your additions as soon as I get to my computer.