Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(GraphQL): This PR addd input coercion from single object to list and fix panic when we pass single ID in filter as a string. #7133

Merged
merged 12 commits into from
Dec 16, 2020

Conversation

JatinDev543
Copy link
Contributor

@JatinDev543 JatinDev543 commented Dec 14, 2020

Fixes GRAPHQL-745
Filter with id as a string was giving panic because we were expecting it to be of ["0x1"] type.

 query {
        queryAuthor(filter:{id: "0x1"}) {
          name
          }
        }

So, we added input coercion such that if we require an object of type list and the input object is scalar then we coerce it to list.


This change is Reviewable

@github-actions github-actions bot added the area/graphql Issues related to GraphQL support on Dgraph. label Dec 14, 2020
@netlify
Copy link

netlify bot commented Dec 14, 2020

✔️ Deploy preview for dgraph-docs ready!

🔨 Explore the source changes: 3072b4f

🔍 Inspect the deploy logs: https://app.netlify.com/sites/dgraph-docs/deploys/5fd75850785a7c00088106c3

😎 Browse the preview: https://deploy-preview-7133--dgraph-docs.netlify.app

Copy link
Contributor

@pawanrawal pawanrawal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 9 of 9 files at r1.
Reviewable status: 8 of 9 files reviewed, 2 unresolved discussions (waiting on @jatindevdg, @manishrjain, @MichaelJCompton, @pawanrawal, and @vvbalaji-dgraph)


go.sum, line 126 at r1 (raw file):

github.com/dgraph-io/gqlgen v0.13.2/go.mod h1:iCOrOv9lngN7KAo+jMgvUPVDlYHdf7qDwsTkQby2Sis=
github.com/dgraph-io/gqlparser/v2 v2.1.1 h1:OBGI6VR+WcegjDB3JdNPMpKpEtITHWzgmiCXtELWolI=
github.com/dgraph-io/gqlparser/v2 v2.1.1/go.mod h1:MYS4jppjyx8b9tuUtjV7jU1UFZK6P9fvO8TsIsQtRKU=

Run go mod tidy


graphql/schema/validation_rules.go, line 37 at r1 (raw file):

		}
		// ExpectedType.Elem will be not nil if it is of list type. Otherwise
		// it will be nil. So it's safe to say that value.Kind should be list

Rephrase the comment as

If the expected value is a list (ExpectedType.Elem != nil) && the value is not of list type, then we need to coerce the value to a list, otherwise, we can return here as we do below.

Copy link
Contributor Author

@JatinDev543 JatinDev543 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewable status: 7 of 9 files reviewed, 2 unresolved discussions (waiting on @manishrjain, @MichaelJCompton, @pawanrawal, and @vvbalaji-dgraph)


go.sum, line 126 at r1 (raw file):

Previously, pawanrawal (Pawan Rawal) wrote…

Run go mod tidy

manually removed this. go mod tidy has no effect.


graphql/schema/validation_rules.go, line 37 at r1 (raw file):

Previously, pawanrawal (Pawan Rawal) wrote…

Rephrase the comment as

If the expected value is a list (ExpectedType.Elem != nil) && the value is not of list type, then we need to coerce the value to a list, otherwise, we can return here as we do below.

done.

@JatinDev543 JatinDev543 merged commit 5e5f5ca into master Dec 16, 2020
@JatinDev543 JatinDev543 deleted the jatin/GRAPHQLInputArrayCoercion branch December 16, 2020 15:06
JatinDev543 added a commit that referenced this pull request Jan 15, 2021
…and fix panic when we pass single ID in filter as a string. (#7133)

Fixes GRAPHQL-745
Filter with id as a string was giving panic because we were expecting it to be of ["0x1"] type.

 query {
        queryAuthor(filter:{id: "0x1"}) {
          name
          }
        }
So, we added input coercion such that if we require an object of type list and the input object is scalar then we coerce it to list.

(cherry picked from commit 5e5f5ca)
JatinDev543 added a commit that referenced this pull request Jan 15, 2021
…and fix panic when we pass single ID in filter as a string. #7133 (#7306)

* fix(GraphQL): This PR addd input coercion from single object to list and fix panic when we pass single ID in filter as a string.  (#7133)

Fixes GRAPHQL-745
Filter with id as a string was giving panic because we were expecting it to be of ["0x1"] type.

 query {
        queryAuthor(filter:{id: "0x1"}) {
          name
          }
        }
So, we added input coercion such that if we require an object of type list and the input object is scalar then we coerce it to list.

(cherry picked from commit 5e5f5ca)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/graphql Issues related to GraphQL support on Dgraph.
Development

Successfully merging this pull request may close these issues.

2 participants