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 #4502: null check in email validation #4520

Conversation

pardyot
Copy link

@pardyot pardyot commented Aug 8, 2018

Resolves #4502
Impact: minor
Type: bugfix

Issue

When login/register was clicked without filling username and password the spinner runs forever.
email.trim() was being called without putting a null check on email and email was passed on as null.

Solution

Added null check before calling email.trim(). This causes the validation to fail in case of empty email.

Breaking changes

None.

Testing

  1. As an anonymous consumer, add a product to your cart.
  2. Checkout.
  3. Leaving email and password fields empty, click Register.
  4. Observe the validation errors.
  5. Repeat the same for Sign In

@zenweasel zenweasel self-requested a review August 9, 2018 05:44
@zenweasel
Copy link
Collaborator

@pardyot I can't fast-forward to test your branch. Can you update to the latest release-1.15.0?

@pardyot
Copy link
Author

pardyot commented Aug 22, 2018

Sure!

@pardyot
Copy link
Author

pardyot commented Aug 23, 2018

@zenweasel - Merged release-1.15.0 and updated the PR.

Copy link
Collaborator

@zenweasel zenweasel left a comment

Choose a reason for hiding this comment

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

Tested. Verified fixed. @pardyot Thanks so much!! 🎉

@aldeed aldeed changed the base branch from release-1.15.0 to release-1.16.0 August 23, 2018 19:26
@aldeed aldeed merged commit 53013ac into reactioncommerce:release-1.16.0 Aug 23, 2018
@spencern spencern mentioned this pull request Sep 5, 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.

None yet

3 participants