Add a/b test for full screen transaction confirmations (#7162)

* Adds ab test controller with a fullScreenVsPopup test

* Add migration for fullScreenVsPopup state

* Move abtest state under an 'abtests' object.

* MetaMask shows fullScreen group of a/b test unapproved txs in a full browser tab

* Ensure cancel metrics event in confirm-transaction-base.component.js is sent in all cases

* Switch to existing tab for unapproved tx if it exists when opening in full screen

* Send metrics event for entering a/b test from confirm screen

* Fix lint, unit and integration tests related to a/b test code

* Remove unnecessary tabs.query call in triggerUiInNewTab

app/scripts/background.js

@@ -233,11 +233,13 @@ function setupController (initState, initLangCode) {
   //
   // MetaMask Controller
   //
+  const { ABTestController = {} } = initState
+  const { abTests = {} } = ABTestController
 
   const controller = new MetamaskController({
     // User confirmation callbacks:
     showUnconfirmedMessage: triggerUi,
-    showUnapprovedTx: triggerUi,
+    showUnapprovedTx: abTests.fullScreenVsPopup === 'fullScreen' ? triggerUiInNewTab : triggerUi,
     openPopup: openPopup,
     closePopup: notificationManager.closePopup.bind(notificationManager),
     // initial state
@@ -441,6 +443,20 @@ function triggerUi () {
   })
 }
 
+/**
+ * Opens a new browser tab for user confirmation
+ */
+function triggerUiInNewTab () {
+  const tabIdsArray = Object.keys(openMetamaskTabsIDs)
+  if (tabIdsArray.length) {
+    extension.tabs.update(parseInt(tabIdsArray[0], 10), { 'active': true }, () => {
+      extension.tabs.reload(parseInt(tabIdsArray[0], 10))
+    })
+  } else {
+    platform.openExtensionInBrowser()
+  }
+}
+
 /**
  * Opens the browser popup for user confirmation of watchAsset
  * then it waits until user interact with the UI

app/scripts/controllers/ab-test.js

@@ -0,0 +1,57 @@
+const ObservableStore = require('obs-store')
+const extend = require('xtend')
+const { getRandomArrayItem } = require('../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
+ */
+
+class ABTestController {
+  /**
+   * @constructor
+   * @param opts
+   */
+  constructor (opts = {}) {
+    const { initState } = opts
+    this.store = new ObservableStore(extend({
+      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
+   * @return {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
+   * @return {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'],
+}
+
+module.exports = ABTestController
+

app/scripts/lib/util.js

@@ -144,6 +144,10 @@ function removeListeners (listeners, emitter) {
   })
 }
 
+function getRandomArrayItem (array) {
+  return array[Math.floor((Math.random() * array.length))]
+}
+
 module.exports = {
   removeListeners,
   applyListeners,
@@ -154,4 +158,5 @@ module.exports = {
   hexToBn,
   bnToHex,
   BnMultiplyByFraction,
+  getRandomArrayItem,
 }

app/scripts/metamask-controller.js

@@ -39,6 +39,7 @@ const TransactionController = require('./controllers/transactions')
 const TokenRatesController = require('./controllers/token-rates')
 const DetectTokensController = require('./controllers/detect-tokens')
 const ProviderApprovalController = require('./controllers/provider-approval')
+const ABTestController = require('./controllers/ab-test')
 const nodeify = require('./lib/nodeify')
 const accountImporter = require('./account-import-strategies')
 const getBuyEthUrl = require('./lib/buy-eth-url')
@@ -270,6 +271,10 @@ module.exports = class MetamaskController extends EventEmitter {
       preferencesController: this.preferencesController,
     })
 
+    this.abTestController = new ABTestController({
+      initState: initState.ABTestController,
+    })
+
     this.store.updateStructure({
       AppStateController: this.appStateController.store,
       TransactionController: this.txController.store,
@@ -285,6 +290,7 @@ module.exports = class MetamaskController extends EventEmitter {
       ProviderApprovalController: this.providerApprovalController.store,
       IncomingTransactionsController: this.incomingTransactionsController.store,
       ThreeBoxController: this.threeBoxController.store,
+      ABTestController: this.abTestController.store,
     })
 
     this.memStore = new ComposableObservableStore(null, {
@@ -311,6 +317,7 @@ module.exports = class MetamaskController extends EventEmitter {
       IncomingTransactionsController: this.incomingTransactionsController.store,
       // ThreeBoxController
       ThreeBoxController: this.threeBoxController.store,
+      ABTestController: this.abTestController.store,
     })
     this.memStore.subscribe(this.sendUpdate.bind(this))
   }
@@ -426,6 +433,7 @@ module.exports = class MetamaskController extends EventEmitter {
     const providerApprovalController = this.providerApprovalController
     const onboardingController = this.onboardingController
     const threeBoxController = this.threeBoxController
+    const abTestController = this.abTestController
 
     return {
       // etc
@@ -539,6 +547,9 @@ module.exports = class MetamaskController extends EventEmitter {
       getThreeBoxLastUpdated: nodeify(threeBoxController.getLastUpdated, threeBoxController),
       turnThreeBoxSyncingOn: nodeify(threeBoxController.turnThreeBoxSyncingOn, threeBoxController),
       initializeThreeBox: nodeify(this.initializeThreeBox, this),
+
+      // a/b test controller
+      getAssignedABTestGroupName: nodeify(abTestController.getAssignedABTestGroupName, abTestController),
     }
   }
 

app/scripts/migrations/038.js

@@ -0,0 +1,37 @@
+const version = 38
+const clone = require('clone')
+const ABTestController = require('../controllers/ab-test')
+const { getRandomArrayItem } = require('../lib/util')
+
+/**
+ * The purpose of this migration is to assign all users to a test group for the fullScreenVsPopup a/b test
+ */
+module.exports = {
+  version,
+  migrate: async function (originalVersionedData) {
+    const versionedData = clone(originalVersionedData)
+    versionedData.meta.version = version
+    const state = versionedData.data
+    versionedData.data = transformState(state)
+    return versionedData
+  },
+}
+
+function transformState (state) {
+  const { ABTestController: ABTestControllerState = {} } = state
+  const { abTests = {} } = ABTestControllerState
+
+  if (!abTests.fullScreenVsPopup) {
+    state = {
+      ...state,
+      ABTestController: {
+        ...ABTestControllerState,
+        abTests: {
+          ...abTests,
+          fullScreenVsPopup: getRandomArrayItem(ABTestController.abTestGroupNames.fullScreenVsPopup),
+        },
+      },
+    }
+  }
+  return state
+}

app/scripts/migrations/index.js

@@ -48,4 +48,5 @@ module.exports = [
   require('./035'),
   require('./036'),
   require('./037'),
+  require('./038'),
 ]

development/states/confirm-sig-requests.json

@@ -23,6 +23,9 @@
         "name": "Send Account 4"
       }
     },
+    "abTests": {
+      "fullScreenVsPopup": "control"
+    },
     "cachedBalances": {},
     "conversionRate": 1200.88200327,
     "conversionDate": 1489013762,

development/states/currency-localization.json

@@ -23,6 +23,9 @@
         "name": "Send Account 4"
       }
     },
+    "abTests": {
+      "fullScreenVsPopup": "control"
+    },
     "cachedBalances": {},
     "unapprovedTxs": {},
     "conversionRate": 19855,

development/states/tx-list-items.json

@@ -23,6 +23,9 @@
         "name": "Send Account 4"
       }
     },
+    "abTests": {
+      "fullScreenVsPopup": "control"
+    },
     "cachedBalances": {},
     "currentCurrency": "USD",
     "conversionRate": 1200.88200327,

test/unit/migrations/038-test.js

@@ -0,0 +1,60 @@
+const assert = require('assert')
+const migration38 = require('../../../app/scripts/migrations/038')
+
+describe('migration #38', () => {
+  it('should update the version metadata', (done) => {
+    const oldStorage = {
+      'meta': {
+        'version': 37,
+      },
+      'data': {},
+    }
+
+    migration38.migrate(oldStorage)
+      .then((newStorage) => {
+        assert.deepEqual(newStorage.meta, {
+          'version': 38,
+        })
+        done()
+      })
+      .catch(done)
+  })
+
+  it('should add a fullScreenVsPopup property set to either "control" or "fullScreen"', (done) => {
+    const oldStorage = {
+      'meta': {},
+      'data': {},
+    }
+
+    migration38.migrate(oldStorage)
+      .then((newStorage) => {
+        assert(newStorage.data.ABTestController.abTests.fullScreenVsPopup.match(/control|fullScreen/))
+        done()
+      })
+      .catch(done)
+  })
+
+  it('should leave the fullScreenVsPopup property unchanged if it exists', (done) => {
+    const oldStorage = {
+      'meta': {},
+      'data': {
+        'ABTestController': {
+          abTests: {
+            'fullScreenVsPopup': 'fullScreen',
+          },
+        },
+      },
+    }
+
+    migration38.migrate(oldStorage)
+      .then((newStorage) => {
+        assert.deepEqual(newStorage.data.ABTestController, {
+          abTests: {
+            'fullScreenVsPopup': 'fullScreen',
+          },
+        })
+        done()
+      })
+      .catch(done)
+  })
+})

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

@@ -349,19 +349,19 @@ export default class ConfirmTransactionBase extends Component {
     const { metricsEvent } = this.context
     const { onCancel, txData, cancelTransaction, history, clearConfirmTransaction, actionKey, txData: { origin }, methodData = {} } = this.props
 
+    metricsEvent({
+      eventOpts: {
+        category: 'Transactions',
+        action: 'Confirm Screen',
+        name: 'Cancel',
+      },
+      customVariables: {
+        recipientKnown: null,
+        functionType: actionKey || getMethodName(methodData.name) || 'contractInteraction',
+        origin,
+      },
+    })
     if (onCancel) {
-      metricsEvent({
-        eventOpts: {
-          category: 'Transactions',
-          action: 'Confirm Screen',
-          name: 'Cancel',
-        },
-        customVariables: {
-          recipientKnown: null,
-          functionType: actionKey || getMethodName(methodData.name) || 'contractInteraction',
-          origin,
-        },
-      })
       onCancel(txData)
     } else {
       cancelTransaction(txData)

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

@@ -23,6 +23,10 @@ import {
 } from '../../helpers/constants/routes'
 
 export default class ConfirmTransaction extends Component {
+  static contextTypes = {
+    metricsEvent: PropTypes.func,
+  }
+
   static propTypes = {
     history: PropTypes.object.isRequired,
     totalUnapprovedCount: PropTypes.number.isRequired,
@@ -39,6 +43,8 @@ export default class ConfirmTransaction extends Component {
     paramsTransactionId: PropTypes.string,
     getTokenParams: PropTypes.func,
     isTokenMethodAction: PropTypes.bool,
+    fullScreenVsPopupTestGroup: PropTypes.string,
+    trackABTest: PropTypes.bool,
   }
 
   componentDidMount () {
@@ -53,6 +59,8 @@ export default class ConfirmTransaction extends Component {
       paramsTransactionId,
       getTokenParams,
       isTokenMethodAction,
+      fullScreenVsPopupTestGroup,
+      trackABTest,
     } = this.props
 
     if (!totalUnapprovedCount && !send.to) {
@@ -67,6 +75,16 @@ export default class ConfirmTransaction extends Component {
     }
     const txId = transactionId || paramsTransactionId
     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

@@ -20,7 +20,14 @@ import ConfirmTransaction from './confirm-transaction.component'
 import { unconfirmedTransactionsListSelector } from '../../selectors/confirm-transaction'
 
 const mapStateToProps = (state, ownProps) => {
-  const { metamask: { send, unapprovedTxs }, confirmTransaction } = state
+  const {
+    metamask: {
+      send,
+      unapprovedTxs,
+      abTests: { fullScreenVsPopup },
+    },
+    confirmTransaction,
+  } = state
   const { match: { params = {} } } = ownProps
   const { id } = params
 
@@ -29,7 +36,9 @@ const mapStateToProps = (state, ownProps) => {
   const transaction = totalUnconfirmed
     ? unapprovedTxs[id] || unconfirmedTransactions[totalUnconfirmed - 1]
     : {}
-  const { id: transactionId, transactionCategory } = transaction
+  const { id: transactionId, transactionCategory, origin } = transaction
+
+  const trackABTest = origin !== 'MetaMask'
 
   return {
     totalUnapprovedCount: totalUnconfirmed,
@@ -42,6 +51,8 @@ const mapStateToProps = (state, ownProps) => {
     unconfirmedTransactions,
     transaction,
     isTokenMethodAction: isTokenMethodAction(transactionCategory),
+    trackABTest,
+    fullScreenVsPopupTestGroup: fullScreenVsPopup,
   }
 }