From 5a8496161347f691d2d88e88269fb22e27bb7619 Mon Sep 17 00:00:00 2001 From: Kelson Warner Date: Wed, 16 Sep 2020 15:57:03 -0700 Subject: [PATCH 1/7] Refactor _trackParamsAndReferrer to introduce a method that can access utms without sending them --- src/amplitude-client.js | 59 +++++++++++++++++++++++++++++------ test/browser/amplitudejs.html | 14 ++++++++- 2 files changed, 63 insertions(+), 10 deletions(-) diff --git a/src/amplitude-client.js b/src/amplitude-client.js index 78722ddd..b1d69863 100644 --- a/src/amplitude-client.js +++ b/src/amplitude-client.js @@ -365,6 +365,23 @@ AmplitudeClient.prototype._trackParamsAndReferrer = function _trackParamsAndRefe } }; +/** + * Collect all utm, referral, and gclid data and combine into a single object + * @private + */ + +AmplitudeClient.prototype._collectParamsAndReferrer = function _collectParamsAndReferrer() { + const utmProperties = this._getUtmProperties(); + const referrerProperties = this._getReferrerProperties(this._getReferrer()); + const gclidProperties = this._getGclidProperties(this._getUrlParams()); + + return { + ...utmProperties, + ...referrerProperties, + ...gclidProperties, + }; +}; + /** * Parse and validate user specified config values and overwrite existing option value * DEFAULT_OPTIONS provides list of all config keys that are modifiable, as well as expected types for values @@ -667,13 +684,21 @@ var _saveCookieData = function _saveCookieData(scope) { }; /** - * Parse the utm properties out of cookies and query for adding to user properties. + * Parse the utm properties out of cookies and query params * @private */ -AmplitudeClient.prototype._initUtmData = function _initUtmData(queryParams, cookieParams) { +AmplitudeClient.prototype._getUtmProperties = function _getUtmProperties(queryParams, cookieParams) { queryParams = queryParams || this._getUrlParams(); cookieParams = cookieParams || this.cookieStorage.get('__utmz'); - var utmProperties = getUtmData(cookieParams, queryParams); + return getUtmData(cookieParams, queryParams); +}; + +/** + * Gets parsed utm properties and adds to user properties + * @private + */ +AmplitudeClient.prototype._initUtmData = function _initUtmData(queryParams, cookieParams) { + var utmProperties = this._getUtmProperties(queryParams, cookieParams); _sendParamsReferrerUserProperties(this, utmProperties); }; @@ -732,12 +757,20 @@ AmplitudeClient.prototype._getUrlParams = function _getUrlParams() { * Try to fetch Google Gclid from url params. * @private */ -AmplitudeClient.prototype._saveGclid = function _saveGclid(urlParams) { +AmplitudeClient.prototype._getGclidProperties = function _getGclidProperties(urlParams) { var gclid = utils.getQueryParam('gclid', urlParams); if (utils.isEmptyString(gclid)) { return; } - var gclidProperties = {'gclid': gclid}; + return {'gclid': gclid}; +}; + +/** + * If Google Gclid exists, add as user properties + * @private + */ +AmplitudeClient.prototype._saveGclid = function _saveGclid(urlParams) { + var gclidProperties = this._getGclidProperties(urlParams); _sendParamsReferrerUserProperties(this, gclidProperties); }; @@ -765,18 +798,26 @@ AmplitudeClient.prototype._getReferringDomain = function _getReferringDomain(ref }; /** - * Fetch the referrer information, parse the domain and send. - * Since user properties are propagated on the server, only send once per session, don't need to send with every event + * Fetch the referrer information and the domain. * @private */ -AmplitudeClient.prototype._saveReferrer = function _saveReferrer(referrer) { +AmplitudeClient.prototype._getReferrerProperties = function _getReferrerProperties(referrer) { if (utils.isEmptyString(referrer)) { return; } - var referrerInfo = { + return { 'referrer': referrer, 'referring_domain': this._getReferringDomain(referrer) }; +}; + +/** + * Check if referrer info exists and send. + * Since user properties are propagated on the server, only send once per session, don't need to send with every event + * @private + */ +AmplitudeClient.prototype._saveReferrer = function _saveReferrer(referrer) { + var referrerInfo = this._getReferrerProperties(referrer); _sendParamsReferrerUserProperties(this, referrerInfo); }; diff --git a/test/browser/amplitudejs.html b/test/browser/amplitudejs.html index 4c60be84..592a2e7f 100644 --- a/test/browser/amplitudejs.html +++ b/test/browser/amplitudejs.html @@ -27,7 +27,7 @@ 'setOptOut', 'setVersionName', 'setDomain', 'setDeviceId', 'setGlobalUserProperties', 'identify', 'clearUserProperties', 'setGroup', 'logRevenueV2', 'regenerateDeviceId', - 'logEventWithTimestamp', 'logEventWithGroups']; + 'logEventWithTimestamp', 'logEventWithGroups', 'onInit']; function setUpProxy(instance) { function proxyMain(fn) { instance[fn] = function() { @@ -89,6 +89,14 @@ }

Amplitude JS Test

From 8d0604fd323ad96069a2e0f905fa36e03632bee2 Mon Sep 17 00:00:00 2001 From: Kelson Warner Date: Wed, 16 Sep 2020 16:37:05 -0700 Subject: [PATCH 2/7] Follow up on naming and PR suggestions --- src/amplitude-client.js | 14 +++++++++----- test/browser/amplitudejs.html | 12 ------------ 2 files changed, 9 insertions(+), 17 deletions(-) diff --git a/src/amplitude-client.js b/src/amplitude-client.js index b1d69863..16af7a5c 100644 --- a/src/amplitude-client.js +++ b/src/amplitude-client.js @@ -370,11 +370,15 @@ AmplitudeClient.prototype._trackParamsAndReferrer = function _trackParamsAndRefe * @private */ -AmplitudeClient.prototype._collectParamsAndReferrer = function _collectParamsAndReferrer() { - const utmProperties = this._getUtmProperties(); - const referrerProperties = this._getReferrerProperties(this._getReferrer()); - const gclidProperties = this._getGclidProperties(this._getUrlParams()); - +AmplitudeClient.prototype._getParamsAndReferrer = function _getParamsAndReferrer() { + const utmProperties = this.options.includeUtm ? this._getUtmProperties() : {}; + const referrerProperties = this.options.includeReferrer + ? this._getReferrerProperties(this._getReferrer()) + : {}; + const gclidProperties = this.options.includeGlid + ? this._getGclidProperties(this._getUrlParams()) + : {}; + return { ...utmProperties, ...referrerProperties, diff --git a/test/browser/amplitudejs.html b/test/browser/amplitudejs.html index 592a2e7f..94a7170e 100644 --- a/test/browser/amplitudejs.html +++ b/test/browser/amplitudejs.html @@ -89,14 +89,6 @@ }

Amplitude JS Test

From 34d785785d77780624015c42acaa0a24b3714db6 Mon Sep 17 00:00:00 2001 From: Kelson Warner Date: Wed, 16 Sep 2020 16:47:47 -0700 Subject: [PATCH 3/7] Update trackParamsAndReferrer to use getParamsAndReferrer --- src/amplitude-client.js | 35 +++++++++++++++++------------------ test/amplitude-client.js | 3 ++- 2 files changed, 19 insertions(+), 19 deletions(-) diff --git a/src/amplitude-client.js b/src/amplitude-client.js index 16af7a5c..1f1a9b16 100644 --- a/src/amplitude-client.js +++ b/src/amplitude-client.js @@ -354,14 +354,16 @@ AmplitudeClient.prototype._migrateUnsentEvents = function _migrateUnsentEvents(c * @private */ AmplitudeClient.prototype._trackParamsAndReferrer = function _trackParamsAndReferrer() { + const { utmProperties, referrerProperties, gclidProperties } = this._getParamsAndReferrer(); + if (this.options.includeUtm) { - this._initUtmData(); + this._initUtmData(utmProperties); } if (this.options.includeReferrer) { - this._saveReferrer(this._getReferrer()); + this._saveReferrer(referrerProperties); } if (this.options.includeGclid) { - this._saveGclid(this._getUrlParams()); + this._saveGclid(gclidProperties); } }; @@ -371,18 +373,18 @@ AmplitudeClient.prototype._trackParamsAndReferrer = function _trackParamsAndRefe */ AmplitudeClient.prototype._getParamsAndReferrer = function _getParamsAndReferrer() { - const utmProperties = this.options.includeUtm ? this._getUtmProperties() : {}; + const utmProperties = this.options.includeUtm ? this._getUtmProperties() : null; const referrerProperties = this.options.includeReferrer ? this._getReferrerProperties(this._getReferrer()) - : {}; + : null; const gclidProperties = this.options.includeGlid ? this._getGclidProperties(this._getUrlParams()) - : {}; + : null; return { - ...utmProperties, - ...referrerProperties, - ...gclidProperties, + utmProperties, + referrerProperties, + gclidProperties, }; }; @@ -701,8 +703,7 @@ AmplitudeClient.prototype._getUtmProperties = function _getUtmProperties(queryPa * Gets parsed utm properties and adds to user properties * @private */ -AmplitudeClient.prototype._initUtmData = function _initUtmData(queryParams, cookieParams) { - var utmProperties = this._getUtmProperties(queryParams, cookieParams); +AmplitudeClient.prototype._initUtmData = function _initUtmData(utmProperties) { _sendParamsReferrerUserProperties(this, utmProperties); }; @@ -770,11 +771,10 @@ AmplitudeClient.prototype._getGclidProperties = function _getGclidProperties(url }; /** - * If Google Gclid exists, add as user properties + * Add Google Gclid as user properties * @private */ -AmplitudeClient.prototype._saveGclid = function _saveGclid(urlParams) { - var gclidProperties = this._getGclidProperties(urlParams); +AmplitudeClient.prototype._saveGclid = function _saveGclid(gclidProperties) { _sendParamsReferrerUserProperties(this, gclidProperties); }; @@ -816,13 +816,12 @@ AmplitudeClient.prototype._getReferrerProperties = function _getReferrerProperti }; /** - * Check if referrer info exists and send. + * Sends referrer properties * Since user properties are propagated on the server, only send once per session, don't need to send with every event * @private */ -AmplitudeClient.prototype._saveReferrer = function _saveReferrer(referrer) { - var referrerInfo = this._getReferrerProperties(referrer); - _sendParamsReferrerUserProperties(this, referrerInfo); +AmplitudeClient.prototype._saveReferrer = function _saveReferrer(referrerProperties) { + _sendParamsReferrerUserProperties(this, referrerProperties); }; /** diff --git a/test/amplitude-client.js b/test/amplitude-client.js index 53813cb9..d82b3c92 100644 --- a/test/amplitude-client.js +++ b/test/amplitude-client.js @@ -2627,7 +2627,8 @@ describe('setVersionName', function() { cookie.set('__utmz', '133232535.1424926227.1.1.utmcct=top&utmccn=new'); var utmParams = '?utm_source=amplitude&utm_medium=email&utm_term=terms'; clock.tick(30 * 60 * 1000 + 1); - amplitude._initUtmData(utmParams); + var utmProperties = amplitude._getUtmProperties(utmParams); + amplitude._initUtmData(utmProperties); var expectedProperties = { utm_campaign: 'new', From 80ab16fc42b9669669964d3e6c5fab317cab4cfa Mon Sep 17 00:00:00 2001 From: Kelson Warner Date: Wed, 16 Sep 2020 16:51:37 -0700 Subject: [PATCH 4/7] Update comments --- src/amplitude-client.js | 33 +++++++++++++++++++-------------- 1 file changed, 19 insertions(+), 14 deletions(-) diff --git a/src/amplitude-client.js b/src/amplitude-client.js index 1f1a9b16..555da525 100644 --- a/src/amplitude-client.js +++ b/src/amplitude-client.js @@ -368,18 +368,23 @@ AmplitudeClient.prototype._trackParamsAndReferrer = function _trackParamsAndRefe }; /** - * Collect all utm, referral, and gclid data and combine into a single object + * Collect all utm, referral, and gclid data * @private */ - AmplitudeClient.prototype._getParamsAndReferrer = function _getParamsAndReferrer() { - const utmProperties = this.options.includeUtm ? this._getUtmProperties() : null; - const referrerProperties = this.options.includeReferrer - ? this._getReferrerProperties(this._getReferrer()) - : null; - const gclidProperties = this.options.includeGlid - ? this._getGclidProperties(this._getUrlParams()) - : null; + let utmProperties = null; + let referrerProperties = null; + let gclidProperties = null; + + if (this.options.includeUtm) { + utmProperties = this._getUtmProperties() + } + if (this.options.includeReferrer) { + referrerProperties = this._getReferrerProperties(this._getReferrer()) + } + if (this.options.includeGclid) { + gclidProperties = this._getGclidProperties(this._getUrlParams()) + } return { utmProperties, @@ -700,7 +705,7 @@ AmplitudeClient.prototype._getUtmProperties = function _getUtmProperties(queryPa }; /** - * Gets parsed utm properties and adds to user properties + * Accepts parsed utm properties and adds to user properties * @private */ AmplitudeClient.prototype._initUtmData = function _initUtmData(utmProperties) { @@ -759,7 +764,7 @@ AmplitudeClient.prototype._getUrlParams = function _getUrlParams() { }; /** - * Try to fetch Google Gclid from url params. + * Attempts to fetch Google Gclid from url params. * @private */ AmplitudeClient.prototype._getGclidProperties = function _getGclidProperties(urlParams) { @@ -771,7 +776,7 @@ AmplitudeClient.prototype._getGclidProperties = function _getGclidProperties(url }; /** - * Add Google Gclid as user properties + * Adds Google Gclid as user properties * @private */ AmplitudeClient.prototype._saveGclid = function _saveGclid(gclidProperties) { @@ -802,7 +807,7 @@ AmplitudeClient.prototype._getReferringDomain = function _getReferringDomain(ref }; /** - * Fetch the referrer information and the domain. + * Gets the referrer information and the domain. * @private */ AmplitudeClient.prototype._getReferrerProperties = function _getReferrerProperties(referrer) { @@ -816,7 +821,7 @@ AmplitudeClient.prototype._getReferrerProperties = function _getReferrerProperti }; /** - * Sends referrer properties + * Accepts parsed referrer properties and adds to user properties * Since user properties are propagated on the server, only send once per session, don't need to send with every event * @private */ From 742b5f977e33c11d5272d87004ae874f788ed35f Mon Sep 17 00:00:00 2001 From: Kelson Warner Date: Wed, 16 Sep 2020 16:58:03 -0700 Subject: [PATCH 5/7] Re-introduce semi colons --- src/amplitude-client.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/amplitude-client.js b/src/amplitude-client.js index 555da525..dd38ec3b 100644 --- a/src/amplitude-client.js +++ b/src/amplitude-client.js @@ -377,13 +377,13 @@ AmplitudeClient.prototype._getParamsAndReferrer = function _getParamsAndReferrer let gclidProperties = null; if (this.options.includeUtm) { - utmProperties = this._getUtmProperties() + utmProperties = this._getUtmProperties(); } if (this.options.includeReferrer) { - referrerProperties = this._getReferrerProperties(this._getReferrer()) + referrerProperties = this._getReferrerProperties(this._getReferrer()); } if (this.options.includeGclid) { - gclidProperties = this._getGclidProperties(this._getUrlParams()) + gclidProperties = this._getGclidProperties(this._getUrlParams()); } return { From 186ccdca7ff53fa6e31a97213751ff474edcccc8 Mon Sep 17 00:00:00 2001 From: Kelson Warner Date: Wed, 16 Sep 2020 17:01:41 -0700 Subject: [PATCH 6/7] Re-introduce semi colons --- src/amplitude-client.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/amplitude-client.js b/src/amplitude-client.js index dd38ec3b..abe69e46 100644 --- a/src/amplitude-client.js +++ b/src/amplitude-client.js @@ -368,7 +368,7 @@ AmplitudeClient.prototype._trackParamsAndReferrer = function _trackParamsAndRefe }; /** - * Collect all utm, referral, and gclid data + * Fetch all utm, referral, and gclid data * @private */ AmplitudeClient.prototype._getParamsAndReferrer = function _getParamsAndReferrer() { From e26c5e23809230bac00be4930c50978e062d1536 Mon Sep 17 00:00:00 2001 From: Kelson Warner Date: Wed, 16 Sep 2020 17:37:42 -0700 Subject: [PATCH 7/7] Get tests passing again after refactor --- test/amplitude-client.js | 2 +- test/amplitude.js | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/test/amplitude-client.js b/test/amplitude-client.js index d82b3c92..1094da45 100644 --- a/test/amplitude-client.js +++ b/test/amplitude-client.js @@ -2626,8 +2626,8 @@ describe('setVersionName', function() { reset(); cookie.set('__utmz', '133232535.1424926227.1.1.utmcct=top&utmccn=new'); var utmParams = '?utm_source=amplitude&utm_medium=email&utm_term=terms'; - clock.tick(30 * 60 * 1000 + 1); var utmProperties = amplitude._getUtmProperties(utmParams); + clock.tick(30 * 60 * 1000 + 1); amplitude._initUtmData(utmProperties); var expectedProperties = { diff --git a/test/amplitude.js b/test/amplitude.js index ef353388..50a6cf25 100644 --- a/test/amplitude.js +++ b/test/amplitude.js @@ -1954,8 +1954,9 @@ describe('setVersionName', function() { reset(); cookie.set('__utmz', '133232535.1424926227.1.1.utmcct=top&utmccn=new'); var utmParams = '?utm_source=amplitude&utm_medium=email&utm_term=terms'; + var utmProperties = amplitude.getInstance()._getUtmProperties(utmParams); clock.tick(30 * 60 * 1000 + 1); - amplitude.getInstance()._initUtmData(utmParams); + amplitude.getInstance()._initUtmData(utmProperties); var expectedProperties = { utm_campaign: 'new',