Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 9 commits into from
Sep 24, 2019
18 changes: 17 additions & 1 deletion app/scripts/background.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -436,6 +438,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
Expand Down
57 changes: 57 additions & 0 deletions app/scripts/controllers/ab-test.js
Original file line number Diff line number Diff line change
@@ -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

5 changes: 5 additions & 0 deletions app/scripts/lib/util.js
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,10 @@ function removeListeners (listeners, emitter) {
})
}

function getRandomArrayItem (array) {
return array[Math.floor((Math.random() * array.length))]
}

module.exports = {
removeListeners,
applyListeners,
Expand All @@ -154,4 +158,5 @@ module.exports = {
hexToBn,
bnToHex,
BnMultiplyByFraction,
getRandomArrayItem,
}
11 changes: 11 additions & 0 deletions app/scripts/metamask-controller.js
Original file line number Diff line number Diff line change
Expand Up @@ -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')
Expand Down Expand Up @@ -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,
Expand All @@ -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, {
Expand All @@ -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))
}
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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),
}
}

Expand Down
37 changes: 37 additions & 0 deletions app/scripts/migrations/038.js
Original file line number Diff line number Diff line change
@@ -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
}
1 change: 1 addition & 0 deletions app/scripts/migrations/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -48,4 +48,5 @@ module.exports = [
require('./035'),
require('./036'),
require('./037'),
require('./038'),
]
3 changes: 3 additions & 0 deletions development/states/confirm-sig-requests.json
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,9 @@
"name": "Send Account 4"
}
},
"abTests": {
"fullScreenVsPopup": "control"
},
"cachedBalances": {},
"conversionRate": 1200.88200327,
"conversionDate": 1489013762,
Expand Down
3 changes: 3 additions & 0 deletions development/states/currency-localization.json
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,9 @@
"name": "Send Account 4"
}
},
"abTests": {
"fullScreenVsPopup": "control"
},
"cachedBalances": {},
"unapprovedTxs": {},
"conversionRate": 19855,
Expand Down
3 changes: 3 additions & 0 deletions development/states/tx-list-items.json
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,9 @@
"name": "Send Account 4"
}
},
"abTests": {
"fullScreenVsPopup": "control"
},
"cachedBalances": {},
"currentCurrency": "USD",
"conversionRate": 1200.88200327,
Expand Down
60 changes: 60 additions & 0 deletions test/unit/migrations/038-test.js
Original file line number Diff line number Diff line change
@@ -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)
})
})
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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 () {
Expand All @@ -53,6 +59,8 @@ export default class ConfirmTransaction extends Component {
paramsTransactionId,
getTokenParams,
isTokenMethodAction,
fullScreenVsPopupTestGroup,
trackABTest,
} = this.props

if (!totalUnapprovedCount && !send.to) {
Expand All @@ -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) {
Expand Down
Loading