Delete the ABTestController and its state (#8620)

app/scripts/controllers/ab-test.js

@@ -1,53 +0,0 @@
-import ObservableStore from 'obs-store'
-import { getRandomArrayItem } from '../lib/util'
-
-/**
- * a/b test descriptions:
- * - `fullScreenVsPopup`:
- *   - description: tests whether showing tx confirmations in full screen in the browser will increase rates of successful
- *   confirmations
- *   - groups:
- *     - popup: this is the control group, which follows the current UX of showing tx confirmations in the notification
- *     window
- *     - fullScreen: this is the only test group, which will cause users to be shown tx confirmations in a full screen
- *     browser tab
- */
-
-export default class ABTestController {
-  /**
-   * @constructor
-   * @param opts
-   */
-  constructor (opts = {}) {
-    const { initState } = opts
-    this.store = new ObservableStore(Object.assign({
-      abTests: {
-        fullScreenVsPopup: this._getRandomizedTestGroupName('fullScreenVsPopup'),
-      },
-    }, initState))
-  }
-
-  /**
-   * Returns the name of the test group to which the current user has been assigned
-   * @param {string} abTestKey - the key of the a/b test
-   * @returns {string} - the name of the assigned test group
-   */
-  getAssignedABTestGroupName (abTestKey) {
-    return this.store.getState().abTests[abTestKey]
-  }
-
-  /**
-   * Returns a randomly chosen name of a test group from a given a/b test
-   * @param {string} abTestKey - the key of the a/b test
-   * @returns {string} - the name of the randomly selected test group
-   * @private
-   */
-  _getRandomizedTestGroupName (abTestKey) {
-    const nameArray = ABTestController.abTestGroupNames[abTestKey]
-    return getRandomArrayItem(nameArray)
-  }
-}
-
-ABTestController.abTestGroupNames = {
-  fullScreenVsPopup: ['control', 'fullScreen'],
-}

app/scripts/lib/util.js

@@ -129,10 +129,6 @@ function removeListeners (listeners, emitter) {
   })
 }
 
