I18n for validation error messages #2878
Conversation
| @@ -66,95 +66,6 @@ mutation dogOperation { | |||
| id | |||
| } | |||
| } | |||
| """ | |||
There was a problem hiding this comment.
Moved to the Lone Anonymous test file - it's hard to find tests via spec number
There was a problem hiding this comment.
I don't like the naming. It should always be a descriptive name.
| @@ -1,133 +0,0 @@ | |||
| package graphql.validation | |||
There was a problem hiding this comment.
Moved into rule test, it's hard to find tests via spec number
| @@ -1,26 +0,0 @@ | |||
| package graphql.validation | |||
There was a problem hiding this comment.
Moved into rule test, it's hard to find tests via spec number
| @@ -21,52 +21,4 @@ query madDog(\$dogCommand: DogCommand){ | |||
| validationErrors.size() == 1 | |||
| validationErrors.get(0).getValidationErrorType() == ValidationErrorType.VariableTypeMismatch | |||
| } | |||
|
|
|||
There was a problem hiding this comment.
Moved into rule test, it's hard to find tests via spec number
| @@ -1,47 +0,0 @@ | |||
| package graphql.validation.rules | |||
There was a problem hiding this comment.
Renamed to ScalarLeavesTest, diff makes it look like a deletion
| @@ -196,7 +196,7 @@ class CoercingTest extends Specification { | |||
| def er = customScalarSchema.execute(ei) | |||
| then: | |||
| er.errors.size() == 1 | |||
| er.errors[0].message.contains("parseLiteral message") | |||
There was a problem hiding this comment.
Coercing error messages remain the same as before, not i18n-ised.
dondonz
changed the title
| * | ||
| * @return a result object that indicates how this operation went | ||
| */ | ||
| public static List<ValidationError> validate(GraphQLSchema graphQLSchema, Document parsedDocument) { |
There was a problem hiding this comment.
This is a breaking change. Leave the old and use Locale.getDefault() in it and delegate to the old method
There was a problem hiding this comment.
That's right, I'll change this
| * | ||
| * @return a result object that indicates how this operation went | ||
| */ | ||
| public static List<ValidationError> validate(GraphQLSchema graphQLSchema, Document parsedDocument, Predicate<Class<?>> rulePredicate) { | ||
| public static List<ValidationError> validate(GraphQLSchema graphQLSchema, Document parsedDocument, Predicate<Class<?>> rulePredicate, Locale locale) { |
There was a problem hiding this comment.
Same breaking change- leave the old, use Locale.getDefault() and win
| return msgArguments.toArray(); | ||
| } | ||
|
|
||
| public I18nMsg argumentAt(int index, Object argument) { |
There was a problem hiding this comment.
I probably named this method but on reflectionaddArgumentAt is a better name
| then: | ||
| executionInput.query == "{ q }" | ||
| executionInput.cacheControl != null | ||
| executionInput.locale == null | ||
| executionInput.locale == Locale.ENGLISH | ||
| executionInput.dataLoaderRegistry != null | ||
| executionInput.variables == [:] | ||
| } |
There was a problem hiding this comment.
Add a test to show it now defaults when build but not set
|
Defaulting the locale is a small regression for anyone that did not assume this behavior. Maybe this should be noted in the release notes? See https://github.com/vert-x3/vertx-web/pull/2244/files#r931731703 |
|
Hi @yeikel thanks for letting me know. I'll amend the release notes |
Adding i18n messages for validation errors. We are starting with validation errors, as they occur most frequently.
This PR follows on from @bbakerman's i18n PR #1473, I have copied the core i18n ideas into this PR, brought it up to date and expanded tests.
Coming up in the next PR - German and Portuguese translations