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

Fix #3733: email validation #3899

Merged
merged 2 commits into from
Mar 8, 2018
Merged

Conversation

Akarshit
Copy link
Contributor

@Akarshit Akarshit commented Mar 6, 2018

Resolves #3733
Impact: major
Type: bugfix

Issue

No client side validation was done on email.

Solution

Added a helper for email validation and doing the validation in the component before submitting.

Testing

  1. Setup a shop using Stripe as the payment provider
  2. Add a product to your cart and start a checkout as a guest user.
  3. On the first step of the checkout, click "Continue as guest",
  4. When prompted, submit the email form by clicking "Add email" without entering anything into the input field.
  5. Observe you get a error "Email is not valid"
  6. Try to enter a wrong email(a.a@a)
  7. Observe you get a error "Email is not valid"
  8. Enter a valid email.
  9. Observe no error.

@Akarshit Akarshit requested a review from jshimko March 6, 2018 18:07
Copy link
Contributor

@jshimko jshimko left a comment

Choose a reason for hiding this comment

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

Good to go. I just added a small tweak to the input change handler so that the validation error is cleared when the user starts typing in the field again.

@aaronjudd aaronjudd changed the base branch from master to release-1.9.0 March 7, 2018 16:42
@aaronjudd aaronjudd added this to the Release 1.9 milestone Mar 7, 2018
@Akarshit
Copy link
Contributor Author

Akarshit commented Mar 7, 2018

@jshimko Thanks :)

@spencern spencern merged commit c27d5ac into release-1.9.0 Mar 8, 2018
@spencern spencern deleted the fix-3733-email-validation branch March 8, 2018 17:33
@spencern spencern mentioned this pull request Mar 8, 2018
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.

5 participants