From 2a2f5cca503dd2fdb3678ab1a2c505b23bb0d9b8 Mon Sep 17 00:00:00 2001 From: Simon Backx Date: Mon, 7 Nov 2022 16:55:17 +0100 Subject: [PATCH] =?UTF-8?q?=F0=9F=90=9B=20Fixed=20source=20tracking=20usin?= =?UTF-8?q?g=20cached=20value=20(#15778)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit refs https://ghost.slack.com/archives/C02G9E68C/p1667834794676479 - When enabling tracking, it could be the case that the server is ignoring the attributions because of the cached setting value. - When disabling tracking, the frontend should take care of not collecting new tracking information to the server, but still the backend value should be used as a fail-safe. --- .../server/services/member-attribution/index.js | 2 +- ghost/member-attribution/lib/service.js | 10 +++++++--- ghost/member-attribution/test/service.test.js | 16 ++++++++-------- 3 files changed, 16 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 12f4888865..84c17e7fec 100644 --- a/ghost/core/core/server/services/member-attribution/index.js +++ b/ghost/core/core/server/services/member-attribution/index.js @@ -40,7 +40,7 @@ class MemberAttributionServiceWrapper { Integration: models.Integration }, attributionBuilder: this.attributionBuilder, - isTrackingEnabled: !!settingsCache.get('members_track_sources') + getTrackingEnabled: () => !!settingsCache.get('members_track_sources') }); } } diff --git a/ghost/member-attribution/lib/service.js b/ghost/member-attribution/lib/service.js index e2233a707a..5306e84e16 100644 --- a/ghost/member-attribution/lib/service.js +++ b/ghost/member-attribution/lib/service.js @@ -5,15 +5,19 @@ 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 + * @param {() => boolean} deps.getTrackingEnabled */ - constructor({attributionBuilder, models, isTrackingEnabled}) { + constructor({attributionBuilder, models, getTrackingEnabled}) { this.models = models; this.attributionBuilder = attributionBuilder; - this.isTrackingEnabled = isTrackingEnabled; + this._getTrackingEnabled = getTrackingEnabled; + } + + get isTrackingEnabled() { + return this._getTrackingEnabled(); } /** diff --git a/ghost/member-attribution/test/service.test.js b/ghost/member-attribution/test/service.test.js index 101b4b02ce..34ac0d8a83 100644 --- a/ghost/member-attribution/test/service.test.js +++ b/ghost/member-attribution/test/service.test.js @@ -13,7 +13,7 @@ describe('MemberAttributionService', function () { describe('getAttributionFromContext', function () { it('returns null if no context is provided', async function () { const service = new MemberAttributionService({ - isTrackingEnabled: true + getTrackingEnabled: () => true }); const attribution = await service.getAttributionFromContext(); @@ -31,7 +31,7 @@ describe('MemberAttributionService', function () { it('returns attribution for importer context', async function () { const service = new MemberAttributionService({ - isTrackingEnabled: true + getTrackingEnabled: () => true }); const attribution = await service.getAttributionFromContext({importer: true}); @@ -40,7 +40,7 @@ describe('MemberAttributionService', function () { it('returns attribution for admin context', async function () { const service = new MemberAttributionService({ - isTrackingEnabled: true + getTrackingEnabled: () => true }); const attribution = await service.getAttributionFromContext({user: 'abc'}); @@ -49,7 +49,7 @@ describe('MemberAttributionService', function () { it('returns attribution for api without integration context', async function () { const service = new MemberAttributionService({ - isTrackingEnabled: true + getTrackingEnabled: () => true }); const attribution = await service.getAttributionFromContext({ api_key: 'abc' @@ -69,7 +69,7 @@ describe('MemberAttributionService', function () { } } }, - isTrackingEnabled: true + getTrackingEnabled: () => true }); const attribution = await service.getAttributionFromContext({ api_key: 'abc', @@ -96,7 +96,7 @@ describe('MemberAttributionService', function () { }; } }, - isTrackingEnabled: true + getTrackingEnabled: () => true }); const model = { id: 'event_id', @@ -130,7 +130,7 @@ describe('MemberAttributionService', function () { }; } }, - isTrackingEnabled: true + getTrackingEnabled: () => true }); const model = { id: 'event_id', @@ -170,7 +170,7 @@ describe('MemberAttributionService', function () { }; } }, - isTrackingEnabled: true + getTrackingEnabled: () => true }); const model = { id: 'event_id',