-
Notifications
You must be signed in to change notification settings - Fork 2.8k
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
allow query root in mutation response (close #1583) #1759
Conversation
Resolve Conflicts (Cherry Pick): server/tests-py/queries/graphql_mutation/insert/permissions/teardown.yaml
Review app for commit 235d5c0 deployed to Heroku: https://hge-ci-pull-1759.herokuapp.com |
mutationRootTy = G.NamedType "mutation_root" | ||
|
||
subscriptionRootTy :: G.NamedType | ||
subscriptionRootTy = G.NamedType "subscription_root" |
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.
I think better naming would be queryRootName
, mutationRootName
etc.
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.
Since those functions give G.NamedType
I name them as .....Ty
. ...Name
is suitable if functions give G.Name
.
@@ -349,12 +357,12 @@ mkGCtx tyAgg (RootFlds flds) insCtxMap = | |||
colTys = Set.toList $ Set.fromList $ map pgiType $ | |||
lefts $ Map.elems fldInfos | |||
mkMutRoot = | |||
mkHsraObjTyInfo (Just "mutation root") (G.NamedType "mutation_root") Set.empty . | |||
mkHsraObjTyInfo (Just "mutation root") mutationRootTy Set.empty . |
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.
Maybe we can move out the descriptions also, so that can be reused?
@@ -152,11 +153,11 @@ mergeMutRoot a b = | |||
|
|||
mkNewEmptyMutRoot :: VT.ObjTyInfo | |||
mkNewEmptyMutRoot = VT.ObjTyInfo (Just "mutation root") |
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.
reuse query/mutation descriptions ?
@@ -31,6 +31,7 @@ buildTx userInfo gCtx sqlCtx fld = do | |||
, orderByCtx | |||
, insCtxMap | |||
, sqlCtx | |||
, queryRootResolver | |||
) $ case opCxt of |
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.
can we change this code to be more readable? the last argument to run convert, the case matching can be separate?
there is also a typo in opCxt
. it should be opCtx
.
Resolve Conflicts: server/src-lib/Hasura/GraphQL/Resolve/Insert.hs server/src-lib/Hasura/GraphQL/Resolve/Introspect.hs server/src-lib/Hasura/GraphQL/Resolve/Mutation.hs server/src-lib/Hasura/RQL/DML/Mutation.hs server/src-lib/Hasura/RQL/DML/Returning.hs server/src-lib/Hasura/RQL/DML/Select.hs
Deploy preview for hasura-docs ready! Built with commit 76ac0c4 |
Review app for commit 5f50a4f deployed to Heroku: https://hge-ci-pull-1759.herokuapp.com |
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.
LGTM
Resolve Conflicts: server/src-lib/Hasura/GraphQL/Resolve/Insert.hs server/src-lib/Hasura/GraphQL/Schema.hs server/src-lib/Hasura/RQL/DML/Mutation.hs
Review app for commit 08e465f deployed to Heroku: https://hge-ci-pull-1759.herokuapp.com |
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.
Removed unqualified query fields in examples. We can add a separate example for the query in mutation use case later on.
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.
docs LGTM
Review app for commit a1370f6 deployed to Heroku: https://hge-ci-pull-1759.herokuapp.com |
Resolve Conflicts: server/src-lib/Hasura/GraphQL/Context.hs server/src-lib/Hasura/GraphQL/Resolve.hs server/src-lib/Hasura/GraphQL/Resolve/Context.hs server/src-lib/Hasura/GraphQL/Resolve/Mutation.hs server/src-lib/Hasura/GraphQL/Resolve/Select.hs
…graphql-engine into issue-1583-query-mut-resp-new
Review app for commit d91591c deployed to Heroku: https://hge-ci-pull-1759.herokuapp.com |
Resolve Conflicts: docs/graphql/manual/mutations/delete.rst server/src-lib/Hasura/GraphQL/Explain.hs server/src-lib/Hasura/GraphQL/Resolve.hs server/src-lib/Hasura/GraphQL/Resolve/Context.hs server/src-lib/Hasura/GraphQL/Resolve/Insert.hs server/src-lib/Hasura/GraphQL/Resolve/Mutation.hs server/src-lib/Hasura/GraphQL/Resolve/Select.hs server/src-lib/Hasura/GraphQL/Schema.hs server/src-lib/Hasura/RQL/DML/Returning.hs server/src-lib/Hasura/RQL/DML/Select.hs server/src-lib/Hasura/RQL/DML/Select/Internal.hs
Review app for commit 7c84b7c deployed to Heroku: https://hge-ci-pull-1759.herokuapp.com |
Resolve Conflicts: server/src-lib/Hasura/RQL/DML/Select/Internal.hs server/src-lib/Hasura/RQL/Types/Error.hs
Review app for commit 76ac0c4 deployed to Heroku: https://hge-ci-pull-1759.herokuapp.com |
Closing this PR due to merge conflicts. |
Description
Support queries in mutation response.
Example:-
Response:-
Affected components
Related Issues
close #1583