From 8713927e5e08e45f7653f32af7ea1cb08ef2a2be Mon Sep 17 00:00:00 2001 From: Mark Stacey Date: Fri, 7 Aug 2020 14:57:27 -0300 Subject: [PATCH] Remove `url` parameter from `metricsEvent` (#9157) * Remove `url` parameter from `metricsEvent` The `url` parameter was used to override the `currentPath`, but it never worked correctly. It was supposed to be used for setting the `url` query parameter that was sent to Matomo, but `currentPath` was always used even if it `url` was set and `currentPath` was empty. Instead, `currentPath` is now always used. There was never a need to provide an "override" for `currentPath` when it can be set directly. The metrics provider does set `currentPath` automatically by default, but this can be overwritten already by passing a second parameter to `metricsEvent`. There were two places this `url` parameter was being used: background events, and path changes. Background events were submitted with no `currentPath`, so because of the bug with the `url` parameter, the metrics utility would crash upon each event. So those were never actually sent. This commit will fix that crash. The `currentPath` parameter was supplied as an empty string for the path change events, so those never crashed. They just had the `url` query string parameter set incorrectly (to an empty string). It should now be correctly populated, which should mean we'll be capturing all path changes now. Previously we were only capturing path changes to pages that happened to include an event, because of this blank `url` problem. * Use `url` query parameter as fallback for generating `pv_id` The `pv_id` parameter currently isn't generated correctly on Firefox, as the generation assumes that the current URL starts with `chrome-extension://`. The `url` query parameter is still unique for each path, so it's probably good enough for generating an id for each page. This is just a temporary fix; it will be removed in a future PR, where Firefox will be properly supported. --- app/scripts/lib/backend-metametrics.js | 2 +- ui/app/helpers/utils/metametrics.util.js | 8 +++----- ui/app/pages/routes/routes.component.js | 6 +++--- 3 files changed, 7 insertions(+), 9 deletions(-) diff --git a/app/scripts/lib/backend-metametrics.js b/app/scripts/lib/backend-metametrics.js index a035b9d91..9328fa3af 100644 --- a/app/scripts/lib/backend-metametrics.js +++ b/app/scripts/lib/backend-metametrics.js @@ -14,7 +14,7 @@ export default function backEndMetaMetricsEvent (metaMaskState, eventData) { sendMetaMetricsEvent({ ...stateEventData, ...eventData, - url: METAMETRICS_TRACKING_URL + '/background', + currentPath: METAMETRICS_TRACKING_URL + '/background', }) } } diff --git a/ui/app/helpers/utils/metametrics.util.js b/ui/app/helpers/utils/metametrics.util.js index 98b83f3d8..3cba4f215 100644 --- a/ui/app/helpers/utils/metametrics.util.js +++ b/ui/app/helpers/utils/metametrics.util.js @@ -13,7 +13,7 @@ const METAMETRICS_BASE_URL = 'https://chromeextensionmm.innocraft.cloud/piwik.ph const METAMETRICS_REQUIRED_PARAMS = `?idsite=${projectId}&rec=1&apiv=1` const METAMETRICS_BASE_FULL = METAMETRICS_BASE_URL + METAMETRICS_REQUIRED_PARAMS -const METAMETRICS_TRACKING_URL = inDevelopment +export const METAMETRICS_TRACKING_URL = inDevelopment ? 'http://www.metamask.io/metametrics' : 'http://www.metamask.io/metametrics-prod' @@ -119,7 +119,6 @@ function composeParamAddition (paramValue, paramName) { * @property {string} config.currentPath The location path the user is on at the time of the event * @property {string} config.metaMetricsId A random id assigned to a user at the time of opting in to metametrics. A hexadecimal number * @property {string} config.confirmTransactionOrigin The origin on a transaction - * @property {string} config.url The url to track an event at. Overrides `currentPath` * @property {boolean} config.excludeMetaMetricsId Whether or not the tracked event data should be associated with a metametrics id * @property {boolean} config.isNewVisit Whether or not the event should be tracked as a new visit/user sessions * @returns {string} - Returns a url to be passed to fetch to make the appropriate request to matomo. @@ -141,7 +140,6 @@ function composeUrl (config) { currentPath, metaMetricsId, confirmTransactionOrigin, - url: configUrl, excludeMetaMetricsId, isNewVisit, } = config @@ -167,10 +165,10 @@ function composeUrl (config) { numberOfTokens: (customVariables && customVariables.numberOfTokens) || numberOfTokens, numberOfAccounts: (customVariables && customVariables.numberOfAccounts) || numberOfAccounts, }) : '' - const url = configUrl || currentPath ? `&url=${encodeURIComponent(currentPath.replace(/chrome-extension:\/\/\w+/, METAMETRICS_TRACKING_URL))}` : '' + const url = currentPath ? `&url=${encodeURIComponent(currentPath.replace(/chrome-extension:\/\/\w+/, METAMETRICS_TRACKING_URL))}` : '' const _id = metaMetricsId && !excludeMetaMetricsId ? `&_id=${metaMetricsId.slice(2, 18)}` : '' const rand = `&rand=${String(Math.random()).slice(2)}` - const pv_id = ((url || currentPath) && `&pv_id=${ethUtil.bufferToHex(ethUtil.sha3(url || currentPath.match(/chrome-extension:\/\/\w+\/(.+)/)[0])).slice(2, 8)}`) || '' + const pv_id = currentPath ? `&pv_id=${ethUtil.bufferToHex(ethUtil.sha3(currentPath.match(/chrome-extension:\/\/\w+\/(.+)/)?.[0] || url)).slice(2, 8)}` : '' const uid = metaMetricsId && !excludeMetaMetricsId ? `&uid=${metaMetricsId.slice(2, 18)}` : excludeMetaMetricsId diff --git a/ui/app/pages/routes/routes.component.js b/ui/app/pages/routes/routes.component.js index 1dc556a98..1aca07f07 100644 --- a/ui/app/pages/routes/routes.component.js +++ b/ui/app/pages/routes/routes.component.js @@ -52,6 +52,8 @@ import { UNLOCK_ROUTE, } from '../../helpers/constants/routes' +import { METAMETRICS_TRACKING_URL } from '../../helpers/utils/metametrics.util' + import { ENVIRONMENT_TYPE_NOTIFICATION, ENVIRONMENT_TYPE_POPUP } from '../../../../app/scripts/lib/enums' import { getEnvironmentType } from '../../../../app/scripts/lib/util' @@ -98,11 +100,9 @@ export default class Routes extends Component { this.props.history.listen((locationObj, action) => { if (action === 'PUSH') { pageChanged(locationObj.pathname) - const url = `&url=${encodeURIComponent('http://www.metamask.io/metametrics' + locationObj.pathname)}` this.context.metricsEvent({}, { - currentPath: '', + currentPath: `${METAMETRICS_TRACKING_URL}${locationObj.pathname}`, pathname: locationObj.pathname, - url, pageOpts: { hideDimensions: true, },