revert(feature-flags): remove feature-flagging

Feature-flagging caused fxa-dev to fail with Redis connection errors. Let's back it out from `master` while I dig into what's going wrong. This reverts commit 62f4b63.
mozilla · Mar 18, 2019 · e64c571 · e64c571
1 parent e58f443
commit e64c571
Show file tree

Hide file tree

Showing 24 changed files with 233 additions and 577 deletions.
diff --git a/.circleci/config.yml b/.circleci/config.yml
@@ -6,7 +6,6 @@ jobs:
         - image: circleci/node:8
           environment:
           - DISPLAY=:99
-        - image: redis
 
         steps:
         - checkout
@@ -38,7 +37,6 @@ jobs:
         working_directory: ~/fxa
         docker:
             - image: circleci/node:8-stretch-browsers
-            - image: redis
         steps:
         - attach_workspace:
               at: ~/
@@ -69,7 +67,6 @@ jobs:
         working_directory: ~/fxa
         docker:
         - image: circleci/node:8-stretch-browsers
-        - image: redis
         steps:
         - attach_workspace:
               at: ~/
@@ -100,7 +97,6 @@ jobs:
         working_directory: ~/fxa
         docker:
         - image: circleci/node:8-stretch-browsers
-        - image: redis
         steps:
         - attach_workspace:
               at: ~/
@@ -131,7 +127,6 @@ jobs:
         working_directory: ~/fxa
         docker:
         - image: circleci/node:8-stretch-browsers
-        - image: redis
         steps:
         - attach_workspace:
               at: ~/
@@ -158,7 +153,6 @@ jobs:
         working_directory: ~/fxa
         docker:
             - image: circleci/node:8-stretch-browsers
-            - image: redis
         steps:
             - attach_workspace:
                   at: ~/
@@ -191,7 +185,6 @@ jobs:
     dockerpush:
         docker:
         - image: circleci/node:8-stretch-browsers
-        - image: redis
 
         steps:
         - checkout

