-
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
feat(GraphQL): Custom logic now supports DQL queries #6115
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 43 of 43 files at r1.
Reviewable status: all files reviewed, 5 unresolved discussions (waiting on @abhimanyusinghgaur)
graphql/resolve/query.go, line 122 at r1 (raw file):
args := query.Arguments() for k, v := range args { vStr, err := x.ScalarToString(v)
Add a comment about why do we convert all scalars to string.
graphql/resolve/query.go, line 127 at r1 (raw file):
k)) } vars["$"+k] = vStr
Also add a comment about this that Query in Alpha expects variable names to be prefixed with $
.
graphql/schema/rules.go, line 1226 at r1 (raw file):
typ.Name, field.Name)) } if _, err := gql.Parse(gql.Request{Str: dqlArg.Value.Raw}); err != nil {
We don't need to these for now.
graphql/schema/wrappers.go, line 1122 at r1 (raw file):
func (q *query) DQLQuery() string { if customDir := q.op.inSchema.customDirectives["Query"][q.Name()]; customDir != nil { if dqlArg := customDir.Arguments.ForName(dqlArg); dqlArg != nil {
if arg :=
x/x.go, line 1048 at r1 (raw file):
func ScalarToString(val interface{}) (string, error) { var str string
Do we need int8
, int16
etc. here or only bool
, string
and float64
? Can you verify once and maybe just keep this function in graphql
package itself
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: 36 of 42 files reviewed, 5 unresolved discussions (waiting on @pawanrawal)
graphql/resolve/query.go, line 122 at r1 (raw file):
Previously, pawanrawal (Pawan Rawal) wrote…
Add a comment about why do we convert all scalars to string.
Done.
graphql/resolve/query.go, line 127 at r1 (raw file):
Previously, pawanrawal (Pawan Rawal) wrote…
Also add a comment about this that Query in Alpha expects variable names to be prefixed with
$
.
Done.
graphql/schema/rules.go, line 1226 at r1 (raw file):
Previously, pawanrawal (Pawan Rawal) wrote…
We don't need to these for now.
Done.
graphql/schema/wrappers.go, line 1122 at r1 (raw file):
Previously, pawanrawal (Pawan Rawal) wrote…
if arg :=
Done.
x/x.go, line 1048 at r1 (raw file):
Previously, pawanrawal (Pawan Rawal) wrote…
Do we need
int8
,int16
etc. here or onlybool
,string
andfloat64
? Can you verify once and maybe just keep this function ingraphql
package itself
Done.
Fixes GraphQL-600. This PR adds DQL query (previously known as GraphQL+-) support in GraphQL through the `@custom` directive. (cherry picked from commit 117ac3c)
Fixes GraphQL-600. This PR adds DQL query (previously known as GraphQL+-) support in GraphQL through the `@custom` directive. (cherry picked from commit 117ac3c)
Fixes GraphQL-600.
This PR adds DQL query (previously known as GraphQL+-) support in GraphQL through the
@custom
directive. So, now it is possible to do something like this in the GraphQL schema:And then you can send a GraphQL query to execute the DQL, like this:
This change is