From 8fafad7f418419c3eea679e345301e03a43c790c Mon Sep 17 00:00:00 2001 From: Shane Tomlinson Date: Mon, 24 Jul 2017 15:37:50 +0100 Subject: [PATCH] refactor(email-opt-in): Extract the email-opt-in frontend logic for re-use. (#5275) r=@vbudhram Extract the email-opt-in login to a mixin for use in the email-first flow. --- .../views/mixins/email-opt-in-mixin.js | 43 ++++++++ app/scripts/views/sign_up.js | 14 +-- .../spec/views/mixins/email-opt-in-mixin.js | 104 ++++++++++++++++++ app/tests/spec/views/sign_up.js | 21 ++-- app/tests/test_start.js | 1 + 5 files changed, 161 insertions(+), 22 deletions(-) create mode 100644 app/scripts/views/mixins/email-opt-in-mixin.js create mode 100644 app/tests/spec/views/mixins/email-opt-in-mixin.js diff --git a/app/scripts/views/mixins/email-opt-in-mixin.js b/app/scripts/views/mixins/email-opt-in-mixin.js new file mode 100644 index 000000000..f243ee81b --- /dev/null +++ b/app/scripts/views/mixins/email-opt-in-mixin.js @@ -0,0 +1,43 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * 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/. */ + +/** + * Mixin that provides email opt-in functionality. + * + * Sets `isEmailOptInVisible` in the context. + * Provides `hasOptedInToEmail` to query whether the user has + * opted-in. + */ +define(function (require, exports, module) { + 'use strict'; + + module.exports = { + initialize (options = {}) { + this._experimentGroupingRules = options.experimentGroupingRules; + }, + + setInitialContext (context) { + context.set('isEmailOptInVisible', this._isEmailOptInEnabled()); + }, + + afterRender () { + this.logViewEvent(`email-optin.visible.${String(this._isEmailOptInEnabled())}`); + }, + + _isEmailOptInEnabled () { + return !! this._experimentGroupingRules.choose('communicationPrefsVisible', { + lang: this.navigator.language + }); + }, + + /** + * Query whether user has opted-in to marketing email. + * + * @returns {Boolean} + */ + hasOptedInToMarketingEmail () { + return !! this.$('.marketing-email-optin').is(':checked'); + } + }; +}); diff --git a/app/scripts/views/sign_up.js b/app/scripts/views/sign_up.js index 22362b325..851d0068f 100644 --- a/app/scripts/views/sign_up.js +++ b/app/scripts/views/sign_up.js @@ -10,6 +10,7 @@ define(function (require, exports, module) { const CheckboxMixin = require('views/mixins/checkbox-mixin'); const Cocktail = require('cocktail'); const CoppaAgeInput = require('views/coppa/coppa-age-input'); + const EmailOptInMixin = require('views/mixins/email-opt-in-mixin'); const ExperimentMixin = require('views/mixins/experiment-mixin'); const FlowBeginMixin = require('views/mixins/flow-begin-mixin'); const FormPrefillMixin = require('views/mixins/form-prefill-mixin'); @@ -91,9 +92,6 @@ define(function (require, exports, module) { }, afterRender () { - this.logViewEvent('email-optin.visible.' + - String(this._isEmailOptInEnabled())); - return this._createCoppaView() .then(() => FormView.prototype.afterRender.call(this)); }, @@ -150,7 +148,6 @@ define(function (require, exports, module) { forceEmail: forceEmail, isAmoMigration: this.isAmoMigration(), isCustomizeSyncChecked: relier.isCustomizeSyncChecked(), - isEmailOptInVisible: this._isEmailOptInEnabled(), isSignInEnabled: ! forceEmail, isSync: isSync, isSyncMigration: this.isSyncMigration(), @@ -334,7 +331,7 @@ define(function (require, exports, module) { var account = this.user.initAccount({ customizeSync: this.$('.customize-sync').is(':checked'), email: this.getElementValue('.email'), - needsOptedInToMarketingEmail: this.$('.marketing-email-optin').is(':checked') + needsOptedInToMarketingEmail: this.hasOptedInToMarketingEmail() }); if (this.relier.isSync()) { @@ -348,12 +345,6 @@ define(function (require, exports, module) { _suggestSignIn (err) { err.forceMessage = t('Account already exists. Sign in'); return this.unsafeDisplayError(err); - }, - - _isEmailOptInEnabled () { - return !! this._experimentGroupingRules.choose('communicationPrefsVisible', { - lang: this.navigator.language - }); } }, { ENTRYPOINT: 'fxa:signup' @@ -363,6 +354,7 @@ define(function (require, exports, module) { View, AccountResetMixin, CheckboxMixin, + EmailOptInMixin, ExperimentMixin, FlowBeginMixin, FormPrefillMixin, diff --git a/app/tests/spec/views/mixins/email-opt-in-mixin.js b/app/tests/spec/views/mixins/email-opt-in-mixin.js new file mode 100644 index 000000000..d2d258fed --- /dev/null +++ b/app/tests/spec/views/mixins/email-opt-in-mixin.js @@ -0,0 +1,104 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * 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/. */ + +define(function (require, exports, module) { + 'use strict'; + + const { assert } = require('chai'); + const Cocktail = require('cocktail'); + const EmailOptInMixin = require('views/mixins/email-opt-in-mixin'); + const sinon = require('sinon'); + const BaseView = require('views/base'); + + const View = BaseView.extend({ + template: () => ` + Opt-in to email + ` + }); + + Cocktail.mixin( + View, + EmailOptInMixin + ); + + describe('views/mixins/email-opt-in-mixin', function () { + let experimentGroupingRules; + let view; + + beforeEach(() => { + experimentGroupingRules = { + choose: () => {} + }; + + view = new View({ + experimentGroupingRules + }); + }); + + it('exports correct interface', function () { + assert.isObject(EmailOptInMixin); + assert.lengthOf(Object.keys(EmailOptInMixin), 5); + assert.isFunction(EmailOptInMixin.hasOptedInToMarketingEmail); + }); + + describe('render', () => { + let communicationPrefsVisible; + + beforeEach(() => { + sinon.stub(experimentGroupingRules, 'choose', () => communicationPrefsVisible); + sinon.spy(view, 'logViewEvent'); + sinon.spy(view, 'template'); + }); + + + describe('enabled for user', () => { + beforeEach(() => { + communicationPrefsVisible = true; + return view.render(); + }); + + it('sets `isEmailOptInVisible=true, logs correctly`', () => { + assert.isTrue(view.template.calledOnce); + const templateArgs = view.template.args[0][0]; + assert.isTrue(templateArgs.isEmailOptInVisible); + + assert.isTrue(view.logViewEvent.calledOnce); + assert.isTrue(view.logViewEvent.calledWith('email-optin.visible.true')); + }); + }); + + describe('disabled for user', () => { + beforeEach(() => { + communicationPrefsVisible = false; + return view.render(); + }); + + it('sets `isEmailOptInVisible=false, logs correctly`', () => { + assert.isTrue(view.template.calledOnce); + const templateArgs = view.template.args[0][0]; + assert.isFalse(templateArgs.isEmailOptInVisible); + + assert.isTrue(view.logViewEvent.calledOnce); + assert.isTrue(view.logViewEvent.calledWith('email-optin.visible.false')); + }); + }); + }); + + describe('hasOptedInToMarketingEmail', () => { + beforeEach(() => { + return view.render(); + }); + + it('returns `true` if the checkbox is checked', () => { + view.$('.marketing-email-optin').attr('checked', 'checked'); + assert.isTrue(view.hasOptedInToMarketingEmail()); + }); + + it('returns `false` if the checkbox is unchecked', () => { + view.$('.marketing-email-optin').removeAttr('checked'); + assert.isFalse(view.hasOptedInToMarketingEmail()); + }); + }); + }); +}); diff --git a/app/tests/spec/views/sign_up.js b/app/tests/spec/views/sign_up.js index 388ffe623..793b2a1d2 100644 --- a/app/tests/spec/views/sign_up.js +++ b/app/tests/spec/views/sign_up.js @@ -850,26 +850,25 @@ define(function (require, exports, module) { describe('signup succeeds', function () { beforeEach(function () { - sinon.stub(view, 'signUp', function () { - return p(); - }); + sinon.stub(view, 'signUp', () => p()); + sinon.stub(view, 'hasOptedInToMarketingEmail', () => true); return view.submit(); }); - it('does not call view.signIn', function () { + it('calls view.signUp correctly, does not display any errors', function () { assert.isFalse(view.signIn.called); - }); - - it('calls view.signUp correctly', function () { assert.equal(view.signUp.callCount, 1); var args = view.signUp.args[0]; - assert.instanceOf(args[0], Account); - assert.equal(args[1], 'password'); - }); + const account = args[0]; + assert.instanceOf(account, Account); + assert.equal(account.get('email'), email); + assert.isTrue(account.get('needsOptedInToMarketingEmail')); + assert.isFalse(account.get('customizeSync')); + + assert.equal(args[1], 'password'); - it('does not display any errors', function () { assert.isFalse(view.displayError.called); assert.isFalse(view.unsafeDisplayError.called); }); diff --git a/app/tests/test_start.js b/app/tests/test_start.js index 39bd1cb20..78392e39a 100644 --- a/app/tests/test_start.js +++ b/app/tests/test_start.js @@ -145,6 +145,7 @@ function (Translator, Session) { '../tests/spec/views/mixins/back-mixin', '../tests/spec/views/mixins/checkbox-mixin', '../tests/spec/views/mixins/connect-another-device-mixin', + '../tests/spec/views/mixins/email-opt-in-mixin', '../tests/spec/views/mixins/experiment-mixin', '../tests/spec/views/mixins/external-links-mixin', '../tests/spec/views/mixins/floating-placeholder-mixin',