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

Add enum value validation in input object #109

Merged
merged 1 commit into from
Nov 7, 2019

Conversation

imjma
Copy link
Contributor

@imjma imjma commented Sep 1, 2019

This will validate the enum value for the enum in input object

@coveralls
Copy link

Coverage Status

Coverage increased (+0.2%) to 92.321% when pulling 188217c on imjm:input-enum-err into 96ed341 on vektah:master.

@vvakame
Copy link
Collaborator

vvakame commented Oct 8, 2019

@imjma
Copy link
Contributor Author

imjma commented Oct 8, 2019

so far, for the value is not in the enum list, it will return 422 error for the enum in argument, but 200 for the enum in the input object.

eg.

type Query {
  Todo(orderBy: OrderBy, input: TestInput): Todo
}

enum OrderBy {
  ASC
  DESC
}

input TestInput {
  inputOrderBy: OrderBy
}

for 422 Unprocessable Entity error

query {
  Todo(orderBy: AAA) { id }
}

for 200

query($input: TestInput) {
  Todo(input: $input) { id }
}

{
  "input": {
    "inputOrderBy": AAA
  }
}

both will give errors, but I think it should also return same http status code?

@vvakame
Copy link
Collaborator

vvakame commented Oct 9, 2019

ah, make sense.

query A {
  enumInInput(input: {enum: "INVALID"})
}

query B($input: InputWithEnumValue) {
  enumInInput(input: $input)
}

I tried it by express-graphql, both returns 400 Bad Request and same response.

{"errors":[{"message":"Expected type EnumTest!, found \"INVALID\".","locations":[{"line":2,"column":29}]}]}

umm... This validation should execute in ValuesOfCorrectType rule.
https://github.com/graphql/graphql-js/blob/48ea2d3e8df5e4fbbdb5e0ce67c0a3c219b024f8/src/validation/rules/ValuesOfCorrectType.js#L118-L132
We wanna aligne to graphql-js implementations...

vvakame added a commit to 99designs/gqlgen that referenced this pull request Oct 9, 2019
Copy link
Collaborator

@vvakame vvakame left a comment

Choose a reason for hiding this comment

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

I investigated this change again.
so, I mentioned ValuesOfCorrectType rule before. but It's maybe not correct...

https://github.com/graphql/graphql-js/blob/7d24b050e2e321f2973a2de4d2834b8494a6945b/src/utilities/coerceInputValue.js#L197-L217
coerceInputValue is it and validator/vars.go is mapped to it.
currently, I think your implementations is maybe correct.

@vektah vektah merged commit 6b4a785 into vektah:master Nov 7, 2019
cgxxv pushed a commit to cgxxv/gqlgen that referenced this pull request Mar 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants