Skip to content

Commit

Permalink
Merge pull request #3275 from mozilla/issue-631, r=@chenba
Browse files Browse the repository at this point in the history
fix(password): Add inline confirm password validation
  • Loading branch information
vbudhram authored Nov 14, 2019
2 parents 4a2420f + 0d7e0c7 commit e098929
Show file tree
Hide file tree
Showing 5 changed files with 113 additions and 13 deletions.
34 changes: 30 additions & 4 deletions packages/fxa-content-server/app/scripts/views/sign_up_password.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
* License, v. 2.0. If a copy of the MPL was not distributed with this
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */

import { assign } from 'underscore';
import { assign, debounce } from 'underscore';
import AuthErrors from '../lib/auth-errors';
import CWTSOnSignupPasswordExperimentMixin from './mixins/cwts-on-signup-password-experiment-mixin';
import Cocktail from 'cocktail';
Expand All @@ -22,6 +22,10 @@ import Template from 'templates/sign_up_password.mustache';

const t = msg => msg;

const PASSWORD_INPUT_SELECTOR = '#password';
const VPASSWORD_INPUT_SELECTOR = '#vpassword';
const DELAY_BEFORE_PASSWORD_CHECK_MS = 1500;

const proto = FormView.prototype;
const SignUpPasswordView = FormView.extend({
template: Template,
Expand All @@ -32,6 +36,7 @@ const SignUpPasswordView = FormView.extend({

events: assign({}, FormView.prototype.events, {
'click .use-different': preventDefaultThen('useDifferentAccount'),
'keyup #vpassword': '_onConfirmPasswordKeyUp',
}),

useDifferentAccount() {
Expand Down Expand Up @@ -63,6 +68,14 @@ const SignUpPasswordView = FormView.extend({
canChangeAccount: !this.model.get('forceEmail'),
email: this.getAccount().get('email'),
});

// We debounce the password check function to give the password input
// some smarts. There will be a slight delay to show the tooltip which
// makes the experience less janky,
this.checkPasswordsMatchDebounce = debounce(
this._checkPasswordsMatch,
DELAY_BEFORE_PASSWORD_CHECK_MS
);
},

isValidEnd() {
Expand All @@ -75,7 +88,10 @@ const SignUpPasswordView = FormView.extend({

showValidationErrorsEnd() {
if (!this._doPasswordsMatch()) {
this.displayError(AuthErrors.toError('PASSWORDS_DO_NOT_MATCH'));
this.showValidationError(
this.$(VPASSWORD_INPUT_SELECTOR),
AuthErrors.toError('PASSWORDS_DO_NOT_MATCH')
);
}
},

Expand All @@ -97,16 +113,26 @@ const SignUpPasswordView = FormView.extend({
},

_getPassword() {
return this.getElementValue('#password');
return this.getElementValue(PASSWORD_INPUT_SELECTOR);
},

_getVPassword() {
return this.getElementValue('#vpassword');
return this.getElementValue(VPASSWORD_INPUT_SELECTOR);
},

_doPasswordsMatch() {
return this._getPassword() === this._getVPassword();
},

_onConfirmPasswordKeyUp() {
this.checkPasswordsMatchDebounce();
},

_checkPasswordsMatch() {
if (this._getVPassword() !== '' && this._getPassword() !== '') {
this.showValidationErrorsEnd();
}
},
});

Cocktail.mixin(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ describe('views/sign_up_password', () => {
beforeEach(() => {
sinon.stub(view, 'signUp').callsFake(() => Promise.resolve());
sinon.stub(view, 'tooYoung');
sinon.spy(view, 'displayError');
sinon.spy(view, 'showValidationError');
});

describe('password and vpassword do not match', () => {
Expand All @@ -155,10 +155,13 @@ describe('views/sign_up_password', () => {
assert.fail,
() => {
assert.isFalse(view.signUp.called);
assert.isTrue(view.displayError.calledOnce);
const displayedError = view.displayError.args[0][0];
assert.isTrue(view.showValidationError.calledOnce);
const displayedError = view.showValidationError.args[0][1];
assert.isTrue(
AuthErrors.is(displayedError, 'PASSWORDS_DO_NOT_MATCH')
AuthErrors.is(
displayedError,
AuthErrors.toError('PASSWORDS_DO_NOT_MATCH')
)
);
}
);
Expand All @@ -174,7 +177,7 @@ describe('views/sign_up_password', () => {
return Promise.resolve(view.validateAndSubmit()).then(() => {
assert.isTrue(view.tooYoung.calledOnce);
assert.isFalse(view.signUp.called);
assert.isFalse(view.displayError.called);
assert.isFalse(view.showValidationError.called);
});
});
});
Expand All @@ -196,7 +199,7 @@ describe('views/sign_up_password', () => {
'take-action-for-the-internet',
]);

assert.isFalse(view.displayError.called);
assert.isFalse(view.showValidationError.called);
});
});
});
Expand All @@ -216,6 +219,66 @@ describe('views/sign_up_password', () => {
});
});

describe('_checkPasswordsMatch', () => {
beforeEach(() => {
sinon.stub(view, 'signUp').callsFake(() => Promise.resolve());
sinon.stub(view, 'tooYoung');
sinon.spy(view, 'showValidationError');
});

describe('password and vpassword do not match', () => {
it('displays an error', () => {
view.$(Selectors.PASSWORD).val('password123123');
view.$(Selectors.VPASSWORD).val('different_password');
view.$(Selectors.AGE).val('21');

return Promise.resolve(view._checkPasswordsMatch()).then(() => {
assert.isTrue(view.showValidationError.calledOnce);
const displayedError = view.showValidationError.args[0][1];
assert.isTrue(
AuthErrors.is(
displayedError,
AuthErrors.toError('PASSWORDS_DO_NOT_MATCH')
)
);
});
});
});

describe('password and vpassword do match', () => {
it('displays an error', () => {
view.$(Selectors.PASSWORD).val('password123123');
view.$(Selectors.VPASSWORD).val('password123123');
view.$(Selectors.AGE).val('21');

return Promise.resolve(view._checkPasswordsMatch()).then(() => {
assert.isFalse(view.showValidationError.called);
});
});
});
});

describe('inline confirm password validation', () => {
beforeEach(() => {
sinon.stub(view, 'signUp').callsFake(() => Promise.resolve());
sinon.spy(view, 'showValidationError');
sinon.spy(view, 'checkPasswordsMatchDebounce');
});

describe('check password and vpassword', () => {
it('calls debounced password check', () => {
view.$(Selectors.PASSWORD).val('password123123');
view.$(Selectors.VPASSWORD).val('different_password');
view.$(Selectors.AGE).val('21');

return Promise.resolve(view.$(Selectors.VPASSWORD).keyup()).then(() => {
assert.isFalse(view.signUp.called);
assert.isTrue(view.checkPasswordsMatchDebounce.calledOnce);
});
});
});
});

describe('useDifferentAccount', () => {
it('navigates to `/` with the account', () => {
sinon.spy(view, 'navigate');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -422,6 +422,7 @@ module.exports = {
SUBMIT: 'button[type="submit"]',
TOOLTIP_AGE_REQUIRED: '#age ~ .tooltip',
TOS: '#fxa-tos',
TOOLTIP: '.tooltip',
VPASSWORD: '#vpassword',
},
SMS_LEARN_MORE: {
Expand Down
8 changes: 7 additions & 1 deletion packages/fxa-content-server/tests/functional/sign_up.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ const {
openVerificationLinkInSameTab,
switchToWindow,
testElementExists,
testElementTextEquals,
testElementTextInclude,
testElementValueEquals,
testErrorTextInclude,
Expand Down Expand Up @@ -421,7 +422,12 @@ registerSuite('signup', {
// wait five seconds to allow any errant navigation to occur
.then(noPageTransition(selectors.SIGNUP_PASSWORD.HEADER))
// the validation tooltip should be visible
.then(visibleByQSA(selectors.SIGNUP_PASSWORD.ERROR))
.then(
testElementTextEquals(
selectors.SIGNUP_PASSWORD.TOOLTIP,
'Passwords do not match'
)
)
);
},

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@ const {
testElementTextEquals,
testElementTextInclude,
testElementValueEquals,
testErrorTextInclude,
testIsBrowserNotified,
type,
visibleByQSA,
Expand Down Expand Up @@ -173,7 +172,12 @@ registerSuite('Firefox Desktop Sync v3 email first', {
selectors.SIGNUP_PASSWORD.ERROR_PASSWORDS_DO_NOT_MATCH
)
)
.then(testErrorTextInclude('Passwords do not match'))
.then(
testElementTextEquals(
selectors.SIGNUP_PASSWORD.TOOLTIP,
'Passwords do not match'
)
)

// fix the password mismatch
.then(type(selectors.SIGNUP_PASSWORD.VPASSWORD, PASSWORD))
Expand Down

0 comments on commit e098929

Please sign in to comment.