Fix apollo-server null issue due to missing columns

[zachrdz]

By submitting a PR to this repository, you agree to the terms within the Code of Conduct. Please see the contributing guidelines for how to create and submit a high-quality PR for this repo.

Description

Recent apollo-server-core changes always define a field resolver (see enablePluginsForSchemaResolvers function, apollo-server issue #3988) so '!field.resolve' is not a good check for columns. Because of this, the query is not built correctly and does not included the requested columns, leading to all fields being null when using ApolloServer. Instead use parentTypeNode.constructor.name.

Current condition

if (fieldConfig.sqlColumn || !field.resolve)

Proposed condition

if (fieldConfig.sqlColumn || ['GraphQLObjectType', 'GraphQLInterfaceType'].includes(parentTypeNode.constructor.name))

References

Reference to existing PR in draft state

Apollo Server breaking change

Testing

Modified an existing test for User > favNums resolver to add ignoreAll since it does not use join-monster functionality.

• This change adds test coverage for new/changed/fixed functionality

Checklist

• I have added documentation for new/changed functionality in this PR via comments and by updating the change log
• All active GitHub checks for tests, formatting, and security are passing

[zachrdz]

This build should be passing, but it looks like pull requests from forks aren't allowed to run the linter which is causing the build to fail. May someone with permission please manually run this step?

[AlessandroFerrariFood]

Someone can merge this very important fix?
Thanks!

[alexbbt]

Looks good, I will merge this, just one thing to fix

[alexbbt]

I've got a PR to fix the Linting and Task action, Ill have you rebase one more time after this to pick it up: #449

[alexbbt]

I have no idea why the actions are not working still, but we can go ahead and merge this anyways. Our publish of the new version may be delayed as our publish pipeline recently broke. We will get this published asap.

[alexbbt]

So it turns out this is because your fork branch name is master. wearerequired/lint-action#13 (comment). These should work on other branches

[zachrdz]

Awesome, thank you!

[alexbbt]

@zachrdz this should be deployed now! Let us know if you have any problems with it!