Skip to content
This repository has been archived by the owner on Apr 3, 2019. It is now read-only.

Commit

Permalink
fix(email-first): Ensure "Mistyped email" links work as expected.
Browse files Browse the repository at this point in the history
If a relier specified an email for the email-first flow, users
that clicked on "Mistyped email" saw no visible change in the
screen.

In reality, there was a subtle change, the user was redirected
to `/`, but the redirected back to `/signin` or `/signup`.

This happened because on the 2nd render, the index page saw
that the relier model contained an email address and redirected
back to where the user just was.

To get around this, before checking the email and redirecting,
clear the email from the relier model. The email is saved
into the formPrefill model and that will be used to
prefill the email when the user comes back.

A small change to the router was needed to. In the "email-first"
flow, ensure the user can go back.

fixes #6033
  • Loading branch information
Shane Tomlinson committed Apr 16, 2018
1 parent 4e5a462 commit 412b246
Show file tree
Hide file tree
Showing 7 changed files with 59 additions and 8 deletions.
13 changes: 12 additions & 1 deletion app/scripts/lib/router.js
Expand Up @@ -146,7 +146,7 @@ define(function (require, exports, module) {
this.notifier.once('view-shown', this._afterFirstViewHasRendered.bind(this));
this.notifier.on('navigate', this.onNavigate.bind(this));
this.notifier.on('navigate-back', this.onNavigateBack.bind(this));
this.notifier.on('email-first-flow', () => this._isEmailFirstFlow = true);
this.notifier.on('email-first-flow', () => this._onEmailFirstFlow());

this.storage = Storage.factory('sessionStorage', this.window);
},
Expand Down Expand Up @@ -299,6 +299,17 @@ define(function (require, exports, module) {
this.storage.set('canGoBack', true);
},

_onEmailFirstFlow () {
this._isEmailFirstFlow = true;

// back is enabled for email-first so that
// users can go back to the / screen from "Mistyped email".
// The initial navigation to the next screen
// happens before the / page is rendered, causing
// `canGoBack` to not be set.
this.storage.set('canGoBack', true);
},

fragmentToViewName (fragment) {
fragment = fragment || '';
// strip leading /
Expand Down
18 changes: 17 additions & 1 deletion app/scripts/views/index.js
Expand Up @@ -33,6 +33,22 @@ define(function (require, exports, module) {
return 'enter-email';
}

beforeRender () {
const email = this.relier.get('email');
if (email) {
// Unsetting the relier email is to ensure the "mistyped email"
// link on the next page works. If the user clicks "mistyped email",
// they'd expect to come back here with the email prefilled and
// editable. If the email was still set in the relier, we'd send
// them right back. So, we unset the email from the relier and depend
// on the email being saved into formPrefill for when the user comes back.
this.relier.unset('email');
this.formPrefill.set('email', email);
// relierEmail is used in afterRender to decide whether to check an email.
this.model.set('relierEmail', email);
}
}

afterRender () {
// 1. COPPA checks whether the user is too young in beforeRender.
// So that COPPA takes precedence, do all other checks afterwards.
Expand All @@ -46,7 +62,7 @@ define(function (require, exports, module) {
} else if (this.isInEmailFirstExperimentGroup('treatment') || action === 'email') {
// let's the router know to use the email-first signin/signup page
this.notifier.trigger('email-first-flow');
const email = this.relier.get('email');
const email = this.model.get('relierEmail');
if (email) {
return this.checkEmail(email);
}
Expand Down
4 changes: 4 additions & 0 deletions app/tests/spec/lib/router.js
Expand Up @@ -419,6 +419,7 @@ define(function (require, exports, module) {
router.onSignUp();
assert.isTrue(router.showView.calledOnce);
assert.isTrue(router.showView.calledWith(SignUpView));
assert.isFalse(router.canGoBack());
});
});

Expand All @@ -428,6 +429,7 @@ define(function (require, exports, module) {
router.onSignUp();
assert.isTrue(router.showView.calledOnce);
assert.isTrue(router.showView.calledWith(SignUpPasswordView));
assert.isTrue(router.canGoBack());
});
});
});
Expand All @@ -443,6 +445,7 @@ define(function (require, exports, module) {
router.onSignIn();
assert.isTrue(router.showView.calledOnce);
assert.isTrue(router.showView.calledWith(SignInView));
assert.isFalse(router.canGoBack());
});
});

