From 101fe0b27a5b4f0188a31808aab3f9c59bddb641 Mon Sep 17 00:00:00 2001 From: Dan J Miller Date: Fri, 22 Jul 2022 18:09:48 -0230 Subject: [PATCH] Metrics adjustments (#15313) * Don't send errors to sentry if users have not opted-in to participate in metametrics * Don't capture opt-out metrics * Move the metrics-opt in screen to immediately after the welcome screen * Ensure that global.getSentryState is set in the background * Fix e2e tests after rearranging onboardin flow * Fix unit tests * More e2e test fixes * Remove unnecessary wrappers around capture exception --- app/scripts/background.js | 17 +++++++++++ app/scripts/lib/setupSentry.js | 12 +++++++- shared/modules/object.utils.js | 22 ++++++++++++++ test/e2e/helpers.js | 12 ++++---- test/e2e/metamask-ui.spec.js | 8 ++--- test/e2e/tests/incremental-security.spec.js | 6 ++-- test/e2e/tests/metamask-responsive-ui.spec.js | 6 ++-- ui/index.js | 24 +-------------- .../metametrics-opt-in.component.js | 29 ++----------------- .../metametrics-opt-in.container.js | 2 -- .../select-action/select-action.component.js | 9 ++++-- .../welcome/welcome.component.js | 21 +++++++++++--- .../welcome/welcome.container.js | 7 ++++- .../first-time-flow/welcome/welcome.test.js | 8 ++--- 14 files changed, 103 insertions(+), 80 deletions(-) create mode 100644 shared/modules/object.utils.js diff --git a/app/scripts/background.js b/app/scripts/background.js index 311cd2781..0246c5d73 100644 --- a/app/scripts/background.js +++ b/app/scripts/background.js @@ -24,11 +24,14 @@ import { REJECT_NOTFICIATION_CLOSE_SIG, } from '../../shared/constants/metametrics'; import { isManifestV3 } from '../../shared/modules/mv3.utils'; +import { maskObject } from '../../shared/modules/object.utils'; import migrations from './migrations'; import Migrator from './lib/migrator'; import ExtensionPlatform from './platforms/extension'; import LocalStore from './lib/local-store'; import ReadOnlyNetworkStore from './lib/network-store'; +import { SENTRY_STATE } from './lib/setupSentry'; + import createStreamSink from './lib/createStreamSink'; import NotificationManager, { NOTIFICATION_MANAGER_EVENTS, @@ -353,6 +356,8 @@ function setupController(initState, initLangCode, remoteSourcePort) { }, ); + setupSentryGetStateGlobal(controller.store); + /** * Assigns the given state to the versioned object (with metadata), and returns that. * @@ -755,3 +760,15 @@ browser.runtime.onInstalled.addListener(({ reason }) => { platform.openExtensionInBrowser(); } }); + +function setupSentryGetStateGlobal(store) { + global.getSentryState = function () { + const fullState = store.getState(); + const debugState = maskObject(fullState, SENTRY_STATE); + return { + browser: window.navigator.userAgent, + store: debugState, + version: global.platform.getVersion(), + }; + }; +} diff --git a/app/scripts/lib/setupSentry.js b/app/scripts/lib/setupSentry.js index 7332041bb..196389439 100644 --- a/app/scripts/lib/setupSentry.js +++ b/app/scripts/lib/setupSentry.js @@ -103,7 +103,17 @@ export default function setupSentry({ release, getState }) { environment, integrations: [new Dedupe(), new ExtraErrorData()], release, - beforeSend: (report) => rewriteReport(report), + beforeSend: (report) => { + if (getState) { + const appState = getState(); + if (!appState?.store?.metamask?.participateInMetaMetrics) { + return null; + } + } else { + return null; + } + return rewriteReport(report); + }, }); function rewriteReport(report) { diff --git a/shared/modules/object.utils.js b/shared/modules/object.utils.js new file mode 100644 index 000000000..ea2af06c4 --- /dev/null +++ b/shared/modules/object.utils.js @@ -0,0 +1,22 @@ +/** + * Return a "masked" copy of the given object. + * + * The returned object includes only the properties present in the mask. The + * mask is an object that mirrors the structure of the given object, except + * the only values are `true` or a sub-mask. `true` implies the property + * should be included, and a sub-mask implies the property should be further + * masked according to that sub-mask. + * + * @param {Object} object - The object to mask + * @param {Object} mask - The mask to apply to the object + */ +export function maskObject(object, mask) { + return Object.keys(object).reduce((state, key) => { + if (mask[key] === true) { + state[key] = object[key]; + } else if (mask[key]) { + state[key] = maskObject(object[key], mask[key]); + } + return state; + }, {}); +} diff --git a/test/e2e/helpers.js b/test/e2e/helpers.js index 72a82acc8..6e1230a05 100644 --- a/test/e2e/helpers.js +++ b/test/e2e/helpers.js @@ -248,12 +248,12 @@ const completeImportSRPOnboardingFlow = async ( tag: 'button', }); - // clicks the "Import Wallet" option - await driver.clickElement({ text: 'Import wallet', tag: 'button' }); - // clicks the "No thanks" option on the metametrics opt-in screen await driver.clickElement('.btn-secondary'); + // clicks the "Import Wallet" option + await driver.clickElement({ text: 'Import wallet', tag: 'button' }); + // Import Secret Recovery Phrase await driver.pasteIntoField( '[data-testid="import-srp__srp-word-0"]', @@ -290,12 +290,12 @@ const completeImportSRPOnboardingFlowWordByWord = async ( tag: 'button', }); - // clicks the "Import Wallet" option - await driver.clickElement({ text: 'Import wallet', tag: 'button' }); - // clicks the "No thanks" option on the metametrics opt-in screen await driver.clickElement('.btn-secondary'); + // clicks the "Import Wallet" option + await driver.clickElement({ text: 'Import wallet', tag: 'button' }); + const words = seedPhrase.split(' '); for (const word of words) { await driver.pasteIntoField( diff --git a/test/e2e/metamask-ui.spec.js b/test/e2e/metamask-ui.spec.js index 854b597a0..9e85bff84 100644 --- a/test/e2e/metamask-ui.spec.js +++ b/test/e2e/metamask-ui.spec.js @@ -99,13 +99,13 @@ describe('MetaMask', function () { await driver.delay(largeDelayMs); }); - it('clicks the "Create New Wallet" option', async function () { - await driver.clickElement({ text: 'Create a Wallet', tag: 'button' }); + it('clicks the "No thanks" option on the metametrics opt-in screen', async function () { + await driver.clickElement('.btn-secondary'); await driver.delay(largeDelayMs); }); - it('clicks the "No thanks" option on the metametrics opt-in screen', async function () { - await driver.clickElement('.btn-secondary'); + it('clicks the "Create New Wallet" option', async function () { + await driver.clickElement({ text: 'Create a Wallet', tag: 'button' }); await driver.delay(largeDelayMs); }); diff --git a/test/e2e/tests/incremental-security.spec.js b/test/e2e/tests/incremental-security.spec.js index a94f0f3e2..a12f49831 100644 --- a/test/e2e/tests/incremental-security.spec.js +++ b/test/e2e/tests/incremental-security.spec.js @@ -38,12 +38,12 @@ describe('Incremental Security', function () { tag: 'button', }); - // clicks the "Create New Wallet" option - await driver.clickElement({ text: 'Create a Wallet', tag: 'button' }); - // clicks the "No thanks" option on the metametrics opt-in screen await driver.clickElement('.btn-secondary'); + // clicks the "Create New Wallet" option + await driver.clickElement({ text: 'Create a Wallet', tag: 'button' }); + // accepts a secure password await driver.fill( '.first-time-flow__form #create-password', diff --git a/test/e2e/tests/metamask-responsive-ui.spec.js b/test/e2e/tests/metamask-responsive-ui.spec.js index e51c449de..3b796834e 100644 --- a/test/e2e/tests/metamask-responsive-ui.spec.js +++ b/test/e2e/tests/metamask-responsive-ui.spec.js @@ -87,12 +87,12 @@ describe('MetaMask Responsive UI', function () { }); await driver.delay(tinyDelayMs); - // clicks the "Create New Wallet" option - await driver.clickElement({ text: 'Create a Wallet', tag: 'button' }); - // clicks the "I Agree" option on the metametrics opt-in screen await driver.clickElement('.btn-primary'); + // clicks the "Create New Wallet" option + await driver.clickElement({ text: 'Create a Wallet', tag: 'button' }); + // accepts a secure password await driver.fill( '.first-time-flow__form #create-password', diff --git a/ui/index.js b/ui/index.js index be61e7c9c..842d0f2a9 100644 --- a/ui/index.js +++ b/ui/index.js @@ -7,6 +7,7 @@ import browser from 'webextension-polyfill'; import { getEnvironmentType } from '../app/scripts/lib/util'; import { ALERT_TYPES } from '../shared/constants/alerts'; +import { maskObject } from '../shared/modules/object.utils'; import { SENTRY_STATE } from '../app/scripts/lib/setupSentry'; import { ENVIRONMENT_TYPE_POPUP } from '../shared/constants/app'; import * as actions from './store/actions'; @@ -171,29 +172,6 @@ async function startApp(metamaskState, backgroundConnection, opts) { return store; } -/** - * Return a "masked" copy of the given object. - * - * The returned object includes only the properties present in the mask. The - * mask is an object that mirrors the structure of the given object, except - * the only values are `true` or a sub-mask. `true` implies the property - * should be included, and a sub-mask implies the property should be further - * masked according to that sub-mask. - * - * @param {Object} object - The object to mask - * @param {Object} mask - The mask to apply to the object - */ -function maskObject(object, mask) { - return Object.keys(object).reduce((state, key) => { - if (mask[key] === true) { - state[key] = object[key]; - } else if (mask[key]) { - state[key] = maskObject(object[key], mask[key]); - } - return state; - }, {}); -} - function setupDebuggingHelpers(store) { window.getCleanAppState = async function () { const state = clone(store.getState()); diff --git a/ui/pages/first-time-flow/metametrics-opt-in/metametrics-opt-in.component.js b/ui/pages/first-time-flow/metametrics-opt-in/metametrics-opt-in.component.js index 673d1c7f3..7fc24e657 100644 --- a/ui/pages/first-time-flow/metametrics-opt-in/metametrics-opt-in.component.js +++ b/ui/pages/first-time-flow/metametrics-opt-in/metametrics-opt-in.component.js @@ -3,12 +3,12 @@ import PropTypes from 'prop-types'; import MetaFoxLogo from '../../../components/ui/metafox-logo'; import PageContainerFooter from '../../../components/ui/page-container/page-container-footer'; import { EVENT } from '../../../../shared/constants/metametrics'; +import { INITIALIZE_SELECT_ACTION_ROUTE } from '../../../helpers/constants/routes'; export default class MetaMetricsOptIn extends Component { static propTypes = { history: PropTypes.object, setParticipateInMetaMetrics: PropTypes.func, - nextRoute: PropTypes.string, firstTimeSelectionMetaMetricsName: PropTypes.string, participateInMetaMetrics: PropTypes.bool, }; @@ -21,7 +21,6 @@ export default class MetaMetricsOptIn extends Component { render() { const { trackEvent, t } = this.context; const { - nextRoute, history, setParticipateInMetaMetrics, firstTimeSelectionMetaMetricsName, @@ -105,29 +104,7 @@ export default class MetaMetricsOptIn extends Component { onCancel={async () => { await setParticipateInMetaMetrics(false); - try { - if ( - participateInMetaMetrics === null || - participateInMetaMetrics === true - ) { - await trackEvent( - { - category: EVENT.CATEGORIES.ONBOARDING, - event: 'Metrics Opt Out', - properties: { - action: 'Metrics Option', - legacy_event: true, - }, - }, - { - isOptIn: true, - flushImmediately: true, - }, - ); - } - } finally { - history.push(nextRoute); - } + history.push(INITIALIZE_SELECT_ACTION_ROUTE); }} cancelText={t('noThanks')} hideCancel={false} @@ -177,7 +154,7 @@ export default class MetaMetricsOptIn extends Component { ); await Promise.all(metrics); } finally { - history.push(nextRoute); + history.push(INITIALIZE_SELECT_ACTION_ROUTE); } }} submitText={t('affirmAgree')} diff --git a/ui/pages/first-time-flow/metametrics-opt-in/metametrics-opt-in.container.js b/ui/pages/first-time-flow/metametrics-opt-in/metametrics-opt-in.container.js index e336a9dd2..9e607ccf7 100644 --- a/ui/pages/first-time-flow/metametrics-opt-in/metametrics-opt-in.container.js +++ b/ui/pages/first-time-flow/metametrics-opt-in/metametrics-opt-in.container.js @@ -1,6 +1,5 @@ import { connect } from 'react-redux'; import { setParticipateInMetaMetrics } from '../../../store/actions'; -import { getFirstTimeFlowTypeRoute } from '../../../selectors'; import MetaMetricsOptIn from './metametrics-opt-in.component'; const firstTimeFlowTypeNameMap = { @@ -12,7 +11,6 @@ const mapStateToProps = (state) => { const { firstTimeFlowType, participateInMetaMetrics } = state.metamask; return { - nextRoute: getFirstTimeFlowTypeRoute(state), firstTimeSelectionMetaMetricsName: firstTimeFlowTypeNameMap[firstTimeFlowType], participateInMetaMetrics, diff --git a/ui/pages/first-time-flow/select-action/select-action.component.js b/ui/pages/first-time-flow/select-action/select-action.component.js index 7feadb9a3..51d2876d7 100644 --- a/ui/pages/first-time-flow/select-action/select-action.component.js +++ b/ui/pages/first-time-flow/select-action/select-action.component.js @@ -2,7 +2,10 @@ import React, { PureComponent } from 'react'; import PropTypes from 'prop-types'; import Button from '../../../components/ui/button'; import MetaFoxLogo from '../../../components/ui/metafox-logo'; -import { INITIALIZE_METAMETRICS_OPT_IN_ROUTE } from '../../../helpers/constants/routes'; +import { + INITIALIZE_CREATE_PASSWORD_ROUTE, + INITIALIZE_IMPORT_WITH_SEED_PHRASE_ROUTE, +} from '../../../helpers/constants/routes'; export default class SelectAction extends PureComponent { static propTypes = { @@ -26,12 +29,12 @@ export default class SelectAction extends PureComponent { handleCreate = () => { this.props.setFirstTimeFlowType('create'); - this.props.history.push(INITIALIZE_METAMETRICS_OPT_IN_ROUTE); + this.props.history.push(INITIALIZE_CREATE_PASSWORD_ROUTE); }; handleImport = () => { this.props.setFirstTimeFlowType('import'); - this.props.history.push(INITIALIZE_METAMETRICS_OPT_IN_ROUTE); + this.props.history.push(INITIALIZE_IMPORT_WITH_SEED_PHRASE_ROUTE); }; render() { diff --git a/ui/pages/first-time-flow/welcome/welcome.component.js b/ui/pages/first-time-flow/welcome/welcome.component.js index 20a929372..46bf36d22 100644 --- a/ui/pages/first-time-flow/welcome/welcome.component.js +++ b/ui/pages/first-time-flow/welcome/welcome.component.js @@ -6,6 +6,7 @@ import Button from '../../../components/ui/button'; import { INITIALIZE_CREATE_PASSWORD_ROUTE, INITIALIZE_SELECT_ACTION_ROUTE, + INITIALIZE_METAMETRICS_OPT_IN_ROUTE, } from '../../../helpers/constants/routes'; import { isBeta } from '../../../helpers/utils/build-types'; import WelcomeFooter from './welcome-footer.component'; @@ -16,6 +17,7 @@ export default class Welcome extends PureComponent { history: PropTypes.object, participateInMetaMetrics: PropTypes.bool, welcomeScreenSeen: PropTypes.bool, + isInitialized: PropTypes.bool, }; static contextTypes = { @@ -29,17 +31,28 @@ export default class Welcome extends PureComponent { } componentDidMount() { - const { history, participateInMetaMetrics, welcomeScreenSeen } = this.props; + const { + history, + participateInMetaMetrics, + welcomeScreenSeen, + isInitialized, + } = this.props; - if (welcomeScreenSeen && participateInMetaMetrics !== null) { + if ( + welcomeScreenSeen && + isInitialized && + participateInMetaMetrics !== null + ) { history.push(INITIALIZE_CREATE_PASSWORD_ROUTE); - } else if (welcomeScreenSeen) { + } else if (welcomeScreenSeen && participateInMetaMetrics !== null) { history.push(INITIALIZE_SELECT_ACTION_ROUTE); + } else if (welcomeScreenSeen) { + history.push(INITIALIZE_METAMETRICS_OPT_IN_ROUTE); } } handleContinue = () => { - this.props.history.push(INITIALIZE_SELECT_ACTION_ROUTE); + this.props.history.push(INITIALIZE_METAMETRICS_OPT_IN_ROUTE); }; render() { diff --git a/ui/pages/first-time-flow/welcome/welcome.container.js b/ui/pages/first-time-flow/welcome/welcome.container.js index 77909d25b..27bd2e198 100644 --- a/ui/pages/first-time-flow/welcome/welcome.container.js +++ b/ui/pages/first-time-flow/welcome/welcome.container.js @@ -5,11 +5,16 @@ import { closeWelcomeScreen } from '../../../store/actions'; import Welcome from './welcome.component'; const mapStateToProps = ({ metamask }) => { - const { welcomeScreenSeen, participateInMetaMetrics } = metamask; + const { + welcomeScreenSeen, + participateInMetaMetrics, + isInitialized, + } = metamask; return { welcomeScreenSeen, participateInMetaMetrics, + isInitialized, }; }; diff --git a/ui/pages/first-time-flow/welcome/welcome.test.js b/ui/pages/first-time-flow/welcome/welcome.test.js index 14d1a22cc..2c0f79106 100644 --- a/ui/pages/first-time-flow/welcome/welcome.test.js +++ b/ui/pages/first-time-flow/welcome/welcome.test.js @@ -15,7 +15,7 @@ describe('Welcome', () => { sinon.restore(); }); - it('routes to select action when participateInMetaMetrics is not initialized', () => { + it('routes to the metametrics screen when participateInMetaMetrics is not initialized', () => { const props = { history: { push: sinon.spy(), @@ -32,11 +32,11 @@ describe('Welcome', () => { ); getStartedButton.simulate('click'); expect(props.history.push.getCall(0).args[0]).toStrictEqual( - '/initialize/select-action', + '/initialize/metametrics-opt-in', ); }); - it('routes to correct password when participateInMetaMetrics is initialized', () => { + it('routes to select action when participateInMetaMetrics is initialized', () => { const props = { welcomeScreenSeen: true, participateInMetaMetrics: false, @@ -55,7 +55,7 @@ describe('Welcome', () => { ); getStartedButton.simulate('click'); expect(props.history.push.getCall(0).args[0]).toStrictEqual( - '/initialize/create-password', + '/initialize/select-action', ); }); });