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

[WIP] Fix bug with incorrect errors for nested fields #271

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

perrin4869
Copy link

This is a work in progress, but I would appreciate your input.
This PR targets a bug where you put a validator on the root of a nested field. Let's say you want to ensure a range is valid. Assert that range.min < range.max by setting a validator on range. This is required, if for example, you have an array of ranges (as in our case).
The error message gets converted to an object with keys.
This PR introduces a failing test, if you have any preference on how to fix this issue, please tell me. Otherwise I can come up with a solution.
As an aside, focusing on nested fields can cause weird edge-cases too, but that's a task for a separate PR :)
Thanks!

@perrin4869 perrin4869 force-pushed the nested-field-level-validation-bugfix branch from 9ea1b69 to ebd3318 Compare August 27, 2019 16:34
@codecov
Copy link

codecov bot commented Aug 29, 2019

Codecov Report

Merging #271 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@          Coverage Diff          @@
##           master   #271   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files          12     12           
  Lines         555    557    +2     
  Branches      115    116    +1     
=====================================
+ Hits          555    557    +2
Impacted Files Coverage Δ
src/structure/setIn.js 100% <100%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4476c45...665462f. Read the comment docs.

@SpOOnman
Copy link

I can confirm this bug also affects me. I didn't test your solution but it looks like it can solve this issue.

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