Skip to content
This repository has been archived by the owner on Oct 6, 2020. It is now read-only.

fix(Formbot): Validate fields when setting state directly #48

Merged
merged 1 commit into from
Jul 26, 2019

Conversation

erikshestopal
Copy link
Contributor

Summary

Bug scenario:

  1. Blur out of a required field, e.g zip
  2. Use Formbot#setValues to update the field programmatically
  3. We expect the validation message to disappear but it doesn't
    formbot

The issue was twofold:

  1. We weren't running validations as a callback after values were set with #setValues
  2. #updateField in #validateAllFields wasn't being called with{ validated: false }, which caused #validateField to bail when it reached this condition:
const fieldState = this.state.fields[field] || {};
// field was already validated the first time around and
// thus bailed out of subsequent validations
if (fieldState.validated) { validations
  resolve();
  return;
}

@erikshestopal erikshestopal requested a review from a team July 26, 2019 01:38
@ghost ghost requested review from cehsu and choochootrain July 26, 2019 01:38
@codecov
Copy link

codecov bot commented Jul 26, 2019

Codecov Report

Merging #48 into master will not change coverage.
The diff coverage is 0%.

Impacted file tree graph

@@          Coverage Diff           @@
##           master     #48   +/-   ##
======================================
  Coverage    68.5%   68.5%           
======================================
  Files          22      22           
  Lines         435     435           
  Branches       92      92           
======================================
  Hits          298     298           
  Misses        107     107           
  Partials       30      30
Impacted Files Coverage Δ
src/Form/Formbot.js 3.12% <0%> (ø) ⬆️

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 88f197c...7a10502. Read the comment docs.

3 similar comments
@codecov
Copy link

codecov bot commented Jul 26, 2019

Codecov Report

Merging #48 into master will not change coverage.
The diff coverage is 0%.

Impacted file tree graph

@@          Coverage Diff           @@
##           master     #48   +/-   ##
======================================
  Coverage    68.5%   68.5%           
======================================
  Files          22      22           
  Lines         435     435           
  Branches       92      92           
======================================
  Hits          298     298           
  Misses        107     107           
  Partials       30      30
Impacted Files Coverage Δ
src/Form/Formbot.js 3.12% <0%> (ø) ⬆️

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 88f197c...7a10502. Read the comment docs.

@codecov
Copy link

codecov bot commented Jul 26, 2019

Codecov Report

Merging #48 into master will not change coverage.
The diff coverage is 0%.

Impacted file tree graph

@@          Coverage Diff           @@
##           master     #48   +/-   ##
======================================
  Coverage    68.5%   68.5%           
======================================
  Files          22      22           
  Lines         435     435           
  Branches       92      92           
======================================
  Hits          298     298           
  Misses        107     107           
  Partials       30      30
Impacted Files Coverage Δ
src/Form/Formbot.js 3.12% <0%> (ø) ⬆️

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 88f197c...7a10502. Read the comment docs.

@codecov
Copy link

codecov bot commented Jul 26, 2019

Codecov Report

Merging #48 into master will not change coverage.
The diff coverage is 0%.

Impacted file tree graph

@@          Coverage Diff           @@
##           master     #48   +/-   ##
======================================
  Coverage    68.5%   68.5%           
======================================
  Files          22      22           
  Lines         435     435           
  Branches       92      92           
======================================
  Hits          298     298           
  Misses        107     107           
  Partials       30      30
Impacted Files Coverage Δ
src/Form/Formbot.js 3.12% <0%> (ø) ⬆️

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 88f197c...7a10502. Read the comment docs.

@erikshestopal erikshestopal requested a review from kylealwyn July 26, 2019 01:39
Copy link
Contributor

@kylealwyn kylealwyn left a comment

Choose a reason for hiding this comment

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

Nice find

@erikshestopal erikshestopal merged commit 944fdd1 into master Jul 26, 2019
@erikshestopal erikshestopal deleted the fix/formbot-validations branch July 26, 2019 17:30
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants