From f1aff45dc7cee01cd8d9115a66c9d904ef45607c Mon Sep 17 00:00:00 2001 From: Rishabh Garg Date: Thu, 27 Oct 2022 21:10:03 +0530 Subject: [PATCH] Disabled attribution calculation when tracking is disabled (#15710) refs https://github.com/TryGhost/Team/issues/2168 - forces attribution service to use empty history or context if attribution tracking is disabled --- .../services/member-attribution/index.js | 4 +- .../test/utils/fixtures/default-settings.json | 8 ++++ ghost/member-attribution/lib/service.js | 11 ++++-- ghost/member-attribution/test/service.test.js | 37 +++++++++++++++---- 4 files changed, 48 insertions(+), 12 deletions(-) diff --git a/ghost/core/core/server/services/member-attribution/index.js b/ghost/core/core/server/services/member-attribution/index.js index 98e2014827..12f4888865 100644 --- a/ghost/core/core/server/services/member-attribution/index.js +++ b/ghost/core/core/server/services/member-attribution/index.js @@ -1,5 +1,6 @@ const urlService = require('../url'); const urlUtils = require('../../../shared/url-utils'); +const settingsCache = require('../../../shared/settings-cache'); class MemberAttributionServiceWrapper { init() { @@ -38,7 +39,8 @@ class MemberAttributionServiceWrapper { SubscriptionCreatedEvent: models.SubscriptionCreatedEvent, Integration: models.Integration }, - attributionBuilder: this.attributionBuilder + attributionBuilder: this.attributionBuilder, + isTrackingEnabled: !!settingsCache.get('members_track_sources') }); } } diff --git a/ghost/core/test/utils/fixtures/default-settings.json b/ghost/core/test/utils/fixtures/default-settings.json index 39d9ca248c..613e9f9c9a 100644 --- a/ghost/core/test/utils/fixtures/default-settings.json +++ b/ghost/core/test/utils/fixtures/default-settings.json @@ -55,6 +55,14 @@ "members_stripe_webhook_secret": { "defaultValue": null, "type": "string" + }, + "members_track_sources": { + "defaultValue": "true", + "validations": { + "isEmpty": false, + "isIn": [["true", "false"]] + }, + "type": "boolean" } }, "site": { diff --git a/ghost/member-attribution/lib/service.js b/ghost/member-attribution/lib/service.js index 1c5795f81b..e2233a707a 100644 --- a/ghost/member-attribution/lib/service.js +++ b/ghost/member-attribution/lib/service.js @@ -5,13 +5,15 @@ class MemberAttributionService { * * @param {Object} deps * @param {Object} deps.attributionBuilder + * @param {boolean} deps.isTrackingEnabled * @param {Object} deps.models * @param {Object} deps.models.MemberCreatedEvent * @param {Object} deps.models.SubscriptionCreatedEvent */ - constructor({attributionBuilder, models}) { + constructor({attributionBuilder, models, isTrackingEnabled}) { this.models = models; this.attributionBuilder = attributionBuilder; + this.isTrackingEnabled = isTrackingEnabled; } /** @@ -20,7 +22,7 @@ class MemberAttributionService { * @returns {Promise} */ async getAttributionFromContext(context) { - if (!context) { + if (!context || !this.isTrackingEnabled) { return null; } @@ -68,7 +70,10 @@ class MemberAttributionService { * @returns {Promise} */ async getAttribution(historyArray) { - const history = UrlHistory.create(historyArray); + let history = UrlHistory.create(historyArray); + if (!this.isTrackingEnabled) { + history = UrlHistory.create([]); + } return await this.attributionBuilder.getAttribution(history); } diff --git a/ghost/member-attribution/test/service.test.js b/ghost/member-attribution/test/service.test.js index 25df2b9992..101b4b02ce 100644 --- a/ghost/member-attribution/test/service.test.js +++ b/ghost/member-attribution/test/service.test.js @@ -12,28 +12,45 @@ describe('MemberAttributionService', function () { describe('getAttributionFromContext', function () { it('returns null if no context is provided', async function () { - const service = new MemberAttributionService({}); + const service = new MemberAttributionService({ + isTrackingEnabled: true + }); + const attribution = await service.getAttributionFromContext(); + + should(attribution).be.null(); + }); + + it('returns null if tracking is disabled is provided', async function () { + const service = new MemberAttributionService({ + isTrackingEnabled: false + }); const attribution = await service.getAttributionFromContext(); should(attribution).be.null(); }); it('returns attribution for importer context', async function () { - const service = new MemberAttributionService({}); + const service = new MemberAttributionService({ + isTrackingEnabled: true + }); const attribution = await service.getAttributionFromContext({importer: true}); should(attribution).containEql({referrerSource: 'Imported', referrerMedium: 'Member Importer'}); }); it('returns attribution for admin context', async function () { - const service = new MemberAttributionService({}); + const service = new MemberAttributionService({ + isTrackingEnabled: true + }); const attribution = await service.getAttributionFromContext({user: 'abc'}); should(attribution).containEql({referrerSource: 'Created manually', referrerMedium: 'Ghost Admin'}); }); it('returns attribution for api without integration context', async function () { - const service = new MemberAttributionService({}); + const service = new MemberAttributionService({ + isTrackingEnabled: true + }); const attribution = await service.getAttributionFromContext({ api_key: 'abc' }); @@ -51,7 +68,8 @@ describe('MemberAttributionService', function () { }; } } - } + }, + isTrackingEnabled: true }); const attribution = await service.getAttributionFromContext({ api_key: 'abc', @@ -77,7 +95,8 @@ describe('MemberAttributionService', function () { } }; } - } + }, + isTrackingEnabled: true }); const model = { id: 'event_id', @@ -110,7 +129,8 @@ describe('MemberAttributionService', function () { } }; } - } + }, + isTrackingEnabled: true }); const model = { id: 'event_id', @@ -149,7 +169,8 @@ describe('MemberAttributionService', function () { } }; } - } + }, + isTrackingEnabled: true }); const model = { id: 'event_id',