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

Commit

Permalink
refactor(oauth): Remove support for AMO migration text (#6131) r=@vla…
Browse files Browse the repository at this point in the history 
…dikoff

`migration=amo` will not cause an error, but no special
help text is displayed either. The value is dropped on
the ground.

fixes #6123
  • Loading branch information
@vladikoff
Shane Tomlinson authored and vladikoff committed Apr 30, 2018
1 parent 530473c commit 8f884aa
Show file tree
Hide file tree
Showing 15 changed files with 25 additions and 307 deletions.
7 changes: 4 additions & 3 deletions app/scripts/models/reliers/relier.js
View file
Expand Up @@ -129,10 +129,11 @@ define(function (require, exports, module) {
this.set('allowCachedCredentials', false);
}

if (this.get('migration') === Constants.SYNC11_MIGRATION) {
if (this.has('migration')) {
// Support for the sync1.1 migration message was
// removed in #6130, accept the value so
// no errors are caused but drop the value on the ground.
// removed in #6130, support for AMO in #6131.
// Accept the value so no errors are caused but
// drop the value on the ground.
this.unset('migration');
}
});
Expand Down
3 changes: 0 additions & 3 deletions app/scripts/templates/sign_in.mustache
View file
Expand Up @@ -19,9 +19,6 @@
{{^error}}
<div class="error"></div>
<div class="success"></div>
{{#isAmoMigration}}
<div class="info nudge pad" id="amo-migration">{{#unsafeTranslate}}Looking for your Add-ons data? <a href="/signup">Sign up</a> for a Firefox Account with your old Add-ons account email address.{{/unsafeTranslate}}</div>
{{/isAmoMigration}}

{{#suggestedAccount}}
<div class="avatar-wrapper avatar-view">
Expand Down
4 changes: 0 additions & 4 deletions app/scripts/templates/sign_up.mustache
View file
Expand Up @@ -22,10 +22,6 @@

{{{ syncSuggestionHTML }}}

{{#isAmoMigration}}
<div class="info nudge" id="amo-migration">{{#t}}Looking for your Add-ons data? Sign up for a Firefox Account with your old Add-ons account email address.{{/t}}</div>
{{/isAmoMigration}}

<form novalidate>
<div class="input-row {{^forceEmail}} label-helper-top{{/forceEmail}}">
{{#forceEmail}}
Expand Down
18 changes: 0 additions & 18 deletions app/scripts/views/mixins/migration-mixin.js
View file

This file was deleted.

11 changes: 1 addition & 10 deletions app/scripts/views/sign_in.js
View file
Expand Up @@ -14,7 +14,6 @@ define(function (require, exports, module) {
const FlowBeginMixin = require('./mixins/flow-begin-mixin');
const FormPrefillMixin = require('./mixins/form-prefill-mixin');
const FormView = require('./form');
const MigrationMixin = require('./mixins/migration-mixin');
const PasswordMixin = require('./mixins/password-mixin');
const PasswordResetMixin = require('./mixins/password-reset-mixin');
const { preventDefaultThen, t } = require('./base');
Expand Down Expand Up @@ -99,7 +98,6 @@ define(function (require, exports, module) {
email: email,
error: this.error,
headerSignInText,
isAmoMigration: this.isAmoMigration(),
password: this.formPrefill.get('password'),
suggestedAccount: hasSuggestedAccount
});
Expand Down Expand Up @@ -160,13 +158,7 @@ define(function (require, exports, module) {
},

_suggestSignUp (err) {
if (this.isAmoMigration()) {
err.forceMessage =
t('Unknown Firefox Account. <a href="/signup">Sign up</a> using your previous Add-ons account email address to access your Add-ons data.');
this.$('#amo-migration').hide();
} else {
err.forceMessage = t('Unknown account. <a href="/signup">Sign up</a>');
}
err.forceMessage = t('Unknown account. <a href="/signup">Sign up</a>');

return this.unsafeDisplayError(err);
},
Expand Down Expand Up @@ -283,7 +275,6 @@ define(function (require, exports, module) {
FlowBeginMixin,
EmailFirstExperimentMixin({ treatmentPathname: '/' }),
FormPrefillMixin,
MigrationMixin,
PasswordMixin,
PasswordResetMixin,
ServiceMixin,
Expand Down
13 changes: 1 addition & 12 deletions app/scripts/views/sign_up.js
View file
Expand Up @@ -18,7 +18,6 @@ define(function (require, exports, module) {
const FormPrefillMixin = require('./mixins/form-prefill-mixin');
const FormView = require('./form');
const mailcheck = require('../lib/mailcheck');
const MigrationMixin = require('./mixins/migration-mixin');
const PasswordMixin = require('./mixins/password-mixin');
const ServiceMixin = require('./mixins/service-mixin');
const SignedInNotificationMixin = require('./mixins/signed-in-notification-mixin');
Expand Down Expand Up @@ -81,8 +80,7 @@ define(function (require, exports, module) {
},

events: {
'blur input.email': 'onEmailBlur',
'click #amo-migration a': 'onAmoSignIn'
'blur input.email': 'onEmailBlur'
},

getPrefillEmail () {
Expand Down Expand Up @@ -112,7 +110,6 @@ define(function (require, exports, module) {
email: prefillEmail,
error: this.error,
forceEmail: forceEmail,
isAmoMigration: this.isAmoMigration(),
isCustomizeSyncChecked: relier.isCustomizeSyncChecked(),
isSignInEnabled: ! forceEmail,
isSync: isSync
Expand Down Expand Up @@ -256,13 +253,6 @@ define(function (require, exports, module) {
mailcheck(this.$el.find('.email'), this);
},

onAmoSignIn () {
// The user has chosen to sign in with a different email, clear the
// email from the relier so it's not used again on the signin page.
this.relier.unset('email');
this.$('input[type=email]').val('');
},

_isEmailSameAsBouncedEmail () {
var bouncedEmail = this.model.get('bouncedEmail');

Expand Down Expand Up @@ -316,7 +306,6 @@ define(function (require, exports, module) {
FloatingPlaceholderMixin,
FlowBeginMixin,
FormPrefillMixin,
MigrationMixin,
PasswordMixin,
ServiceMixin,
SignInMixin,
Expand Down
3 changes: 1 addition & 2 deletions app/tests/spec/lib/metrics.js
View file
Expand Up @@ -45,7 +45,6 @@ define(function (require, exports, module) {
entrypoint: 'menupanel',
isSampledUser: true,
lang: 'db_LB',
migration: 'amo',
notifier,
screenHeight: 1200,
screenWidth: 1600,
Expand Down Expand Up @@ -193,7 +192,7 @@ define(function (require, exports, module) {
assert.equal(filteredData.lang, 'db_LB');
assert.equal(filteredData.emailDomain, 'none');
assert.equal(filteredData.entrypoint, 'menupanel');
assert.equal(filteredData.migration, 'amo');
assert.equal(filteredData.migration, 'none');
assert.equal(filteredData.uniqueUserId, '0ae7fe2b-244f-4a78-9857-dff3ae263927');
assert.equal(filteredData.startTime, 1439233336187);

Expand Down
18 changes: 10 additions & 8 deletions app/tests/spec/models/reliers/relier.js
View file
Expand Up @@ -114,18 +114,20 @@ define(function (require, exports, module) {
testInvalidQueryParam('migration', token);
});

[undefined, Constants.AMO_MIGRATION].forEach(function (value) {
[undefined].forEach(function (value) {
testValidQueryParam('migration', value, 'migration', value);
});

describe('sync11 migration', () => {
it('accepts the value, but drops it on the ground', () => {
windowMock.location.search = TestHelpers.toSearchString({ migration: Constants.SYNC11_MIGRATION });
[Constants.SYNC11_MIGRATION, Constants.AMO_MIGRATION].forEach((migration) => {
describe(`${migration} migration`, () => {
it('accepts the value, but drops it on the ground', () => {
windowMock.location.search = TestHelpers.toSearchString({ migration });

return relier.fetch()
.then(() => {
assert.isFalse(relier.has('migration'));
});
return relier.fetch()
.then(() => {
assert.isFalse(relier.has('migration'));
});
});
});
});

Expand Down
47 changes: 0 additions & 47 deletions app/tests/spec/views/mixins/migration-mixin.js
View file

This file was deleted.

66 changes: 8 additions & 58 deletions app/tests/spec/views/oauth_sign_in.js
View file
Expand Up @@ -90,15 +90,12 @@ define(function (require, exports, module) {
}

describe('render', function () {
it('displays oAuth client name, does not display AMO help text by default', function () {
sinon.stub(view, 'isAmoMigration').callsFake(() => false);

it('displays oAuth client name', function () {
return view.render()
.then(function () {
assert.include(view.$('#fxa-signin-header').text(), CLIENT_NAME);
// also make sure link is correct
assert.equal(view.$('.sign-up').attr('href'), '/oauth/signup' + encodedLocationSearch);
assert.lengthOf(view.$('#amo-migration'), 0);
});
});

Expand All @@ -121,17 +118,6 @@ define(function (require, exports, module) {
assert.equal(view.$('.sign-up').attr('href'), '/oauth/signup' + encodedLocationSearch);
});
});

describe('AMO migration', () => {
it('displays AMO help text', () => {
sinon.stub(view, 'isAmoMigration').callsFake(() => true);
return view.render()
.then(() => {
assert.lengthOf(view.$('#amo-migration'), 1);
assert.equal(view.$('#amo-migration a').attr('href'), '/oauth/signup' + encodedLocationSearch);
});
});
});
});

describe('submit', function () {
Expand All @@ -155,53 +141,17 @@ define(function (require, exports, module) {
});

describe('_suggestSignUp', () => {
let err;

beforeEach(() => {
err = AuthErrors.toError('UNKNOWN_ACCOUNT');
const err = AuthErrors.toError('UNKNOWN_ACCOUNT');
sinon.spy(view, 'unsafeDisplayError');
return view._suggestSignUp(err);
});

describe('AMO migration', () => {
let $amoMigrationElement;
beforeEach(() => {
$amoMigrationElement = {
hide: sinon.spy()
};
sinon.stub(view, 'isAmoMigration').callsFake(() => true);
const orig$ = view.$;
sinon.stub(view, '$').callsFake((selector) => {
if (selector === '#amo-migration') {
return $amoMigrationElement;
} else {
return orig$.call(view, selector);
}
});

return view._suggestSignUp(err);
});

it('shows addons help text with link to the signup page, hides AMO migration text', () => {
var err = view.unsafeDisplayError.args[0][0];
assert.isTrue(AuthErrors.is(err, 'UNKNOWN_ACCOUNT'));
assert.include(err.forceMessage, '/signup');
assert.include(err.forceMessage, 'Add-ons');
assert.isTrue($amoMigrationElement.hide.calledOnce);
});
});

describe('not AMO migration', () => {
beforeEach(() => {
sinon.stub(view, 'isAmoMigration').callsFake(() => false);
return view._suggestSignUp(err);
});

it('shows a link to the signup page', () => {
var err = view.unsafeDisplayError.args[0][0];
assert.isTrue(AuthErrors.is(err, 'UNKNOWN_ACCOUNT'));
assert.include(err.forceMessage, '/signup');
assert.notInclude(err.forceMessage, 'Add-ons');
});
it('shows a link to the signup page', () => {
const err = view.unsafeDisplayError.args[0][0];
assert.isTrue(AuthErrors.is(err, 'UNKNOWN_ACCOUNT'));
assert.include(err.forceMessage, '/signup');
assert.notInclude(err.forceMessage, 'Add-ons');
});
});
});
Expand Down
31 changes: 0 additions & 31 deletions app/tests/spec/views/sign_in.js
View file
Expand Up @@ -216,37 +216,6 @@ define(function (require, exports, module) {
});
});

describe('migration', () => {
beforeEach(() => {
initView();
});

it('does not display migration message if no migration', () => {
return view.render()
.then(() => {
assert.lengthOf(view.$('#amo-migration'), 0);
});
});

it('displays migration message if isAmoMigration returns true', () => {
sinon.stub(view, 'isAmoMigration').callsFake(() => true);

return view.render()
.then(() => {
assert.lengthOf(view.$('#amo-migration'), 1);
});
});

it('does not display migration message if isAmoMigration returns false', () => {
sinon.stub(view, 'isAmoMigration').callsFake(() => false);

return view.render()
.then(() => {
assert.lengthOf(view.$('#amo-migration'), 0);
});
});
});

describe('isValid', () => {
it('returns true if both email and password are valid', () => {
view.$('[type=email]').val('testuser@testuser.com');
Expand Down

0 comments on commit 8f884aa

Please sign in to comment.