feat(CAD): ABC test for CAD phase 3 (deep link) (#5332) r=@philbooth

The new officially supported way to enable signinCodes is to use:

`&forceExperiment=sendSms&forceExperimentGroup=signinCodes`

Support for `&signinCodes=true` has been removed.

After adding the ExperimentMixin to the SmsMixin, I saw a bug
in the ExperimentMixin where it assumes `this._account` always
exists on the View, which isn't the case for the SMSSendView.
Most views declare a `this.getAccount()` function that returns
the account being used. I updated the mixin to first check for
`this.getAccount`, then as a fallback, use `this._account`

fixes #5278

app/scripts/lib/experiments/grouping-rules/send-sms-install-link.js

@@ -10,9 +10,9 @@ define((require, exports, module) => {
 
   const BaseGroupingRule = require('./base');
 
-  const GROUPS = ['control', 'treatment'];
+  const GROUPS = ['control', 'treatment', 'signinCodes'];
 
-  function isEmailInTreatment (email) {
+  function isEmailInSigninCodesGroup (email) {
     return /@softvision\.(com|ro)$/.test(email) ||
            /@mozilla\.(com|org)$/.test(email);
   }
@@ -30,10 +30,10 @@ define((require, exports, module) => {
 
       let choice;
 
-      if (subject.forceExperimentGroup) {
+      if (subject.forceExperiment === this.name && subject.forceExperimentGroup) {
         choice = subject.forceExperimentGroup;
-      } else if (isEmailInTreatment(subject.account.get('email'))) {
-        choice = 'treatment';
+      } else if (isEmailInSigninCodesGroup(subject.account.get('email'))) {
+        choice = 'signinCodes';
       } else {
         choice = this.uniformChoice(GROUPS, subject.uniqueUserId);
       }

app/scripts/models/reliers/sync.js

@@ -32,16 +32,13 @@ define(function (require, exports, module) {
     // because users that open the verification link with an
     // invalid signinCode should still be able to sign in.
     // The error will be logged and the signinCode ignored.
-    signin: Vat.string().empty('').renameTo('signinCode'),
-    // allow relier to enable signinCodes, used for testing.
-    signinCodes: Vat.boolean().renameTo('enableSigninCodes')
+    signin: Vat.string().empty('').renameTo('signinCode')
   };
   /*eslint-enable camelcase*/
 
   module.exports = Relier.extend({
     defaults: _.extend({}, Relier.prototype.defaults, {
       customizeSync: false,
-      enableSigninCodes: false,
       signinCode: undefined
     }),
 

app/scripts/views/mixins/connect-another-device-mixin.js

@@ -74,9 +74,9 @@ define((require, exports, module) => {
             const group = this.getExperimentGroup('sendSms', { account });
             this.createExperiment('sendSms', group);
 
-            if (group === 'treatment') {
+            if (group === 'treatment' || group === 'signinCodes') {
               this.navigate('sms', { account, country });
-            } else { // there are only two groups, by default, this is the control
+            } else { // there are only three groups, by default, this is the control
               this.navigate('connect_another_device', { account });
             }
           } else {

app/scripts/views/mixins/experiment-mixin.js

@@ -2,15 +2,45 @@
  * 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 to provide experiment related functionality.
+ *
+ * Expects the view to expose:
+ * - either this.getAccount() or this._account
+ * - this.metrics
+ * - this.translator
+ * - this.window
+ *
+ * @mixin ExperimentMixin
+ */
 define(function (require, exports, module) {
   'use strict';
 
+  const { isFunction } = require('underscore');
   const ExperimentInterface = require('lib/experiment');
 
+  function getAccount () {
+    // make no assumptions about the availability of this.getAccount.
+    return (isFunction(this.getAccount) && this.getAccount()) || this._account;
+  }
+
   module.exports = {
     initialize (options = {}) {
-      this.experiments = options.experiments || new ExperimentInterface({
-        account: this._account,
+      this.experiments = options.experiments || this._createExperimentInterface(options);
+
+      this.experiments.chooseExperiments();
+    },
+
+    /**
+     * Create an ExperimentInterface instance using `options`
+     *
+     * @param {Object} [options={}]
+     * @returns {Object}
+     * @private
+     */
+    _createExperimentInterface (options = {}) {
+      return new ExperimentInterface({
+        account: getAccount.call(this),
         experimentGroupingRules: options.experimentGroupingRules,
         metrics: this.metrics,
         notifier: options.notifier,
@@ -18,8 +48,6 @@ define(function (require, exports, module) {
         user: options.user,
         window: this.window
       });
-
-      this.experiments.chooseExperiments();
     },
 
     /**

app/scripts/views/mixins/sms-mixin.js

@@ -4,13 +4,18 @@
 
 /**
  * Get the features that should be enabled when sending an SMS.
- * Requires the SeachParamMixin
+ *
+ * @mixin SmsMixin
  */
 
 define((require, exports, module) => {
   'use strict';
 
+  const ExperimentMixin = require('views/mixins/experiment-mixin');
+
   module.exports = {
+    dependsOn: [ ExperimentMixin ],
+
     /**
      * Get the features that should be enabled when sending an SMS
      *
@@ -18,7 +23,7 @@ define((require, exports, module) => {
      */
     getSmsFeatures () {
       const features = [];
-      if (this.relier.get('enableSigninCodes')) {
+      if (this.isInExperimentGroup('sendSms', 'signinCodes')) {
         features.push('signinCodes');
       }
       return features;

app/tests/spec/lib/experiments/grouping-rules/send-sms-install-link.js

@@ -25,23 +25,23 @@ define(function (require, exports, module) {
         assert.isFalse(experiment.choose({ uniqueUserId: 'user-id' }));
       });
 
-      describe('forceExperimentGroup set', () => {
+      describe('forceExperiment, forceExperimentGroup set', () => {
         it('returns `treatment`', () => {
           account.set('email', 'testuser@testuser.com');
-          assert.equal(experiment.choose({ account, forceExperimentGroup: 'treatment', uniqueUserId: 'user-id' }), 'treatment');
+          assert.equal(experiment.choose({ account, forceExperiment: 'sendSms', forceExperimentGroup: 'treatment', uniqueUserId: 'user-id' }), 'treatment');
         });
       });
 
-      describe('email forces treatment', () => {
+      describe('email forces `signinCodes`', () => {
         it('returns true', () => {
           account.set('email', 'testuser@softvision.com');
-          assert.equal(experiment.choose({ account, uniqueUserId: 'user-id' }), 'treatment');
+          assert.equal(experiment.choose({ account, uniqueUserId: 'user-id' }), 'signinCodes');
           account.set('email', 'testuser@softvision.ro');
-          assert.equal(experiment.choose({ account, uniqueUserId: 'user-id' }), 'treatment');
+          assert.equal(experiment.choose({ account, uniqueUserId: 'user-id' }), 'signinCodes');
           account.set('email', 'testuser@mozilla.com');
-          assert.equal(experiment.choose({ account, uniqueUserId: 'user-id' }), 'treatment');
+          assert.equal(experiment.choose({ account, uniqueUserId: 'user-id' }), 'signinCodes');
           account.set('email', 'testuser@mozilla.org');
-          assert.equal(experiment.choose({ account, uniqueUserId: 'user-id' }), 'treatment');
+          assert.equal(experiment.choose({ account, uniqueUserId: 'user-id' }), 'signinCodes');
         });
       });
 
@@ -52,7 +52,7 @@ define(function (require, exports, module) {
 
           assert.equal(experiment.choose({ account, uniqueUserId: 'user-id' }), 'control');
           assert.isTrue(experiment.uniformChoice.calledOnce);
-          assert.isTrue(experiment.uniformChoice.calledWith(['control', 'treatment'], 'user-id'));
+          assert.isTrue(experiment.uniformChoice.calledWith(['control', 'treatment', 'signinCodes'], 'user-id'));
         });
       });
     });

app/tests/spec/models/reliers/sync.js

@@ -50,8 +50,7 @@ define(function (require, exports, module) {
           customizeSync: 'true',
           migration: SYNC_MIGRATION,
           service: SYNC_SERVICE,
-          signin: 'signin-code',
-          signinCodes: 'true'
+          signin: 'signin-code'
         });
 
         return relier.fetch()
@@ -62,7 +61,6 @@ define(function (require, exports, module) {
             assert.equal(relier.get('service'), SYNC_SERVICE);
             assert.isTrue(relier.get('customizeSync'));
             assert.equal(relier.get('signinCode'), 'signin-code');
-            assert.isTrue(relier.get('enableSigninCodes'));
           });
       });
 
@@ -294,100 +292,6 @@ define(function (require, exports, module) {
         });
       });
 
-      describe('signinCodes query parameter', () => {
-        describe('missing', () => {
-          beforeEach(() => {
-            windowMock.location.search = TestHelpers.toSearchString({
-              context: CONTEXT
-            });
-
-            return relier.fetch();
-          });
-
-          it('succeeds', () => {
-            assert.isFalse(relier.get('enableSigninCodes'));
-          });
-        });
-
-        describe('emtpy', () => {
-          beforeEach(() => {
-            windowMock.location.search = TestHelpers.toSearchString({
-              context: CONTEXT,
-              signinCodes: ''
-            });
-
-            return fetchExpectError();
-          });
-
-          it('errors correctly', () => {
-            assert.isTrue(AuthErrors.is(err, 'INVALID_PARAMETER'));
-            assert.equal(err.param, 'signinCodes');
-          });
-        });
-
-        describe('whitespace', () => {
-          beforeEach(() => {
-            windowMock.location.search = TestHelpers.toSearchString({
-              context: CONTEXT,
-              signinCodes: ' '
-            });
-
-            return fetchExpectError();
-          });
-
-          it('errors correctly', () => {
-            assert.isTrue(AuthErrors.is(err, 'INVALID_PARAMETER'));
-            assert.equal(err.param, 'signinCodes');
-          });
-        });
-
-        describe('not a boolean', () => {
-          beforeEach(() => {
-            windowMock.location.search = TestHelpers.toSearchString({
-              context: CONTEXT,
-              signinCodes: 'not a boolean'
-            });
-
-            return fetchExpectError();
-          });
-
-          it('errors correctly', () => {
-            assert.isTrue(AuthErrors.is(err, 'INVALID_PARAMETER'));
-            assert.equal(err.param, 'signinCodes');
-          });
-        });
-
-        describe('true', () => {
-          beforeEach(() => {
-            windowMock.location.search = TestHelpers.toSearchString({
-              context: CONTEXT,
-              signinCodes: 'true'
-            });
-
-            return relier.fetch();
-          });
-
-          it('succeeds', () => {
-            assert.isTrue(relier.get('enableSigninCodes'));
-          });
-        });
-
-        describe('false', () => {
-          beforeEach(() => {
-            windowMock.location.search = TestHelpers.toSearchString({
-              context: CONTEXT,
-              signinCodes: 'false'
-            });
-
-            return relier.fetch();
-          });
-
-          it('succeeds', () => {
-            assert.isFalse(relier.get('enableSigninCodes'));
-          });
-        });
-      });
-
       it('translates `service` to `serviceName`', () => {
         windowMock.location.search = TestHelpers.toSearchString({
           context: CONTEXT,

app/tests/spec/views/mixins/experiment-mixin.js

@@ -24,7 +24,9 @@ define(function (require, exports, module) {
 
   describe('views/mixins/experiment-mixin', () => {
     let experiments;
+    let metrics;
     let notifier;
+    let translator;
     let view;
     let windowMock;
 
@@ -43,14 +45,21 @@ define(function (require, exports, module) {
         isInExperimentGroup: sinon.spy()
       };
 
+      metrics = {};
+
       notifier = {
         trigger: sinon.spy()
       };
+
+      translator = {};
+
       windowMock = new WindowMock();
 
       view = new View({
         experiments,
+        metrics,
         notifier,
+        translator,
         window: windowMock
       });
     });
@@ -59,6 +68,56 @@ define(function (require, exports, module) {
       return view.destroy();
     });
 
+    describe('_createExperimentInterface', () => {
+      const experimentGroupingRules = {};
+      const notifier = {};
+      const user = {};
+
+      const getAccountAccount = { uid: 'get-account-account' };
+      const _accountAccount = { uid: '_account-account' };
+
+      describe('view has `this.getAccount', () => {
+        it('creates ExperimentInterface with account returned by this.getAccount', () => {
+          view.getAccount = sinon.spy(() => getAccountAccount);
+          view._account = _accountAccount;
+
+          const experimentInterface = view._createExperimentInterface({
+            experimentGroupingRules,
+            notifier,
+            user
+          });
+
+          assert.strictEqual(experimentInterface.account, getAccountAccount);
+          assert.strictEqual(experimentInterface.experimentGroupingRules, experimentGroupingRules);
+          assert.strictEqual(experimentInterface.notifier, notifier);
+          assert.strictEqual(experimentInterface.metrics, metrics);
+          assert.strictEqual(experimentInterface.translator, translator);
+          assert.strictEqual(experimentInterface.user, user);
+          assert.strictEqual(experimentInterface.window, windowMock);
+        });
+      });
+
+      describe('view does not have this.getAccount, has this._account', () => {
+        it('creates ExperimentInterface with this._account', () => {
+          view._account = _accountAccount;
+
+          const experimentInterface = view._createExperimentInterface({
+            experimentGroupingRules,
+            notifier,
+            user
+          });
+
+          assert.strictEqual(experimentInterface.account, _accountAccount);
+          assert.strictEqual(experimentInterface.experimentGroupingRules, experimentGroupingRules);
+          assert.strictEqual(experimentInterface.notifier, notifier);
+          assert.strictEqual(experimentInterface.metrics, metrics);
+          assert.strictEqual(experimentInterface.translator, translator);
+          assert.strictEqual(experimentInterface.user, user);
+          assert.strictEqual(experimentInterface.window, windowMock);
+        });
+      });
+    });
+
     describe('initialize', () => {
       it('chooses experiments', () => {
         assert.isTrue(experiments.chooseExperiments.calledOnce);

app/tests/spec/views/mixins/sms-mixin.js

@@ -9,8 +9,7 @@ define(function (require, exports, module) {
   const { assert } = require('chai');
   const BaseView = require('views/base');
   const Cocktail = require('cocktail');
-  const Relier = require('models/reliers/base');
-  const SearchParamMixin = require('lib/search-param-mixin');
+  const sinon = require('sinon');
   const Template = require('stache!templates/test_template');
 
   const SmsView = BaseView.extend({
@@ -18,34 +17,30 @@ define(function (require, exports, module) {
   });
   Cocktail.mixin(
     SmsView,
-    SearchParamMixin,
     SmsMixin
   );
 
   describe('views/mixins/sms-mixin', () => {
-    let relier;
     let view;
 
     beforeEach(() => {
-      relier = new Relier();
 
       view = new SmsView({
-        relier,
         windowMock: window
       });
     });
 
     describe('getSmsFeatures', () => {
-      describe('relier.enableSigninCodes=true', () => {
+      describe('user in `signinCodes` experiment group', () => {
         it('returns an array with `signinCodes`', () => {
-          relier.set('enableSigninCodes', true);
+          sinon.stub(view, 'isInExperimentGroup', () => true);
           assert.isTrue(view.getSmsFeatures().indexOf('signinCodes') > -1);
         });
       });
 
-      describe('relier.enableSigninCodes=false', () => {
+      describe('user not in `signinCodes` experiment group', () => {
         it('returns an empty array', () => {
-          relier.set('enableSigninCodes', false);
+          sinon.stub(view, 'isInExperimentGroup', () => false);
           assert.deepEqual(view.getSmsFeatures(), []);
         });
       });

docs/query-params.md

@@ -316,10 +316,8 @@ Should not be used by reliers. Should only be used by functional tests.
 Force a particular AB test.
 
 #### Options
-* `connectAnotherDevice` - the "connect another device" experiment that
-    encourages users to set up multiple devices.
-* `mailcheck` - provide a tooltip to correct common domain name errors in the
-    email.
+* `disabledButtonState` - Are the submit buttons on signin/signup disabled?
+* `sendSms` - Allow users to send an SMS containing a Firefox Mobile installation link
 
 ### `forceExperimentGroup`
 Force the user into a particular AB test experiment group.
@@ -327,6 +325,8 @@ Force the user into a particular AB test experiment group.
 #### Options
 * `control` - default behavior.
 * `treatment` - new behavior.
+* `signinCodes` - a second treatment group, only used for the `sendSms` experiment.
+  When sending an SMS, the install link contains a signinCode that helps the user sign in more easily on the second device.
 
 ## Reset Password parameters
 
@@ -357,7 +357,7 @@ if they perform a reset password.
 Shows the option to change a user's primary email address.
 
 #### Options
-* `true` 
+* `true`
 * `false` (default)
 
 #### When to specify

tests/functional/fx_desktop_handshake.js

@@ -28,7 +28,7 @@ define([
   const SETTINGS_PAGE_URL = `${config.fxaContentRoot}settings?automatedBrowser=true&forceUA=${encodeURIComponent(userAgent)}`;
   const SYNC_SETTINGS_PAGE_URL = `${SETTINGS_PAGE_URL}&service=sync`;
 
-  const SYNC_SMS_PAGE_URL = `${config.fxaContentRoot}sms?automatedBrowser=true&service=sync&signinCodes=true&forceUA=${encodeURIComponent(userAgent)}`;
+  const SYNC_SMS_PAGE_URL = `${config.fxaContentRoot}sms?automatedBrowser=true&service=sync&forceExperiment=sendSms&forceExperimentGroup=signinCodes&forceUA=${encodeURIComponent(userAgent)}`;  //eslint-disable-line max-len
 
   var browserSignedInEmail;
   let browserSignedInAccount;

tests/functional/fx_fennec_v1_sign_in.js

@@ -13,7 +13,7 @@ define([
 
   const config = intern.config;
   const SIGNIN_PAGE_URL = `${config.fxaContentRoot}signin?context=fx_fennec_v1&service=sync`;
-  const SMS_PAGE_URL = `${config.fxaContentRoot}sms?context=fx_desktop_v1&service=sync&signinCodes=true`;
+  const SMS_PAGE_URL = `${config.fxaContentRoot}sms?context=fx_desktop_v1&service=sync&forceExperiment=sendSms&forceExperimentGroup=signinCodes`;
 
   let email;
   const PASSWORD = '12345678';

tests/functional/fx_ios_v1_sign_in.js

@@ -16,7 +16,7 @@ define([
 
   const config = intern.config;
   const SIGNIN_PAGE_URL = `${config.fxaContentRoot}signin?context=fx_ios_v1&service=sync`;
-  const SMS_PAGE_URL = `${config.fxaContentRoot}sms?context=fx_desktop_v1&service=sync&signinCodes=true`;
+  const SMS_PAGE_URL = `${config.fxaContentRoot}sms?context=fx_desktop_v1&service=sync&forceExperiment=sendSms&forceExperimentGroup=signinCodes`;
 
   let email;
   const PASSWORD = '12345678';

tests/functional/send_sms.js

@@ -27,7 +27,7 @@
 
 
    const SEND_SMS_URL = `${config.fxaContentRoot}sms?service=sync&country=US`;
-   const SEND_SMS_SIGNIN_CODE_URL = `${SEND_SMS_URL}&signinCodes=true`;
+   const SEND_SMS_SIGNIN_CODE_URL = `${SEND_SMS_URL}&forceExperiment=sendSms&forceExperimentGroup=signinCodes`;
    const SEND_SMS_NO_QUERY_URL = `${config.fxaContentRoot}sms`;