diff --git a/app/scripts/lib/app-start.js b/app/scripts/lib/app-start.js
@@ -95,8 +95,7 @@ Start.prototype = {
 
   initializeExperimentGroupingRules () {
     this._experimentGroupingRules = new ExperimentGroupingRules({
-      env: this._config.env,
-      featureFlags: this._config.featureFlags
+      env: this._config.env
     });
   },
 

diff --git a/app/scripts/lib/config-loader.js b/app/scripts/lib/config-loader.js
@@ -53,11 +53,6 @@ ConfigLoader.prototype = {
     try {
       const serializedJSONConfig = decodeURIComponent(configFromHTML);
       config = JSON.parse(serializedJSONConfig);
-
-      const serializedFeatureFlags = decodeURIComponent(
-        $('meta[name="fxa-feature-flags"]').attr('content')
-      );
-      config.featureFlags = JSON.parse(serializedFeatureFlags);
     } catch (e) {
       return Promise.reject(ConfigLoader.Errors.toError('INVALID_CONFIG'));
     }

diff --git a/app/scripts/lib/experiments/grouping-rules/communication-prefs.js b/app/scripts/lib/experiments/grouping-rules/communication-prefs.js
@@ -25,39 +25,30 @@ const AVAILABLE_LANGUAGES = [
   'zh-tw'
 ];
 
-const AVAILABLE_LANGUAGES_REGEX = arrayToRegex(AVAILABLE_LANGUAGES);
+const availableLocalesRegExpStr = `^(${AVAILABLE_LANGUAGES.join('|')})$`;
+const availableLocalesRegExp = new RegExp(availableLocalesRegExpStr);
 
 function normalizeLanguage(lang) {
   return lang.toLowerCase().replace(/_/g, '-');
 }
 
-function areCommunicationPrefsAvailable(lang, availableLanguages) {
+function areCommunicationPrefsAvailable(lang) {
   const normalizedLanguage = normalizeLanguage(lang);
-  return availableLanguages.test(normalizedLanguage);
-}
-
-function arrayToRegex (array) {
-  return new RegExp(`^(?:${array.join('|')})$`);
+  return availableLocalesRegExp.test(normalizedLanguage);
 }
 
 module.exports = class CommunicationPrefsGroupingRule extends BaseGroupingRule {
   constructor () {
     super();
     this.name = 'communicationPrefsVisible';
+    this.availableLanguages = AVAILABLE_LANGUAGES;
   }
 
   choose (subject = {}) {
-    const { featureFlags, lang } = subject;
-    let availableLanguages = AVAILABLE_LANGUAGES_REGEX;
-
-    if (featureFlags && Array.isArray(featureFlags.communicationPrefLanguages)) {
-      availableLanguages = arrayToRegex(featureFlags.communicationPrefLanguages);
-    }
-
-    if (! lang) {
+    if (! subject.lang) {
       return false;
     }
 
-    return areCommunicationPrefsAvailable(lang, availableLanguages);
+    return areCommunicationPrefsAvailable(subject.lang);
   }
 };
diff --git a/app/scripts/lib/experiments/grouping-rules/index.js b/app/scripts/lib/experiments/grouping-rules/index.js
@@ -24,7 +24,6 @@ class ExperimentChoiceIndex {
   constructor (options = {}) {
     this._env = options.env;
     this._experimentGroupingRules = options.experimentGroupingRules || experimentGroupingRules;
-    this._featureFlags = options.featureFlags;
   }
 
   /**
@@ -52,7 +51,6 @@ class ExperimentChoiceIndex {
       const subjectCopy = Object.create(subject);
       subjectCopy.env = subject.env || this._env;
       subjectCopy.experimentGroupingRules = this;
-      subjectCopy.featureFlags = subject.featureFlags || this._featureFlags;
       return experiment.choose(subjectCopy);
     }
   }

diff --git a/app/scripts/lib/experiments/grouping-rules/is-sampled-user.js b/app/scripts/lib/experiments/grouping-rules/is-sampled-user.js
@@ -16,24 +16,18 @@ module.exports = class IsSampledUserGroupingRule extends BaseGroupingRule {
   }
 
   choose (subject = {}) {
-    const sampleRate = IsSampledUserGroupingRule.sampleRate(subject);
+    const sampleRate = IsSampledUserGroupingRule.sampleRate(subject.env);
     return !! (subject.env && subject.uniqueUserId && this.bernoulliTrial(sampleRate, subject.uniqueUserId));
   }
 
   /**
-   * Return the sample rate from `featureFlags` or `env`
+   * Return the sample rate for `env`
    *
    * @static
-   * @param {Object} options
-   * @param {String} [options.env]
-   * @param {Object} [options.featureFlags]
+   * @param {String} env
    * @returns {Number}
    */
-  static sampleRate ({ env, featureFlags }) {
-    if (featureFlags && featureFlags.metricsSampleRate >= 0) {
-      return featureFlags.metricsSampleRate;
-    }
-
+  static sampleRate (env) {
     return env === 'development' ? 1.0 : 0.1;
   }
 };
diff --git a/app/scripts/lib/experiments/grouping-rules/send-sms-install-link.js b/app/scripts/lib/experiments/grouping-rules/send-sms-install-link.js
@@ -24,23 +24,13 @@ module.exports = class SmsGroupingRule extends BaseGroupingRule {
   }
 
   choose (subject = {}) {
-    if (! subject.account || ! subject.uniqueUserId || ! subject.country) {
-      return false;
-    }
-
-    let telephoneInfo = CountryTelephoneInfo[subject.country];
-    const { featureFlags } = subject;
-    if (featureFlags && featureFlags.smsCountries) {
-      telephoneInfo = featureFlags.smsCountries[subject.country];
-    }
-
-    if (! telephoneInfo) {
+    if (! subject.account || ! subject.uniqueUserId || ! subject.country || ! CountryTelephoneInfo[subject.country]) {
       return false;
     }
 
     let choice = false;
     // If rolloutRate is not specified, assume 0.
-    const rolloutRate = telephoneInfo.rolloutRate || 0;
+    const { rolloutRate } = CountryTelephoneInfo[subject.country] || 0;
 
     if (this.isTestEmail(subject.account.get('email'))) {
       choice = 'signinCodes';

diff --git a/app/scripts/lib/experiments/grouping-rules/sentry.js b/app/scripts/lib/experiments/grouping-rules/sentry.js
@@ -16,7 +16,7 @@ module.exports = class SentryGroupingRule extends BaseGroupingRule {
   }
 
   choose (subject = {}) {
-    const sampleRate = SentryGroupingRule.sampleRate(subject);
+    const sampleRate = SentryGroupingRule.sampleRate(subject.env);
 
     return !! (subject.env && subject.uniqueUserId && this.bernoulliTrial(sampleRate, subject.uniqueUserId));
   }
@@ -28,11 +28,7 @@ module.exports = class SentryGroupingRule extends BaseGroupingRule {
    * @param {String} env
    * @returns {Number}
    */
-  static sampleRate ({ env, featureFlags }) {
-    if (featureFlags && featureFlags.sentrySampleRate >= 0) {
-      return featureFlags.sentrySampleRate;
-    }
-
+  static sampleRate (env) {
     return env === 'development' ? 1.0 : 0.3;
   }
 };
diff --git a/app/scripts/lib/experiments/grouping-rules/token-code.js b/app/scripts/lib/experiments/grouping-rules/token-code.js
@@ -48,13 +48,8 @@ module.exports = class TokenCodeGroupingRule extends BaseGroupingRule {
       return false;
     }
 
-    const { featureFlags } = subject;
-
     if (subject.clientId) {
-      let client = this.ROLLOUT_CLIENTS[subject.clientId];
-      if (featureFlags && featureFlags.tokenCodeClients) {
-        client = featureFlags.tokenCodeClients[subject.clientId];
-      }
+      const client = this.ROLLOUT_CLIENTS[subject.clientId];
 
       if (client) {
         const groups = client.groups || GROUPS_DEFAULT;
@@ -75,12 +70,7 @@ module.exports = class TokenCodeGroupingRule extends BaseGroupingRule {
     }
 
     if (subject.service && subject.service === Constants.SYNC_SERVICE) {
-      let syncRolloutRate = this.SYNC_ROLLOUT_RATE;
-      if (featureFlags && featureFlags.tokenCodeClients) {
-        syncRolloutRate = featureFlags.tokenCodeClients.sync.rolloutRate;
-      }
-
-      if (this.bernoulliTrial(syncRolloutRate, subject.uniqueUserId)) {
+      if (this.bernoulliTrial(this.SYNC_ROLLOUT_RATE, subject.uniqueUserId)) {
         return this.uniformChoice(GROUPS_DEFAULT, subject.uniqueUserId);
       }
     }

diff --git a/app/tests/spec/lib/app-start.js b/app/tests/spec/lib/app-start.js
@@ -10,7 +10,6 @@ define(function (require, exports, module) {
   const BaseBroker = require('models/auth_brokers/base');
   const Constants = require('lib/constants');
   const ErrorUtils = require('lib/error-utils');
-  const ExperimentGroupingRules = require('lib/experiments/grouping-rules');
   const FxFennecV1Broker = require('models/auth_brokers/fx-fennec-v1');
   const FxFirstrunV1Broker = require('models/auth_brokers/fx-firstrun-v1');
   const FxFirstrunV2Broker = require('models/auth_brokers/fx-firstrun-v2');
@@ -39,7 +38,6 @@ define(function (require, exports, module) {
     let appStart;
     let backboneHistoryMock;
     let brokerMock;
-    let config;
     let notifier;
     let routerMock;
     let translator;
@@ -49,12 +47,6 @@ define(function (require, exports, module) {
     beforeEach(() => {
       brokerMock = new BaseBroker();
       backboneHistoryMock = new HistoryMock();
-      config = {
-        env: 'production',
-        featureFlags: {
-          foo: 'bar'
-        }
-      };
       notifier = new Notifier();
       routerMock = { navigate: sinon.spy() };
       translator = {
@@ -68,7 +60,6 @@ define(function (require, exports, module) {
 
       appStart = new AppStart({
         broker: brokerMock,
-        config,
         history: backboneHistoryMock,
         notifier,
         router: routerMock,
@@ -122,17 +113,34 @@ define(function (require, exports, module) {
         });
     });
 
-    it('initializeExperimentGroupingRules propagates env and featureFlags', () => {
-      assert.isUndefined(appStart._experimentGroupingRules);
+    it('initializeErrorMetrics skips error metrics on empty config', () => {
+      appStart.initializeExperimentGroupingRules();
+      const ableChoose = sinon.stub(appStart._experimentGroupingRules, 'choose').callsFake(() => {
+        return true;
+      });
 
+      appStart.initializeErrorMetrics();
+      assert.isUndefined(appStart._sentryMetrics);
+      ableChoose.restore();
+    });
+
+    it('initializeErrorMetrics skips error metrics if env is not defined', () => {
       appStart.initializeExperimentGroupingRules();
 
-      assert.instanceOf(appStart._experimentGroupingRules, ExperimentGroupingRules);
-      assert.equal(appStart._experimentGroupingRules._env, 'production');
-      assert.deepEqual(appStart._experimentGroupingRules._featureFlags, { foo: 'bar' });
+      appStart.initializeErrorMetrics();
+      assert.isUndefined(appStart._sentryMetrics);
     });
 
     it('initializeErrorMetrics creates error metrics', () => {
+      const appStart = new AppStart({
+        broker: brokerMock,
+        config: {
+          env: 'development'
+        },
+        history: backboneHistoryMock,
+        router: routerMock,
+        window: windowMock
+      });
       appStart.initializeExperimentGroupingRules();
 
       const ableChoose = sinon.stub(appStart._experimentGroupingRules, 'choose').callsFake(() => {
@@ -176,35 +184,6 @@ define(function (require, exports, module) {
       assert.instanceOf(appStart._refreshObserver, RefreshObserver);
     });
 
-    describe('without config', () => {
-      beforeEach(() => {
-        appStart = new AppStart({
-          broker: brokerMock,
-          history: backboneHistoryMock,
-          router: routerMock,
-          window: windowMock
-        });
-      });
-
-      it('initializeErrorMetrics skips error metrics on empty config', () => {
-        appStart.initializeExperimentGroupingRules();
-        const ableChoose = sinon.stub(appStart._experimentGroupingRules, 'choose').callsFake(() => {
-          return true;
-        });
-
-        appStart.initializeErrorMetrics();
-        assert.isUndefined(appStart._sentryMetrics);
-        ableChoose.restore();
-      });
-
-      it('initializeErrorMetrics skips error metrics if env is not defined', () => {
-        appStart.initializeExperimentGroupingRules();
-
-        appStart.initializeErrorMetrics();
-        assert.isUndefined(appStart._sentryMetrics);
-      });
-    });
-
     describe('fatalError', () => {
       var err;
       var sandbox;