Expand All @@ -452,6 +455,7 @@ define(function (require, exports, module) {
router.onSignIn();
assert.isTrue(router.showView.calledOnce);
assert.isTrue(router.showView.calledWith(SignInPasswordView));
assert.isTrue(router.canGoBack());
});
});
});
Expand Down
6 changes: 4 additions & 2 deletions app/tests/spec/views/index.js
Expand Up @@ -149,8 +149,10 @@ define(function(require, exports, module) {

return view.render()
.then(() => {
assert.isTrue(view.checkEmail.calledOnce);
assert.isTrue(view.checkEmail.calledWith('testuser@testuser.com'));
assert.isTrue(view.checkEmail.calledOnceWith('testuser@testuser.com'));
assert.isFalse(relier.has('email'));
assert.equal(relier.get('relierEmail', 'testuser@testuser.com'));
assert.equal(view.formPrefill.get('email'), 'testuser@testuser.com');
});
});
});
Expand Down
12 changes: 10 additions & 2 deletions tests/functional/fx_firstrun_v2_email_first.js
Expand Up @@ -163,7 +163,11 @@ registerSuite('Firstrun Sync v2 email first', {
'fxaccounts:can_link_account': {ok: true}
}
}))
.then(testElementValueEquals(selectors.SIGNUP_PASSWORD.EMAIL, email));
.then(testElementValueEquals(selectors.SIGNUP_PASSWORD.EMAIL, email))
// user realizes it's the wrong email address.
.then(click(selectors.SIGNUP_PASSWORD.LINK_MISTYPED_EMAIL, selectors.ENTER_EMAIL.HEADER))

.then(testElementValueEquals(selectors.ENTER_EMAIL.EMAIL, email));
},

'email specified by relier, registered': function () {
Expand All @@ -177,7 +181,11 @@ registerSuite('Firstrun Sync v2 email first', {
'fxaccounts:can_link_account': {ok: true}
}
}))
.then(testElementValueEquals(selectors.SIGNIN_PASSWORD.EMAIL, email));
.then(testElementValueEquals(selectors.SIGNIN_PASSWORD.EMAIL, email))
// user realizes it's the wrong email address.
.then(click(selectors.SIGNIN_PASSWORD.LINK_MISTYPED_EMAIL, selectors.ENTER_EMAIL.HEADER))

.then(testElementValueEquals(selectors.ENTER_EMAIL.EMAIL, email));
}
}
});
2 changes: 2 additions & 0 deletions tests/functional/lib/selectors.js
Expand Up @@ -172,6 +172,7 @@ module.exports = {
SIGNIN_PASSWORD: {
EMAIL: 'input[type=email]',
HEADER: '#fxa-signin-password-header',
LINK_MISTYPED_EMAIL: '#back',
PASSWORD: 'input[type=password]',
SUBMIT: 'button[type="submit"]',
},
Expand Down Expand Up @@ -228,6 +229,7 @@ module.exports = {
EMAIL: 'input[type=email]',
ERROR_PASSWORDS_DO_NOT_MATCH: '.error',
HEADER: '#fxa-signup-password-header',
LINK_MISTYPED_EMAIL: '#back',
PASSWORD: '#password',
SUBMIT: 'button[type="submit"]',
VPASSWORD: '#vpassword',
Expand Down
12 changes: 10 additions & 2 deletions tests/functional/sync_v3_email_first.js
Expand Up @@ -213,7 +213,11 @@ registerSuite('Firefox Desktop Sync v3 email first', {
'fxaccounts:can_link_account': {ok: true}
}
}))
.then(testElementValueEquals(selectors.SIGNUP_PASSWORD.EMAIL, email));
.then(testElementValueEquals(selectors.SIGNUP_PASSWORD.EMAIL, email))
// user realizes it's the wrong email address.
.then(click(selectors.SIGNUP_PASSWORD.LINK_MISTYPED_EMAIL, selectors.ENTER_EMAIL.HEADER))

.then(testElementValueEquals(selectors.ENTER_EMAIL.EMAIL, email));
},

'email specified by relier, registered': function () {
Expand All @@ -227,7 +231,11 @@ registerSuite('Firefox Desktop Sync v3 email first', {
'fxaccounts:can_link_account': {ok: true}
}
}))
.then(testElementValueEquals(selectors.SIGNIN_PASSWORD.EMAIL, email));
.then(testElementValueEquals(selectors.SIGNIN_PASSWORD.EMAIL, email))
// user realizes it's the wrong email address.
.then(click(selectors.SIGNIN_PASSWORD.LINK_MISTYPED_EMAIL, selectors.ENTER_EMAIL.HEADER))

.then(testElementValueEquals(selectors.ENTER_EMAIL.EMAIL, email));
}
}
});

0 comments on commit 412b246

Please sign in to comment.