diff --git a/services/web/app/src/Features/Analytics/AnalyticsRegistrationSourceHelper.js b/services/web/app/src/Features/Analytics/AnalyticsRegistrationSourceHelper.js index ed79e1f17d..bbe9058e5c 100644 --- a/services/web/app/src/Features/Analytics/AnalyticsRegistrationSourceHelper.js +++ b/services/web/app/src/Features/Analytics/AnalyticsRegistrationSourceHelper.js @@ -1,5 +1,6 @@ +const RefererParser = require('referer-parser') +const { URL } = require('url') const AnalyticsManager = require('./AnalyticsManager') -const RequestHelper = require('./RequestHelper') function clearSource(session) { if (session) { @@ -8,10 +9,57 @@ function clearSource(session) { } } +const UTM_KEYS = [ + 'utm_campaign', + 'utm_source', + 'utm_term', + 'utm_medium', + 'utm_count', +] + +function parseUtm(query) { + const utmValues = {} + for (const utmKey of UTM_KEYS) { + if (query[utmKey]) { + utmValues[utmKey] = query[utmKey] + } + } + return Object.keys(utmValues).length > 0 ? utmValues : null +} + +function parseReferrer(referrer, url) { + if (!referrer) { + return { + medium: 'direct', + } + } + + const parsedReferrer = new RefererParser(referrer, url) + + const referrerValues = { + medium: parsedReferrer.medium, + source: parsedReferrer.referer || 'other', + } + + if (referrerValues.medium === 'unknown') { + try { + const referrerHostname = new URL(referrer).hostname + if (referrerHostname) { + referrerValues.medium = 'link' + referrerValues.source = referrerHostname + } + } catch (error) { + // ignore referrer parsing errors + } + } + + return referrerValues +} + function setInbound(session, url, query, referrer) { const inboundSession = { - referrer: RequestHelper.parseReferrer(referrer, url), - utm: RequestHelper.parseUtm(query), + referrer: parseReferrer(referrer, url), + utm: parseUtm(query), } if (inboundSession.referrer || inboundSession.utm) { @@ -75,7 +123,7 @@ function addUserProperties(userId, session) { } if (session.inbound.utm) { - for (const utmKey of RequestHelper.UTM_KEYS) { + for (const utmKey of UTM_KEYS) { if (session.inbound.utm[utmKey]) { AnalyticsManager.setUserPropertyForUser( userId, diff --git a/services/web/app/src/Features/Analytics/AnalyticsRegistrationSourceMiddleware.js b/services/web/app/src/Features/Analytics/AnalyticsRegistrationSourceMiddleware.js index 33116579ea..b1c0571355 100644 --- a/services/web/app/src/Features/Analytics/AnalyticsRegistrationSourceMiddleware.js +++ b/services/web/app/src/Features/Analytics/AnalyticsRegistrationSourceMiddleware.js @@ -29,7 +29,7 @@ function setInbound() { } if (SessionManager.isUserLoggedIn(req.session)) { - return next() // don't store referrer if user is already logged in + return next() // don't store referrer if user is alread logged in } const referrer = req.header('referrer') diff --git a/services/web/app/src/Features/Analytics/AnalyticsUTMTrackingMiddleware.js b/services/web/app/src/Features/Analytics/AnalyticsUTMTrackingMiddleware.js deleted file mode 100644 index e746287fe6..0000000000 --- a/services/web/app/src/Features/Analytics/AnalyticsUTMTrackingMiddleware.js +++ /dev/null @@ -1,60 +0,0 @@ -const _ = require('lodash') -const { URL } = require('url') -const RequestHelper = require('./RequestHelper') -const AnalyticsManager = require('./AnalyticsManager') -const querystring = require('querystring') -const OError = require('@overleaf/o-error') -const logger = require('logger-sharelatex') - -function recordUTMTags() { - return function (req, res, next) { - const query = req.query - - try { - const utmValues = RequestHelper.parseUtm(query) - - if (utmValues) { - const pathname = new URL(req.url).pathname - - AnalyticsManager.recordEventForSession(req.session, 'page-view', { - path: pathname, - ...utmValues, - }) - - const propertyValue = [ - 'utm_source', - 'utm_medium', - 'utm_campaign', - 'utm_term', - ] - .map(tag => utmValues[tag] || 'N/A') - .join(';') - AnalyticsManager.setUserPropertyForSession( - req.session, - 'utm-tags', - propertyValue - ) - - // redirect to URL without UTM query params - const queryWithoutUtm = _.omit(query, RequestHelper.UTM_KEYS) - const queryString = - Object.keys(queryWithoutUtm).length > 0 - ? '?' + querystring.stringify(queryWithoutUtm) - : '' - res.redirect(pathname + queryString) - } - } catch (error) { - // log errors and fail silently - OError.tag(error, 'failed to track UTM tags', { - query, - }) - logger.warn({ error }, error.message) - } - - next() - } -} - -module.exports = { - recordUTMTags, -} diff --git a/services/web/app/src/Features/Analytics/RequestHelper.js b/services/web/app/src/Features/Analytics/RequestHelper.js deleted file mode 100644 index cf07bf55a7..0000000000 --- a/services/web/app/src/Features/Analytics/RequestHelper.js +++ /dev/null @@ -1,55 +0,0 @@ -const RefererParser = require('referer-parser') -const { URL } = require('url') - -const UTM_KEYS = [ - 'utm_campaign', - 'utm_source', - 'utm_term', - 'utm_medium', - 'utm_count', -] - -function parseUtm(query) { - const utmValues = {} - for (const utmKey of UTM_KEYS) { - if (query[utmKey]) { - utmValues[utmKey] = query[utmKey] - } - } - return Object.keys(utmValues).length > 0 ? utmValues : null -} - -function parseReferrer(referrer, url) { - if (!referrer) { - return { - medium: 'direct', - } - } - - const parsedReferrer = new RefererParser(referrer, url) - - const referrerValues = { - medium: parsedReferrer.medium, - source: parsedReferrer.referer || 'other', - } - - if (referrerValues.medium === 'unknown') { - try { - const referrerHostname = new URL(referrer).hostname - if (referrerHostname) { - referrerValues.medium = 'link' - referrerValues.source = referrerHostname - } - } catch (error) { - // ignore referrer parsing errors - } - } - - return referrerValues -} - -module.exports = { - UTM_KEYS, - parseUtm, - parseReferrer, -} diff --git a/services/web/app/src/router.js b/services/web/app/src/router.js index 99b9d29400..34144a55ed 100644 --- a/services/web/app/src/router.js +++ b/services/web/app/src/router.js @@ -50,7 +50,6 @@ const InstitutionsController = require('./Features/Institutions/InstitutionsCont const UserMembershipRouter = require('./Features/UserMembership/UserMembershipRouter') const SystemMessageController = require('./Features/SystemMessages/SystemMessageController') const AnalyticsRegistrationSourceMiddleware = require('./Features/Analytics/AnalyticsRegistrationSourceMiddleware') -const AnalyticsUTMTrackingMiddleware = require('./Features/Analytics/AnalyticsUTMTrackingMiddleware') const { Joi, validate } = require('./infrastructure/Validation') const { renderUnsupportedBrowserPage, @@ -70,7 +69,6 @@ function initialize(webRouter, privateApiRouter, publicApiRouter) { } webRouter.get('*', AnalyticsRegistrationSourceMiddleware.setInbound()) - webRouter.get('*', AnalyticsUTMTrackingMiddleware.recordUTMTags()) webRouter.get('/login', UserPagesController.loginPage) AuthenticationController.addEndpointToLoginWhitelist('/login') diff --git a/services/web/test/unit/src/Analytics/AnalyticsUTMTrackingMiddlewareTests.js b/services/web/test/unit/src/Analytics/AnalyticsUTMTrackingMiddlewareTests.js deleted file mode 100644 index b090ced218..0000000000 --- a/services/web/test/unit/src/Analytics/AnalyticsUTMTrackingMiddlewareTests.js +++ /dev/null @@ -1,189 +0,0 @@ -const SandboxedModule = require('sandboxed-module') -const path = require('path') -const sinon = require('sinon') -const MockRequest = require('../helpers/MockRequest') -const MockResponse = require('../helpers/MockResponse') -const { assert } = require('chai') - -const MODULE_PATH = path.join( - __dirname, - '../../../../app/src/Features/Analytics/AnalyticsUTMTrackingMiddleware' -) - -describe('AnalyticsUTMTrackingMiddleware', function () { - beforeEach(function () { - this.analyticsId = 'ecdb935a-52f3-4f91-aebc-7a70d2ffbb55' - this.userId = '61795fcb013504bb7b663092' - - this.req = new MockRequest() - this.res = new MockResponse() - this.next = sinon.stub().returns() - this.req.session = { - user: { - _id: this.userId, - analyticsId: this.analyticsId, - }, - } - - this.AnalyticsUTMTrackingMiddleware = SandboxedModule.require(MODULE_PATH, { - requires: { - './AnalyticsManager': (this.AnalyticsManager = { - recordEventForSession: sinon.stub().resolves(), - setUserPropertyForSession: sinon.stub().resolves(), - }), - }, - }) - - this.middleware = this.AnalyticsUTMTrackingMiddleware.recordUTMTags() - }) - - describe('without UTM tags in query', function () { - beforeEach(function () { - this.middleware(this.req, this.res, this.next) - }) - - it('user is not redirected', function () { - assert.isFalse(this.res.redirected) - }) - - it('next middleware is executed', function () { - sinon.assert.calledOnce(this.next) - }) - - it('no event or user property is recorded', function () { - sinon.assert.notCalled(this.AnalyticsManager.recordEventForSession) - sinon.assert.notCalled(this.AnalyticsManager.setUserPropertyForSession) - }) - }) - - describe('with all UTM tags in query', function () { - beforeEach(function () { - this.req.url = 'https://www.overleaf.com/project' - this.req.query = { - utm_source: 'Organic', - utm_medium: 'Horizon', - utm_campaign: 'Some Campaign', - utm_term: 'foo-bar', - } - this.middleware(this.req, this.res, this.next) - }) - - it('user is redirected', function () { - assert.isTrue(this.res.redirected) - assert.equal('/project', this.res.redirectedTo) - }) - - it('next middleware is executed', function () { - sinon.assert.calledOnce(this.next) - }) - - it('page-view event is recorded for session', function () { - sinon.assert.calledWith( - this.AnalyticsManager.recordEventForSession, - this.req.session, - 'page-view', - { - path: '/project', - utm_source: 'Organic', - utm_medium: 'Horizon', - utm_campaign: 'Some Campaign', - utm_term: 'foo-bar', - } - ) - }) - - it('utm-tags user property is set for session', function () { - sinon.assert.calledWith( - this.AnalyticsManager.setUserPropertyForSession, - this.req.session, - 'utm-tags', - 'Organic;Horizon;Some Campaign;foo-bar' - ) - }) - }) - - describe('with some UTM tags in query', function () { - beforeEach(function () { - this.req.url = 'https://www.overleaf.com/project' - this.req.query = { - utm_medium: 'Horizon', - utm_campaign: 'Some Campaign', - } - this.middleware(this.req, this.res, this.next) - }) - - it('user is redirected', function () { - assert.isTrue(this.res.redirected) - assert.equal('/project', this.res.redirectedTo) - }) - - it('next middleware is executed', function () { - sinon.assert.calledOnce(this.next) - }) - - it('page-view event is recorded for session', function () { - sinon.assert.calledWith( - this.AnalyticsManager.recordEventForSession, - this.req.session, - 'page-view', - { - path: '/project', - utm_medium: 'Horizon', - utm_campaign: 'Some Campaign', - } - ) - }) - - it('utm-tags user property is set for session', function () { - sinon.assert.calledWith( - this.AnalyticsManager.setUserPropertyForSession, - this.req.session, - 'utm-tags', - 'N/A;Horizon;Some Campaign;N/A' - ) - }) - }) - - describe('with some UTM tags and additional parameters in query', function () { - beforeEach(function () { - this.req.url = 'https://www.overleaf.com/project' - this.req.query = { - utm_medium: 'Horizon', - utm_campaign: 'Some Campaign', - other_param: 'some-value', - } - this.middleware(this.req, this.res, this.next) - }) - - it('user is redirected', function () { - assert.isTrue(this.res.redirected) - assert.equal('/project?other_param=some-value', this.res.redirectedTo) - }) - - it('next middleware is executed', function () { - sinon.assert.calledOnce(this.next) - }) - - it('page-view event is recorded for session', function () { - sinon.assert.calledWith( - this.AnalyticsManager.recordEventForSession, - this.req.session, - 'page-view', - { - path: '/project', - utm_medium: 'Horizon', - utm_campaign: 'Some Campaign', - } - ) - }) - - it('utm-tags user property is set for session', function () { - sinon.assert.calledWith( - this.AnalyticsManager.setUserPropertyForSession, - this.req.session, - 'utm-tags', - 'N/A;Horizon;Some Campaign;N/A' - ) - }) - }) -})