generateIncludes leaves non-nullable fields out about graphql-sequelize HOT 9 CLOSED

Hmm, i wasn't actually aware that you would need to return fields other than those requested.
But in any case i have been considering whether we should remove attribute filtering at all or just fetch them all (since it continues to cause small subtle bugs).

from graphql-sequelize.

Yeah I may have misdiagnosed this. Taking a look now

On Tue, Oct 20, 2015 at 9:18 AM Mick Hansen [email protected]
wrote:

Hmm, i wasn't actually aware that you would need to return fields other
than those requested.
But in any case i have been considering whether we should remove attribute
filtering at all or just fetch them all (since it continues to cause small
subtle bugs).

—
Reply to this email directly or view it on GitHub
#45 (comment)
.

from graphql-sequelize.

Okay the real problem is that the map option is only on attributeFields, not on the resolver. So anything you map doesn't actually get resolved unless you manually specify a resolver.

I think we should either get rid of the map option, or somehow make resolver aware of mapped fields. The problem is, I don't see a good way to get that map object shared between the two without having to specify it in both places, which is not great.

from graphql-sequelize.

Gah.. my brain wasn't working properly.. This is very simple.. just need to add a resolver in attributeFields. I'll work on a pull request now.

from graphql-sequelize.

Ah yeah, mapped attributes will need a resolver or a virtual field (but with a virtual we don't really need map).

from graphql-sequelize.

See this branch for the problem (with failing test): master...idris:mapped-fields

Even after adding a resolver to the field, the field is never included in findOptions.attributes, so it never gets queried for. I can't think of a way to get the actual field name in resolver.js so we can automatically include mapped fields.

If we can't think of a solution for this, I think we should just get rid of the map functionality altogether, to avoid confusion. It's easy enough to do it manually.

from graphql-sequelize.

Or we disable findOptions.attributes filtering, it keeps giving me headaches.

from graphql-sequelize.

I think you at least want the attributes filtering in case there are large fields, like binary or JSONB fields that the user is never going to use.

map could also cause confusion and problems in the future. For example, if you try to map an associated field.

from graphql-sequelize.

Alright since you requested the feature initially i'll kill it for now.

from graphql-sequelize.