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

feat(schema) do not run custom_entity_check if fields are invalid #3848

Merged
merged 1 commit into from Oct 15, 2018

Conversation

Projects
None yet
3 participants
@hishamhm
Copy link
Member

hishamhm commented Oct 11, 2018

This is inspired by #3846.

For the generic checkers we had so far, running them on invalid fields and
reporting everything at once was okay, and avoided the "get one error, fix
thing, get another error" experience. But for a custom checker, it's safer to
only run it on valid fields (especially thinking of making things less
error-prone for third-party plugin authors).

The commit is lengthy because the errors table is now passed along
and is used to check whether there are recorded errors by the time
entity checkers run; a side effect of this is that the code is now
simpler, and produces fewer intermediate tables.

feat(schema) do not run custom_entity_check if fields are invalid
For the generic checkers we had so far, running them on invalid fields and
reporting everything at once was okay, and avoided the "get one error, fix
thing, get another error" experience. But for a custom checker, it's safer to
only run it on valid fields (especially thinking of making things less
error-prone for third-party plugin authors).

The commit is lengthy because the `errors` table is now passed along
and is used to check whether there are recorded errors by the time
entity checkers run; a side effect of this is that the code is now
simpler, and produces fewer intermediate tables.
@james-callahan

This comment has been minimized.

Copy link
Contributor

james-callahan commented Oct 12, 2018

Is there a way that a custom validator could specify run_with_missing_fields/run_with_invalid_fields itself?

@hishamhm

This comment has been minimized.

Copy link
Member Author

hishamhm commented Oct 12, 2018

@james-callahan not at this point, I didn't want to expose this unless it becomes actually needed, to avoid feature bloat/documentation overhead

@hishamhm hishamhm merged commit 475d361 into next Oct 15, 2018

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@hishamhm hishamhm deleted the feat/custom-entity-checks-only-with-valid-fields branch Oct 15, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.