-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
graphql: Introduce @cascade
in GraphQL
#5511
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed 40 of 40 files at r1.
Reviewable status: all files reviewed, 5 unresolved discussions (waiting on @abhimanyusinghgaur and @MichaelJCompton)
graphql/e2e/common/query.go, line 1595 at r1 (raw file):
},{ "reputation": 4.6, "posts": []
You should try and fetch title
as well for this query and then because you have @cascade
on posts
it should not be returned.
graphql/resolve/query_test.yaml, line 658 at r1 (raw file):
gqlquery: | query { getAuthor(id: "0x1") @cascade {
Is @cascade
allowed in Dgraph on predicates without children? If not, then what happens in that case from GraphQL?
graphql/resolve/query_test.yaml, line 706 at r1 (raw file):
queryAuthor { dob posts @cascade {
Also add a test case where the directive is both on root and a field below.
graphql/schema/gqlschema.go, line 121 at r1 (raw file):
directive @custom(http: CustomHTTP) on FIELD_DEFINITION directive @remote on OBJECT | INTERFACE directive @cascade on FIELD
So the directive isn't allowed while querying interfaces?
graphql/schema/wrappers.go, line 1162 at r1 (raw file):
continue } if m.Cascade() && !f.Cascade() {
Are we adding the directive to the field here from the mutation so that it is applied to the query after?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: all files reviewed, 5 unresolved discussions (waiting on @MichaelJCompton and @pawanrawal)
graphql/e2e/common/query.go, line 1595 at r1 (raw file):
Previously, pawanrawal (Pawan Rawal) wrote…
You should try and fetch
title
as well for this query and then because you have@cascade
onposts
it should not be returned.
Done.
graphql/resolve/query_test.yaml, line 658 at r1 (raw file):
Previously, pawanrawal (Pawan Rawal) wrote…
Is
@cascade
allowed in Dgraph on predicates without children? If not, then what happens in that case from GraphQL?
It is allowed on such predicates. I checked, it didn't give any errors and worked fine.
graphql/resolve/query_test.yaml, line 706 at r1 (raw file):
Previously, pawanrawal (Pawan Rawal) wrote…
Also add a test case where the directive is both on root and a field below.
Done.
graphql/schema/gqlschema.go, line 121 at r1 (raw file):
Previously, pawanrawal (Pawan Rawal) wrote…
So the directive isn't allowed while querying interfaces?
Doesn't matter what type is returned I guess.
As all the queries/mutations are basically fields of type Query
and type Mutation
, so cascade can be applied on them, and any field in their selection set. So, anything where @cascade
is going to be applied is going to be a field.
graphql/schema/wrappers.go, line 1162 at r1 (raw file):
Previously, pawanrawal (Pawan Rawal) wrote…
Are we adding the directive to the field here from the mutation so that it is applied to the query after?
Yes, so that if @cascade
was given on mutation itself, then it should get applied for the query which gets executed to fetch the results of that mutation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed 3 of 3 files at r2.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @MichaelJCompton)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just to master for this one - it's a new feature, so it won't go into 20.03
Can you make sure you mark the milestone on the github issue and the fix version on the jira ticket at 20.07...
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @abhimanyusinghgaur)
graphql/resolve/mutation_query_test.yaml, line 157 at r2 (raw file):
gqlquery: | mutation { ADD_UPDATE_MUTATION @cascade {
nice
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @MichaelJCompton)
graphql/resolve/mutation_query_test.yaml, line 157 at r2 (raw file):
Previously, MichaelJCompton (Michael Compton) wrote…
nice
Done.
Fixes hypermodeinc#4789. Fixes #GRAPHQL-277.
Fixes #4789.
Fixes #GRAPHQL-277.
This change is
Docs Preview: