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

Scalars validation fix (#1686) and errors extensions merging #1725 with #1727 #1728

Conversation

liqweed
Copy link

@liqweed liqweed commented Jan 6, 2020

This PR merges #1725 with #1727 to address #1686 .

Both PRs improves scalars validation by preserving the original scalar coercion error messages.

#1725 allows CoercingParseLiteralException, CoercingParseValueException & CoercingSerializeException to include extensions map that is carried to the serialized errors. That is useful for conveying specific information in particular scalars validation logic (e.g. the allowed pattern).

#1727 automatically populates the extensions map for all errors with the information that is available at the framework level (validationErrorType, queryPath, argument, value, requiredType, objectType). This information is not available at the scalars coercion point. Since this information is provided in the extensions map, this PR simplified error messages by stripping the messages from having that information concatenated to them.


This change is Reviewable

@bbakerman
Copy link
Member

Because of all the merge conflicts here I created

#1740

As a really rough attempt to loosely merge in your changes.

It does not compile but we needed to get a view of your intentions

Ideally you would merge master into this branch and then represent it so we can get a true picture of th e proposed changes

@bbakerman
Copy link
Member

Closing as its too old with these many merge coflicts

@bbakerman bbakerman closed this Nov 13, 2020
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.

None yet

2 participants