-function getRandomArrayItem (array) {
-  return array[Math.floor((Math.random() * array.length))]
-}
-
 function mapObjectValues (object, cb) {
   const mappedObject = {}
   Object.keys(object).forEach((key) => {
@@ -167,7 +163,6 @@ export {
   hexToBn,
   bnToHex,
   BnMultiplyByFraction,
-  getRandomArrayItem,
   mapObjectValues,
   checkForError,
 }

app/scripts/metamask-controller.js

@@ -43,7 +43,6 @@ import TypedMessageManager from './lib/typed-message-manager'
 import TransactionController from './controllers/transactions'
 import TokenRatesController from './controllers/token-rates'
 import DetectTokensController from './controllers/detect-tokens'
-import ABTestController from './controllers/ab-test'
 import { PermissionsController } from './controllers/permissions'
 import getRestrictedMethods from './controllers/permissions/restrictedMethods'
 import nodeify from './lib/nodeify'
@@ -219,10 +218,6 @@ export default class MetamaskController extends EventEmitter {
       keyringMemStore: this.keyringController.memStore,
     })
 
-    this.abTestController = new ABTestController({
-      initState: initState.ABTestController,
-    })
-
     this.addressBookController = new AddressBookController(undefined, initState.AddressBookController)
 
     this.alertController = new AlertController({
@@ -304,7 +299,6 @@ export default class MetamaskController extends EventEmitter {
       AlertController: this.alertController.store,
       OnboardingController: this.onboardingController.store,
       IncomingTransactionsController: this.incomingTransactionsController.store,
-      ABTestController: this.abTestController.store,
       PermissionsController: this.permissionsController.permissions,
       PermissionsMetadata: this.permissionsController.store,
       ThreeBoxController: this.threeBoxController.store,
@@ -333,7 +327,6 @@ export default class MetamaskController extends EventEmitter {
       PermissionsController: this.permissionsController.permissions,
       PermissionsMetadata: this.permissionsController.store,
       ThreeBoxController: this.threeBoxController.store,
-      ABTestController: this.abTestController.store,
       // ENS Controller
       EnsController: this.ensController.store,
     })
@@ -449,7 +442,6 @@ export default class MetamaskController extends EventEmitter {
     const permissionsController = this.permissionsController
     const preferencesController = this.preferencesController
     const threeBoxController = this.threeBoxController
-    const abTestController = this.abTestController
     const txController = this.txController
 
     return {
@@ -572,9 +564,6 @@ export default class MetamaskController extends EventEmitter {
       turnThreeBoxSyncingOn: nodeify(threeBoxController.turnThreeBoxSyncingOn, threeBoxController),
       initializeThreeBox: nodeify(this.initializeThreeBox, this),
 
-      // a/b test controller
-      getAssignedABTestGroupName: nodeify(abTestController.getAssignedABTestGroupName, abTestController),
-
       // permissions
       approvePermissionsRequest: nodeify(permissionsController.approvePermissionsRequest, permissionsController),
       clearPermissions: permissionsController.clearPermissions.bind(permissionsController),

app/scripts/migrations/038.js

@@ -1,7 +1,5 @@
 const version = 38
 import { cloneDeep } from 'lodash'
-import ABTestController from '../controllers/ab-test'
-import { getRandomArrayItem } from '../lib/util'
 
 /**
  * The purpose of this migration is to assign all users to a test group for the fullScreenVsPopup a/b test
@@ -28,7 +26,7 @@ function transformState (state) {
         ...ABTestControllerState,
         abTests: {
           ...abTests,
-          fullScreenVsPopup: getRandomArrayItem(ABTestController.abTestGroupNames.fullScreenVsPopup),
+          fullScreenVsPopup: 'control',
         },
       },
     }

app/scripts/migrations/046.js

@@ -0,0 +1,23 @@
+const version = 46
+import { cloneDeep } from 'lodash'
+
+/**
+ * Delete {@code ABTestController} state
+ */
+export default {
+  version,
+  migrate: async function (originalVersionedData) {
+    const versionedData = cloneDeep(originalVersionedData)
+    versionedData.meta.version = version
+    const state = versionedData.data
+    versionedData.data = transformState(state)
+    return versionedData
+  },
+}
+
+function transformState (state) {
+  if (typeof state?.ABTestController !== 'undefined') {
+    delete state.ABTestController
+  }
+  return state
+}

app/scripts/migrations/index.js

@@ -56,6 +56,7 @@ const migrations = [
   require('./043').default,
   require('./044').default,
   require('./045').default,
+  require('./046').default,
 ]
 
 export default migrations

test/unit/migrations/038-test.js

@@ -1,4 +1,4 @@
-import assert from 'assert'
+import { strict as assert } from 'assert'
 import migration38 from '../../../app/scripts/migrations/038'
 
 describe('migration #38', function () {
@@ -28,7 +28,7 @@ describe('migration #38', function () {
 
     migration38.migrate(oldStorage)
       .then((newStorage) => {
-        assert(newStorage.data.ABTestController.abTests.fullScreenVsPopup.match(/control|fullScreen/))
+        assert.equal(newStorage.data.ABTestController.abTests.fullScreenVsPopup, 'control')
         done()
       })
       .catch(done)

test/unit/migrations/046-test.js

@@ -0,0 +1,52 @@
+import { strict as assert } from 'assert'
+import migration46 from '../../../app/scripts/migrations/046'
+
+describe('migration #46', function () {
+  it('should update the version metadata', async function () {
+    const oldStorage = {
+      'meta': {
+        'version': 45,
+      },
+      'data': {},
+    }
+
+    const newStorage = await migration46.migrate(oldStorage)
+    assert.deepEqual(newStorage.meta, {
+      'version': 46,
+    })
+  })
+
+  it('should delete ABTestController state', async function () {
+    const oldStorage = {
+      meta: {},
+      data: {
+        ABTestController: {
+          abTests: {
+            fullScreenVsPopup: 'control',
+          },
+        },
+        foo: 'bar',
+      },
+    }
+
+    const newStorage = await migration46.migrate(oldStorage)
+    assert.deepEqual(newStorage.data, {
+      foo: 'bar',
+    })
+  })
+
+  it('should do nothing if ABTestController state does not exist', async function () {
+    const oldStorage = {
+      meta: {},
+      data: {
+        AppStateController: {
+          bar: 'baz',
+        },
+        foo: 'bar',
+      },
+    }
+
+    const newStorage = await migration46.migrate(oldStorage)
+    assert.deepEqual(oldStorage.data, newStorage.data)
+  })
+})

ui/app/pages/confirm-transaction/confirm-transaction.component.js

@@ -45,8 +45,6 @@ export default class ConfirmTransaction extends Component {
     paramsTransactionId: PropTypes.string,
     getTokenParams: PropTypes.func,
     isTokenMethodAction: PropTypes.bool,
-    fullScreenVsPopupTestGroup: PropTypes.string,
-    trackABTest: PropTypes.bool,
   }
 
   componentDidMount () {
@@ -61,8 +59,6 @@ export default class ConfirmTransaction extends Component {
       paramsTransactionId,
       getTokenParams,
       isTokenMethodAction,
-      fullScreenVsPopupTestGroup,
-      trackABTest,
     } = this.props
 
     if (!totalUnapprovedCount && !send.to) {
@@ -79,16 +75,6 @@ export default class ConfirmTransaction extends Component {
     if (txId) {
       this.props.setTransactionToConfirm(txId)
     }
-
-    if (trackABTest) {
-      this.context.metricsEvent({
-        eventOpts: {
-          category: 'abtesting',
-          action: 'fullScreenVsPopup',
-          name: fullScreenVsPopupTestGroup === 'fullScreen' ? 'fullscreen' : 'original',
-        },
-      })
-    }
   }
 
   componentDidUpdate (prevProps) {

ui/app/pages/confirm-transaction/confirm-transaction.container.js

@@ -24,7 +24,6 @@ const mapStateToProps = (state, ownProps) => {
     metamask: {
       send,
       unapprovedTxs,
-      abTests: { fullScreenVsPopup },
     },
   } = state
   const { match: { params = {} } } = ownProps
@@ -37,8 +36,6 @@ const mapStateToProps = (state, ownProps) => {
     : {}
   const { id: transactionId, transactionCategory } = transaction
 
-  const trackABTest = false
-
   return {
     totalUnapprovedCount: totalUnconfirmed,
     send,
@@ -48,8 +45,6 @@ const mapStateToProps = (state, ownProps) => {
     transactionId: transactionId && String(transactionId),
     transaction,
     isTokenMethodAction: isTokenMethodAction(transactionCategory),
-    trackABTest,
-    fullScreenVsPopupTestGroup: fullScreenVsPopup,
   }
 }