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

client side validation on address book #4183

Merged
merged 5 commits into from Apr 18, 2018

Conversation

4 participants
@Akarshit
Copy link
Contributor

Akarshit commented Apr 16, 2018

Resolves #4155
Impact: major
Type: bugfix

Issue

There was no client side validation on the addressBookForm.

Solution

Added client side validation by checking individual values. Right now only empty fields are checked.

Breaking changes

None

Testing

  1. Add product to the cart and go to checkout screen.
  2. Proceed without filling the address.
  3. Notice the validation errors.
  4. Fix some of them and again submit.
  5. Notice error on rest of them.
  6. Fix the errors and proceed.
  7. Address should be saved.

Akarshit added some commits Apr 17, 2018

@Akarshit

This comment has been minimized.

Copy link
Contributor Author

Akarshit commented Apr 17, 2018

@zenweasel Updated.

const requiredFields = ["country", "fullName", "address1", "postal", "city", "region", "phone"];
const validation = { messages: {} };
let isValid = true;
Object.keys(enteredAddress).forEach((key) => {

This comment has been minimized.

Copy link
@zenweasel

zenweasel Apr 17, 2018

Member

I wish this is a little more extendable, but since this will probably be replaced by some sort of React-Autoform form I am fine for now.

@zenweasel zenweasel self-requested a review Apr 17, 2018

@Akarshit

This comment has been minimized.

Copy link
Contributor Author

Akarshit commented Apr 17, 2018

This has been updated to fix the setState of unmounted component bug. Also there a bug that when country was set from geocoder API, region field was set to a object instead of string, so fixed that.

@zenweasel

This comment has been minimized.

Copy link
Member

zenweasel commented Apr 18, 2018

If I leave any field empty and hit save I get the following error:

addressBookForm.js:147 Uncaught TypeError: Cannot read property 'trim' of undefined
    at addressBookForm.js:147
    at Array.forEach (<anonymous>)
    at AddressBookForm._this.clientValidation (addressBookForm.js:146)
    at AddressBookForm._this.onSubmit (addressBookForm.js:178)
    at HTMLUnknownElement.callCallback (modules.js?hash=5c04a8da772c5f883666b6e5f38911f40f5ebf0c:246142)
    at Object.invokeGuardedCallbackDev (modules.js?hash=5c04a8da772c5f883666b6e5f38911f40f5ebf0c:246181)
    at Object.invokeGuardedCallback (modules.js?hash=5c04a8da772c5f883666b6e5f38911f40f5ebf0c:246038)
    at Object.invokeGuardedCallbackAndCatchFirstError (modules.js?hash=5c04a8da772c5f883666b6e5f38911f40f5ebf0c:246052)
    at executeDispatch (modules.js?hash=5c04a8da772c5f883666b6e5f38911f40f5ebf0c:246436)
    at executeDispatchesInOrder (modules.js?hash=5c04a8da772c5f883666b6e5f38911f40f5ebf0c:246458)
@zenweasel
Copy link
Member

zenweasel left a comment

Getting an error when validation runs

@spencern

This comment has been minimized.

Copy link
Member

spencern commented Apr 18, 2018

@zenweasel @Akarshit, I'm unable to reproduce the trim issue using either Avalara or TaxCloud, though my Avalar account seems to be suspended. I'm ready to merge this and if we have a bug related to validated addresses, we can address that in a future issue.

@spencern
Copy link
Member

spencern left a comment

I've been unable to reproduce @zenweasel's issue, though we should open a new issue for it if we can create solid reproduction steps.

@spencern spencern merged commit 4d0ff29 into release-1.11.0 Apr 18, 2018

9 of 10 checks passed

ci/circleci: snyk-security Your tests failed on CircleCI
Details
WIP ready for review
Details
ci/circleci: build Your tests passed on CircleCI!
Details
ci/circleci: docker-build Your tests passed on CircleCI!
Details
ci/circleci: docker-push Your tests passed on CircleCI!
Details
ci/circleci: dockerfile-lint Your tests passed on CircleCI!
Details
ci/circleci: eslint Your tests passed on CircleCI!
Details
ci/circleci: test-app Your tests passed on CircleCI!
Details
ci/circleci: test-unit Your tests passed on CircleCI!
Details
security/snyk No dependency changes
Details

@spencern spencern deleted the fix-4155-akarshit-address-client-validation branch Apr 18, 2018

@spencern spencern referenced this pull request Apr 19, 2018

Merged

Release 1.11.0 #4151

@Akarshit

This comment has been minimized.

Copy link
Contributor Author

Akarshit commented Apr 19, 2018

@spencern It's not an issue, he just forgot to pull the changes.
Sorry should have mentioned it here.

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.