Fix Login Page Validations (#134)

- On form submission check for frontend validations first before submitting. - On form submission error should be displayed above and below the field VAN-345
2021-02-10 21:32:00 +05:00
parent 4a3fa3fdee
commit 43b3cbd515
7 changed files with 143 additions and 144 deletions
--- a/src/login/LoginFailure.jsx
+++ b/src/login/LoginFailure.jsx
@@ -12,6 +12,7 @@ import {
  INACTIVE_USER,
  INCORRECT_EMAIL_PASSWORD,
  INTERNAL_SERVER_ERROR,
+  INVALID_FORM,
  NON_COMPLIANT_PASSWORD_EXCEPTION,
 } from './data/constants';
 import messages from './messages';
@@ -39,6 +40,13 @@ const LoginFailureMessage = (props) => {
      );
      break;
    }
+    case FORBIDDEN_REQUEST:
+      errorList = (
+        <li key={FORBIDDEN_REQUEST}>
+          {intl.formatMessage(messages['login.rate.limit.reached.message'])}
+        </li>
+      );
+      break;
    case INACTIVE_USER: {
      const supportLink = (
        <Alert.Link href={context.supportLink}>
@@ -62,13 +70,6 @@ const LoginFailureMessage = (props) => {
      );
      break;
    }
-    case FORBIDDEN_REQUEST:
-      errorList = (
-        <li key={FORBIDDEN_REQUEST}>
-          {intl.formatMessage(messages['login.rate.limit.reached.message'])}
-        </li>
-      );
-      break;
    case INTERNAL_SERVER_ERROR:
      errorList = (
        <li key={INTERNAL_SERVER_ERROR}>
@@ -76,6 +77,14 @@ const LoginFailureMessage = (props) => {
        </li>
      );
      break;
+    case INVALID_FORM:
+      errorList = (
+        <>
+          {context.email && <li key={`${INVALID_FORM}-email`}>{context.email}</li>}
+          {context.password && <li key={`${INVALID_FORM}-password`}>{context.password}</li>}
+        </>
+      );
+      break;
    case FAILED_LOGIN_ATTEMPT: {
      const resetLink = (
        <Alert.Link href="/reset">
--- a/src/login/LoginPage.jsx
+++ b/src/login/LoginPage.jsx
@@ -14,7 +14,8 @@ import { faSpinner } from '@fortawesome/free-solid-svg-icons';

 import AccountActivationMessage from './AccountActivationMessage';
 import ConfirmationAlert from '../common-components/ConfirmationAlert';
-import { loginRequest } from './data/actions';
+import { loginRequest, loginRequestFailure } from './data/actions';
+import { INVALID_FORM } from './data/constants';
 import { getThirdPartyAuthContext } from '../common-components/data/actions';
 import { loginErrorSelector, loginRequestSelector } from './data/selectors';
 import { thirdPartyAuthContextSelector } from '../common-components/data/selectors';
@@ -42,9 +43,6 @@ class LoginPage extends React.Component {
        email: '',
        password: '',
      },
-      emailValid: false,
-      passwordValid: false,
-      formValid: false,
      institutionLogin: false,
      isSubmitted: false,
    };
@@ -68,13 +66,29 @@ class LoginPage extends React.Component {
    this.setState(prevState => ({ institutionLogin: !prevState.institutionLogin }));
  }

+  handleOnBlur = () => {
+    if (this.state.isSubmitted) {
+      this.setState({ isSubmitted: false });
+    }
+  }
+
  handleSubmit = (e) => {
    e.preventDefault();
    this.setState({ isSubmitted: true });

-    const params = (new URL(document.location)).searchParams;
-    const { email, password, formValid } = this.state;
+    const { email, password } = this.state;
+    const emailValidationError = this.validateEmail(email);
+    const passwordValidationError = this.validatePassword(password);

+    if (emailValidationError !== '' || passwordValidationError !== '') {
+      this.props.loginRequestFailure({
+        errorCode: INVALID_FORM,
+        context: { email: emailValidationError, password: passwordValidationError },
+      });
+      return;
+    }
+
+    const params = (new URL(document.location)).searchParams;
    const payload = { email, password };
    const next = params.get('next');
    const courseId = params.get('course_id');
@@ -84,52 +98,30 @@ class LoginPage extends React.Component {
    if (courseId) {
      payload.course_id = courseId;
    }
-    if (!formValid) {
-      this.validateInput('email', payload.email);
-      this.validateInput('password', payload.password);
-      return;
-    }
    this.props.loginRequest(payload);
  }

-  validateInput(inputName, value) {
-    let { emailValid, passwordValid } = this.state;
+  validateEmail(email) {
    const { errors } = this.state;
    const regex = new RegExp(/^([\w.%+-]+)@([\w-]+\.)+([\w]{2,})$/i, 'i');

-    switch (inputName) {
-      case 'email':
-        emailValid = regex.test(value);
-        errors.email = emailValid ? '' : null;
-        break;
-      case 'password':
-        passwordValid = value.length > 0;
-        errors.password = passwordValid ? '' : null;
-        break;
-      default:
-        break;
+    if (email === '') {
+      errors.email = this.props.intl.formatMessage(messages['email.validation.message']);
+    } else if (!regex.test(email)) {
+      errors.email = this.props.intl.formatMessage(messages['email.format.validation.message']);
+    } else {
+      errors.email = '';
    }
-
-    this.setState({
-      errors,
-      emailValid,
-      passwordValid,
-    }, this.validateForm);
+    this.setState({ errors });
+    return errors.email;
  }

-  handleOnChange(e) {
-    this.setState({
-      [e.target.name]: e.target.value,
-      isSubmitted: false,
-    });
-    this.validateInput(e.target.name, e.target.value);
-  }
+  validatePassword(password) {
+    const { errors } = this.state;
+    errors.password = password.length > 0 ? '' : this.props.intl.formatMessage(messages['password.validation.message']);

-  validateForm() {
-    const { emailValid, passwordValid } = this.state;
-    this.setState({
-      formValid: emailValid && passwordValid,
-    });
+    this.setState({ errors });
+    return errors.password;
  }

  handleCreateAccountLinkClickEvent() {
@@ -157,6 +149,7 @@ class LoginPage extends React.Component {
  }

  render() {
+    const { email, errors, password } = this.state;
    const {
      intl, submitState, thirdPartyAuthContext, thirdPartyAuthApiStatus,
    } = this.props;
@@ -194,7 +187,7 @@ class LoginPage extends React.Component {
                />
              )}
              {this.props.loginError ? <LoginFailureMessage loginError={this.props.loginError} /> : null}
-              {this.state.isSubmitted ? window.scrollTo({ left: 0, top: 0, behavior: 'smooth' }) : null}
+              {submitState === DEFAULT_STATE && this.state.isSubmitted ? window.scrollTo({ left: 0, top: 0, behavior: 'smooth' }) : null}
              {activationMsgType && <AccountActivationMessage messageType={activationMsgType} />}
              {this.props.forgotPassword.status === 'complete' && !this.props.loginError ? (
                <ConfirmationAlert email={this.props.forgotPassword.email} />
@@ -215,24 +208,23 @@ class LoginPage extends React.Component {
                  for="email"
                  name="email"
                  type="email"
-                  invalid={this.state.errors.email !== ''}
-                  invalidMessage={this.state.email === '' ? intl.formatMessage(messages['email.validation.message']) : intl.formatMessage(messages['email.format.validation.message'])}
-                  placeholder="username@domain.com"
-                  value={this.state.email}
-                  onChange={(e) => this.handleOnChange(e)}
+                  invalid={errors.email !== ''}
+                  invalidMessage={errors.email}
+                  value={email}
                  helpText={intl.formatMessage(messages['email.help.message'])}
-                  className="w-100"
+                  onBlur={(e) => { this.handleOnBlur(); this.validateEmail(e.target.value); }}
+                  onChange={(e) => this.setState({ email: e.target.value, isSubmitted: false })}
                />
                <AuthnValidationFormGroup
                  label={intl.formatMessage(messages['password.label'])}
                  for="password"
                  name="password"
                  type="password"
-                  invalid={this.state.errors.password !== ''}
-                  invalidMessage={intl.formatMessage(messages['password.validation.message'])}
-                  placeholder=""
-                  value={this.state.password}
-                  onChange={(e) => this.handleOnChange(e)}
+                  invalid={errors.password !== ''}
+                  invalidMessage={errors.password}
+                  value={password}
+                  onBlur={(e) => { this.handleOnBlur(); this.validatePassword(e.target.value); }}
+                  onChange={(e) => this.setState({ password: e.target.value, isSubmitted: false })}
                />
                <LoginHelpLinks page={LOGIN_PAGE} />
                <Hyperlink className="field-link mt-0 mb-3 small" destination={this.getEnterPriseLoginURL()}>
@@ -289,6 +281,7 @@ LoginPage.propTypes = {
  intl: intlShape.isRequired,
  loginError: PropTypes.objectOf(PropTypes.any),
  loginRequest: PropTypes.func.isRequired,
+  loginRequestFailure: PropTypes.func.isRequired,
  loginResult: PropTypes.shape({
    redirectUrl: PropTypes.string,
    success: PropTypes.bool,
@@ -324,5 +317,6 @@ export default connect(
  {
    getThirdPartyAuthContext,
    loginRequest,
+    loginRequestFailure,
  },
 )(injectIntl(LoginPage));
--- a/src/login/data/constants.js
+++ b/src/login/data/constants.js
@@ -1,6 +1,7 @@
 // Login Error Codes
 export const INACTIVE_USER = 'inactive-user';
 export const INTERNAL_SERVER_ERROR = 'internal-server-error';
+export const INVALID_FORM = 'invalid-form';
 export const NON_COMPLIANT_PASSWORD_EXCEPTION = 'NonCompliantPasswordException';
 export const FORBIDDEN_REQUEST = 'forbidden-request';
 export const FAILED_LOGIN_ATTEMPT = 'failed-login-attempt';
--- a/src/login/tests/LoginFailure.test.jsx
+++ b/src/login/tests/LoginFailure.test.jsx
@@ -8,6 +8,7 @@ import {
  FORBIDDEN_REQUEST,
  INACTIVE_USER,
  INTERNAL_SERVER_ERROR,
+  INVALID_FORM,
  NON_COMPLIANT_PASSWORD_EXCEPTION,
 } from '../data/constants';

@@ -97,6 +98,24 @@ describe('LoginFailureMessage', () => {
    expect(loginFailureMessage.find('#login-failure-alert').first().text()).toEqual(expectedMessage);
  });

+  it('should match invalid form error message', () => {
+    props = {
+      loginError: {
+        errorCode: INVALID_FORM,
+        context: { email: 'Please enter your Email.', password: 'Please enter your Password.' },
+      },
+    };
+
+    const loginFailureMessage = mount(
+      <IntlProvider locale="en">
+        <IntlLoginFailureMessage {...props} />
+      </IntlProvider>,
+    );
+
+    const expectedMessage = 'We couldn\'t sign you in.Please enter your Email.Please enter your Password.';
+    expect(loginFailureMessage.find('#login-failure-alert').first().text()).toEqual(expectedMessage);
+  });
+
  it('should match direct render of error message', () => {
    const errorMessage = 'Email or password is incorrect.';
    props = {
--- a/src/login/tests/LoginPage.test.jsx
+++ b/src/login/tests/LoginPage.test.jsx
@@ -4,23 +4,23 @@ import renderer from 'react-test-renderer';
 import { mount } from 'enzyme';
 import configureStore from 'redux-mock-store';

-import { getConfig } from '@edx/frontend-platform';
-import { IntlProvider, injectIntl } from '@edx/frontend-platform/i18n';
-import * as analytics from '@edx/frontend-platform/analytics';
 import CookiePolicyBanner from '@edx/frontend-component-cookie-policy-banner';
-import LoginPage from '../LoginPage';
-import { RenderInstitutionButton } from '../../common-components';
-import { PENDING_STATE } from '../../data/constants';
+import { getConfig } from '@edx/frontend-platform';
+import * as analytics from '@edx/frontend-platform/analytics';
+import { IntlProvider, injectIntl } from '@edx/frontend-platform/i18n';

 import LoginFailureMessage from '../LoginFailure';
-
-const IntlLoginFailureMessage = injectIntl(LoginFailureMessage);
+import LoginPage from '../LoginPage';
+import { loginRequest, loginRequestFailure } from '../data/actions';
+import { RenderInstitutionButton } from '../../common-components';
+import { PENDING_STATE } from '../../data/constants';

 jest.mock('@edx/frontend-platform/analytics');

 analytics.sendTrackEvent = jest.fn();
 analytics.sendPageEvent = jest.fn();

+const IntlLoginFailureMessage = injectIntl(LoginFailureMessage);
 const IntlLoginPage = injectIntl(LoginPage);
 const mockStore = configureStore();

@@ -147,22 +147,62 @@ describe('LoginPage', () => {
  });

  it('updates the error state for invalid email', () => {
-    const errorState = { email: null, password: '' };
-    const loginPage = mount(reduxWrapper(<IntlLoginPage {...props} />));
+    const errorState = { email: 'Please enter your Email.', password: '' };
+    store.dispatch = jest.fn(store.dispatch);
+
+    const loginPage = (mount(reduxWrapper(<IntlLoginPage {...props} />))).find('LoginPage');

    loginPage.find('input#password').simulate('change', { target: { value: 'test', name: 'password' } });
    loginPage.find('button.btn-brand').simulate('click');

-    expect(loginPage.find('LoginPage').state('errors')).toEqual(errorState);
+    expect(loginPage.state('errors')).toEqual(errorState);
+    expect(store.dispatch).toHaveBeenCalledWith(
+      loginRequestFailure({ errorCode: 'invalid-form', context: errorState }),
+    );
  });

  it('updates the error state for invalid password', () => {
-    const errorState = { email: '', password: null };
-    const loginPage = mount(reduxWrapper(<IntlLoginPage {...props} />));
+    const errorState = { email: '', password: 'Please enter your Password.' };
+    store.dispatch = jest.fn(store.dispatch);
+
+    const loginPage = (mount(reduxWrapper(<IntlLoginPage {...props} />))).find('LoginPage');

    loginPage.find('input#email').simulate('change', { target: { value: 'test@example.com', name: 'email' } });
    loginPage.find('button.btn-brand').simulate('click');
-    expect(loginPage.find('LoginPage').state('errors')).toEqual(errorState);
+
+    expect(loginPage.state('errors')).toEqual(errorState);
+    expect(store.dispatch).toHaveBeenCalledWith(
+      loginRequestFailure({ errorCode: 'invalid-form', context: errorState }),
+    );
+  });
+
+  it('should update the error message on focus out', () => {
+    const errorState = { email: 'Please enter your Email.', password: 'Please enter your Password.' };
+    const loginPage = (mount(reduxWrapper(<IntlLoginPage {...props} />))).find('LoginPage');
+
+    loginPage.find('input#password').simulate('blur', { target: { value: '', name: 'password' } });
+    loginPage.find('input#email').simulate('blur', { target: { value: '', name: 'email' } });
+
+    expect(loginPage.state('errors')).toEqual(errorState);
+
+    errorState.email = 'The email address you\'ve provided isn\'t formatted correctly.';
+
+    // Enter email with invalid format
+    loginPage.find('input#email').simulate('blur', { target: { value: 'invalid-email', name: 'email' } });
+    expect(loginPage.state('errors')).toEqual(errorState);
+  });
+
+  it('submits login request for valid email and password values', () => {
+    store.dispatch = jest.fn(store.dispatch);
+    const loginPage = (mount(reduxWrapper(<IntlLoginPage {...props} />))).find('LoginPage');
+
+    loginPage.find('input#email').simulate('change', { target: { value: 'test@example.com' } });
+    loginPage.find('input#password').simulate('change', { target: { value: 'password' } });
+    loginPage.find('button.btn-brand').simulate('click');
+
+    expect(store.dispatch).toHaveBeenCalledWith(
+      loginRequest({ email: 'test@example.com', password: 'password' }),
+    );
  });

  it('should match url after redirection', () => {
--- a/src/login/tests/snapshots/LoginPage.test.jsx.snap
+++ b/src/login/tests/snapshots/LoginPage.test.jsx.snap
@@ -34,7 +34,7 @@ exports[`LoginPage should match TPA provider snapshot 1`] = `
        className="m-0"
      >
        <div
-          className="form-group w-100"
+          className="form-group"
        >
          <span />
          <input
@@ -52,12 +52,6 @@ exports[`LoginPage should match TPA provider snapshot 1`] = `
            value=""
          />
          <span />
-          <strong
-            className="invalid-feedback"
-            id="email-invalid-feedback"
-          >
-            Please enter your Email.
-          </strong>
        </div>
        <div
          className="form-group"
@@ -78,12 +72,6 @@ exports[`LoginPage should match TPA provider snapshot 1`] = `
            value=""
          />
          <span />
-          <strong
-            className="invalid-feedback"
-            id="password-invalid-feedback"
-          >
-            Please enter your Password.
-          </strong>
        </div>
        <button
          className="mt-2 field-link small"
@@ -224,7 +212,7 @@ exports[`LoginPage should match default section snapshot 1`] = `
        className="m-0"
      >
        <div
-          className="form-group w-100"
+          className="form-group"
        >
          <span />
          <input
@@ -242,12 +230,6 @@ exports[`LoginPage should match default section snapshot 1`] = `
            value=""
          />
          <span />
-          <strong
-            className="invalid-feedback"
-            id="email-invalid-feedback"
-          >
-            Please enter your Email.
-          </strong>
        </div>
        <div
          className="form-group"
@@ -268,12 +250,6 @@ exports[`LoginPage should match default section snapshot 1`] = `
            value=""
          />
          <span />
-          <strong
-            className="invalid-feedback"
-            id="password-invalid-feedback"
-          >
-            Please enter your Password.
-          </strong>
        </div>
        <button
          className="mt-2 field-link small"
@@ -416,7 +392,7 @@ exports[`LoginPage should match forget password alert message snapshot 1`] = `
        className="m-0"
      >
        <div
-          className="form-group w-100"
+          className="form-group"
        >
          <span />
          <input
@@ -434,12 +410,6 @@ exports[`LoginPage should match forget password alert message snapshot 1`] = `
            value=""
          />
          <span />
-          <strong
-            className="invalid-feedback"
-            id="email-invalid-feedback"
-          >
-            Please enter your Email.
-          </strong>
        </div>
        <div
          className="form-group"
@@ -460,12 +430,6 @@ exports[`LoginPage should match forget password alert message snapshot 1`] = `
            value=""
          />
          <span />
-          <strong
-            className="invalid-feedback"
-            id="password-invalid-feedback"
-          >
-            Please enter your Password.
-          </strong>
        </div>
        <button
          className="mt-2 field-link small"
@@ -570,7 +534,7 @@ exports[`LoginPage should match pending button state snapshot 1`] = `
        className="m-0"
      >
        <div
-          className="form-group w-100"
+          className="form-group"
        >
          <span />
          <input
@@ -588,12 +552,6 @@ exports[`LoginPage should match pending button state snapshot 1`] = `
            value=""
          />
          <span />
-          <strong
-            className="invalid-feedback"
-            id="email-invalid-feedback"
-          >
-            Please enter your Email.
-          </strong>
        </div>
        <div
          className="form-group"
@@ -614,12 +572,6 @@ exports[`LoginPage should match pending button state snapshot 1`] = `
            value=""
          />
          <span />
-          <strong
-            className="invalid-feedback"
-            id="password-invalid-feedback"
-          >
-            Please enter your Password.
-          </strong>
        </div>
        <button
          className="mt-2 field-link small"
@@ -761,7 +713,7 @@ exports[`LoginPage should show error message 1`] = `
        className="m-0"
      >
        <div
-          className="form-group w-100"
+          className="form-group"
        >
          <span />
          <input
@@ -779,12 +731,6 @@ exports[`LoginPage should show error message 1`] = `
            value=""
          />
          <span />
-          <strong
-            className="invalid-feedback"
-            id="email-invalid-feedback"
-          >
-            Please enter your Email.
-          </strong>
        </div>
        <div
          className="form-group"
@@ -805,12 +751,6 @@ exports[`LoginPage should show error message 1`] = `
            value=""
          />
          <span />
-          <strong
-            className="invalid-feedback"
-            id="password-invalid-feedback"
-          >
-            Please enter your Password.
-          </strong>
        </div>
        <button
          className="mt-2 field-link small"
--- a/src/register/tests/RegistrationPage.test.jsx
+++ b/src/register/tests/RegistrationPage.test.jsx
@@ -222,10 +222,6 @@ describe('./RegistrationPage.js', () => {
  });

  it('should dispatch fetchRegistrationForm on ComponentDidMount', () => {
-    store = mockStore({
-      ...initialState,
-    });
-
    store.dispatch = jest.fn(store.dispatch);
    mount(reduxWrapper(<IntlRegistrationPage {...props} />));
    expect(store.dispatch).toHaveBeenCalledWith(fetchRegistrationForm());