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): fixed output coercing for admin fields. #7617

Merged
merged 2 commits into from
Mar 19, 2021

Conversation

JatinDev543
Copy link
Contributor

@JatinDev543 JatinDev543 commented Mar 19, 2021

Fixes GRAPHQL-1100
We were getting output coercing error for fields like maxNodes in admin/state because internally that return value of type UInt64 but we were parsing it to Int. So we added UInt64 in GraphQL output types , Int64 in admin schema and changed the fields types accordingly in admin types.


This change is Reviewable

@JatinDev543 JatinDev543 requested a review from pawanrawal as a code owner March 19, 2021 07:43
@github-actions github-actions bot added the area/graphql Issues related to GraphQL support on Dgraph. label Mar 19, 2021
@JatinDev543 JatinDev543 changed the title fix(GRAPHQL): fixed output coercing for admin operations. fix(GRAPHQL): fixed output coercing for admin fields. Mar 19, 2021
Copy link
Contributor

@abhimanyusinghgaur abhimanyusinghgaur left a comment

Choose a reason for hiding this comment

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

:lgtm:

Reviewed 4 of 4 files at r1.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @jatindevdg and @pawanrawal)


graphql/admin/admin.go, line 57 at r1 (raw file):

-(2^64)

0
UInt64 can't give -ve values.


graphql/schema/completion.go, line 432 at r1 (raw file):

			return nil, valueCoercionError(v)
		}
	case "UInt64":

add a comment here that UInt64 is present only in admin schema.

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: 1 of 4 files reviewed, 2 unresolved discussions (waiting on @abhimanyusinghgaur and @pawanrawal)


graphql/admin/admin.go, line 57 at r1 (raw file):

Previously, abhimanyusinghgaur (Abhimanyu Singh Gaur) wrote…
-(2^64)

0
UInt64 can't give -ve values.

changed.


graphql/schema/completion.go, line 432 at r1 (raw file):

Previously, abhimanyusinghgaur (Abhimanyu Singh Gaur) wrote…

add a comment here that UInt64 is present only in admin schema.

added.

@JatinDev543 JatinDev543 merged commit 907662f into release/v21.03 Mar 19, 2021
@JatinDev543 JatinDev543 deleted the jatin/GRAPHQL-1097 branch March 19, 2021 10:46
aman-bansal pushed a commit that referenced this pull request Mar 25, 2021
Fixes GRAPHQL-1100
We were getting output coercing error for fields like maxNodes in admin/state because internally that return value of type UInt64 but we were parsing it to Int. So we added UInt64 in GraphQL output types , Int64 in admin schema and changed the fields types accordingly in admin types.
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