From 4bd3ec58ceb5fc41fb8f841ff4679a8f5431a8ad Mon Sep 17 00:00:00 2001 From: alex-magana Date: Thu, 6 Feb 2025 17:15:31 +0300 Subject: [PATCH 01/11] Remove isLive check to enable Reverb on LIVE --- src/app/lib/analyticsUtils/sendBeacon/index.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/app/lib/analyticsUtils/sendBeacon/index.js b/src/app/lib/analyticsUtils/sendBeacon/index.js index a8078dfbfd2..466423bf40f 100644 --- a/src/app/lib/analyticsUtils/sendBeacon/index.js +++ b/src/app/lib/analyticsUtils/sendBeacon/index.js @@ -1,4 +1,3 @@ -import isLive from '../../utilities/isLive'; import onClient from '../../utilities/onClient'; import nodeLogger from '../../logger.node'; import { ATI_LOGGING_ERROR } from '../../logger.const'; @@ -107,7 +106,7 @@ const callReverb = async eventDetails => { const sendBeacon = async (url, reverbBeaconConfig) => { if (onClient()) { try { - if (!isLive() && reverbBeaconConfig) { + if (reverbBeaconConfig) { const { params: { page, user }, eventDetails, From 521a70e384758892e4c6b38d2ed54a27c17c9fdb Mon Sep 17 00:00:00 2001 From: alex-magana Date: Thu, 6 Feb 2025 18:28:24 +0300 Subject: [PATCH 02/11] Assert Reverb invocation on LIVE --- src/app/lib/analyticsUtils/sendBeacon/index.test.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/app/lib/analyticsUtils/sendBeacon/index.test.js b/src/app/lib/analyticsUtils/sendBeacon/index.test.js index faac7b9eef6..dbc1ddc52f2 100644 --- a/src/app/lib/analyticsUtils/sendBeacon/index.test.js +++ b/src/app/lib/analyticsUtils/sendBeacon/index.test.js @@ -116,20 +116,20 @@ describe('sendBeacon', () => { process.env.SIMORGH_APP_ENV = 'live'; }); - it('should not call Reverb viewEvent if Reverb config is passed', async () => { + it('should call Reverb viewEvent if Reverb config is passed', async () => { const sendBeacon = require('./index').default; await sendBeacon('https://foobar.com', reverbConfig); - expect(reverbMock.viewEvent).not.toHaveBeenCalled(); + expect(reverbMock.viewEvent).toHaveBeenCalled(); }); - it('should call "fetch" when Reverb config is passed', async () => { + it('should not call "fetch" when Reverb config is passed', async () => { const sendBeacon = require('./index').default; await sendBeacon('https://foobar.com', reverbConfig); - expect(fetch).toHaveBeenCalledTimes(1); + expect(fetch).not.toHaveBeenCalledTimes(1); }); }); }); From 9fd2b5c6d92e587b5891dbad00727dbebcab1d73 Mon Sep 17 00:00:00 2001 From: alex-magana Date: Thu, 6 Feb 2025 20:32:26 +0300 Subject: [PATCH 03/11] Check Reverb is not invoked when disabled for a service --- .../analyticsUtils/sendBeacon/index.test.js | 24 +++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/src/app/lib/analyticsUtils/sendBeacon/index.test.js b/src/app/lib/analyticsUtils/sendBeacon/index.test.js index dbc1ddc52f2..31131c9b5b9 100644 --- a/src/app/lib/analyticsUtils/sendBeacon/index.test.js +++ b/src/app/lib/analyticsUtils/sendBeacon/index.test.js @@ -80,6 +80,14 @@ describe('sendBeacon', () => { expect(reverbMock.viewEvent).toHaveBeenCalledTimes(1); }); + it('should not call Reverb viewEvent if Reverb is not enabled for a service', async () => { + const sendBeacon = require('./index').default; + + await sendBeacon('https://foobar.com', null); + + expect(reverbMock.viewEvent).not.toHaveBeenCalled; + }); + it('should not call "fetch" if Reverb config is passed', async () => { const sendBeacon = require('./index').default; @@ -102,6 +110,14 @@ describe('sendBeacon', () => { expect(reverbMock.viewEvent).toHaveBeenCalledTimes(1); }); + it('should not call Reverb viewEvent if Reverb is not enabled for a service', async () => { + const sendBeacon = require('./index').default; + + await sendBeacon('https://foobar.com', null); + + expect(reverbMock.viewEvent).not.toHaveBeenCalled; + }); + it('should not call "fetch" if Reverb config is passed', async () => { const sendBeacon = require('./index').default; @@ -124,6 +140,14 @@ describe('sendBeacon', () => { expect(reverbMock.viewEvent).toHaveBeenCalled(); }); + it('should not call Reverb viewEvent if Reverb is not enabled for a service', async () => { + const sendBeacon = require('./index').default; + + await sendBeacon('https://foobar.com', null); + + expect(reverbMock.viewEvent).not.toHaveBeenCalled; + }); + it('should not call "fetch" when Reverb config is passed', async () => { const sendBeacon = require('./index').default; From 4b81e17b6927e27c221262111759f532de7fa847 Mon Sep 17 00:00:00 2001 From: alex-magana Date: Thu, 6 Feb 2025 20:59:19 +0300 Subject: [PATCH 04/11] Fix test assertion --- src/app/lib/analyticsUtils/sendBeacon/index.test.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/app/lib/analyticsUtils/sendBeacon/index.test.js b/src/app/lib/analyticsUtils/sendBeacon/index.test.js index 31131c9b5b9..a6aeac13ba1 100644 --- a/src/app/lib/analyticsUtils/sendBeacon/index.test.js +++ b/src/app/lib/analyticsUtils/sendBeacon/index.test.js @@ -85,7 +85,7 @@ describe('sendBeacon', () => { await sendBeacon('https://foobar.com', null); - expect(reverbMock.viewEvent).not.toHaveBeenCalled; + expect(reverbMock.viewEvent).not.toHaveBeenCalled(); }); it('should not call "fetch" if Reverb config is passed', async () => { @@ -115,7 +115,7 @@ describe('sendBeacon', () => { await sendBeacon('https://foobar.com', null); - expect(reverbMock.viewEvent).not.toHaveBeenCalled; + expect(reverbMock.viewEvent).not.toHaveBeenCalled(); }); it('should not call "fetch" if Reverb config is passed', async () => { @@ -145,7 +145,7 @@ describe('sendBeacon', () => { await sendBeacon('https://foobar.com', null); - expect(reverbMock.viewEvent).not.toHaveBeenCalled; + expect(reverbMock.viewEvent).not.toHaveBeenCalled(); }); it('should not call "fetch" when Reverb config is passed', async () => { From 7593d5bbb84aa7fc735d7a4f2c0404f64b721e7e Mon Sep 17 00:00:00 2001 From: alex-magana Date: Fri, 7 Feb 2025 16:44:30 +0300 Subject: [PATCH 05/11] Refactor mocking of onClient function --- .../analyticsUtils/sendBeacon/index.test.js | 31 +++---------------- 1 file changed, 4 insertions(+), 27 deletions(-) diff --git a/src/app/lib/analyticsUtils/sendBeacon/index.test.js b/src/app/lib/analyticsUtils/sendBeacon/index.test.js index a6aeac13ba1..eee423ad7f6 100644 --- a/src/app/lib/analyticsUtils/sendBeacon/index.test.js +++ b/src/app/lib/analyticsUtils/sendBeacon/index.test.js @@ -1,6 +1,8 @@ /* eslint-disable global-require */ import loggerMock from '#testHelpers/loggerMock'; import { ATI_LOGGING_ERROR } from '#app/lib/logger.const'; +import sendBeacon from './index'; +import * as onClient from '../../utilities/onClient'; let fetchResponse; let isOnClient; @@ -17,13 +19,12 @@ window.__reverb = { __reverbLoadedPromise: Promise.resolve(reverbMock), }; +jest.spyOn(onClient, 'default').mockImplementation(() => isOnClient); + describe('sendBeacon', () => { beforeEach(() => { isOnClient = true; fetch.mockImplementation(() => fetchResponse); - jest.mock('../../utilities/onClient', () => jest.fn()); - const onClient = require('../../utilities/onClient'); - onClient.mockImplementation(() => isOnClient); }); afterEach(() => { @@ -31,8 +32,6 @@ describe('sendBeacon', () => { }); it(`should fetch`, () => { - const sendBeacon = require('./index').default; - sendBeacon('https://foobar.com'); expect(fetch).toHaveBeenCalledWith('https://foobar.com', { @@ -43,8 +42,6 @@ describe('sendBeacon', () => { it(`should not fetch when not on client`, () => { isOnClient = false; - const sendBeacon = require('./index').default; - sendBeacon('https://foobar.com'); expect(fetch).not.toHaveBeenCalled(); @@ -73,24 +70,18 @@ describe('sendBeacon', () => { }); it('should call Reverb viewEvent if Reverb config is passed', async () => { - const sendBeacon = require('./index').default; - await sendBeacon('https://foobar.com', reverbConfig); expect(reverbMock.viewEvent).toHaveBeenCalledTimes(1); }); it('should not call Reverb viewEvent if Reverb is not enabled for a service', async () => { - const sendBeacon = require('./index').default; - await sendBeacon('https://foobar.com', null); expect(reverbMock.viewEvent).not.toHaveBeenCalled(); }); it('should not call "fetch" if Reverb config is passed', async () => { - const sendBeacon = require('./index').default; - await sendBeacon('https://foobar.com', reverbConfig); expect(fetch).not.toHaveBeenCalled(); @@ -103,24 +94,18 @@ describe('sendBeacon', () => { }); it('should call Reverb viewEvent if Reverb config is passed', async () => { - const sendBeacon = require('./index').default; - await sendBeacon('https://foobar.com', reverbConfig); expect(reverbMock.viewEvent).toHaveBeenCalledTimes(1); }); it('should not call Reverb viewEvent if Reverb is not enabled for a service', async () => { - const sendBeacon = require('./index').default; - await sendBeacon('https://foobar.com', null); expect(reverbMock.viewEvent).not.toHaveBeenCalled(); }); it('should not call "fetch" if Reverb config is passed', async () => { - const sendBeacon = require('./index').default; - await sendBeacon('https://foobar.com', reverbConfig); expect(fetch).not.toHaveBeenCalled(); @@ -133,24 +118,18 @@ describe('sendBeacon', () => { }); it('should call Reverb viewEvent if Reverb config is passed', async () => { - const sendBeacon = require('./index').default; - await sendBeacon('https://foobar.com', reverbConfig); expect(reverbMock.viewEvent).toHaveBeenCalled(); }); it('should not call Reverb viewEvent if Reverb is not enabled for a service', async () => { - const sendBeacon = require('./index').default; - await sendBeacon('https://foobar.com', null); expect(reverbMock.viewEvent).not.toHaveBeenCalled(); }); it('should not call "fetch" when Reverb config is passed', async () => { - const sendBeacon = require('./index').default; - await sendBeacon('https://foobar.com', reverbConfig); expect(fetch).not.toHaveBeenCalledTimes(1); @@ -167,8 +146,6 @@ describe('sendBeacon', () => { }); it(`should send error to logger`, async () => { - const sendBeacon = require('./index').default; - await sendBeacon('https://foobar.com'); expect(loggerMock.error).toHaveBeenCalledWith(ATI_LOGGING_ERROR, { From 0fd1c08dc9a9dd9d43b7c742a59e36c800887b58 Mon Sep 17 00:00:00 2001 From: alex-magana Date: Fri, 7 Feb 2025 17:00:56 +0300 Subject: [PATCH 06/11] Add explicit reverb null config --- src/app/lib/analyticsUtils/sendBeacon/index.test.js | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/app/lib/analyticsUtils/sendBeacon/index.test.js b/src/app/lib/analyticsUtils/sendBeacon/index.test.js index eee423ad7f6..098c37113e1 100644 --- a/src/app/lib/analyticsUtils/sendBeacon/index.test.js +++ b/src/app/lib/analyticsUtils/sendBeacon/index.test.js @@ -58,6 +58,10 @@ describe('sendBeacon', () => { }, }; + // Simulates reverbParams set to null in ATIAnalytics and sendEventBeacon + // in the event useReverb resolves to 'false' + const reverbConfigWhenReverbIsDisabled = null; + const originalProcessEnv = process.env; afterEach(() => { @@ -124,7 +128,7 @@ describe('sendBeacon', () => { }); it('should not call Reverb viewEvent if Reverb is not enabled for a service', async () => { - await sendBeacon('https://foobar.com', null); + await sendBeacon('https://foobar.com', reverbConfigWhenReverbIsDisabled); expect(reverbMock.viewEvent).not.toHaveBeenCalled(); }); From c8d09693e09a56567c12c8abcaa675c49a2414d6 Mon Sep 17 00:00:00 2001 From: alex-magana Date: Fri, 7 Feb 2025 17:17:26 +0300 Subject: [PATCH 07/11] Fix indentation --- src/app/lib/analyticsUtils/sendBeacon/index.test.js | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/app/lib/analyticsUtils/sendBeacon/index.test.js b/src/app/lib/analyticsUtils/sendBeacon/index.test.js index 098c37113e1..4f24c457403 100644 --- a/src/app/lib/analyticsUtils/sendBeacon/index.test.js +++ b/src/app/lib/analyticsUtils/sendBeacon/index.test.js @@ -128,7 +128,10 @@ describe('sendBeacon', () => { }); it('should not call Reverb viewEvent if Reverb is not enabled for a service', async () => { - await sendBeacon('https://foobar.com', reverbConfigWhenReverbIsDisabled); + await sendBeacon( + 'https://foobar.com', + reverbConfigWhenReverbIsDisabled, + ); expect(reverbMock.viewEvent).not.toHaveBeenCalled(); }); From 2019ae249a40971500cea7a4ce69a1786e7bcb43 Mon Sep 17 00:00:00 2001 From: alex-magana Date: Fri, 7 Feb 2025 17:25:41 +0300 Subject: [PATCH 08/11] Test reverbParams value when Reverb is diabled --- .../components/ATIAnalytics/beacon/index.test.ts | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/src/app/components/ATIAnalytics/beacon/index.test.ts b/src/app/components/ATIAnalytics/beacon/index.test.ts index db1f8ab828b..4a709708e9c 100644 --- a/src/app/components/ATIAnalytics/beacon/index.test.ts +++ b/src/app/components/ATIAnalytics/beacon/index.test.ts @@ -99,6 +99,22 @@ describe('beacon', () => { true, ); }); + + it('should resolve reverbParams to null when Reverb is disabled for a service', () => { + sendEventBeacon({ + type: 'click', + service: 'news', + componentName: 'component', + pageIdentifier: 'pageIdentifier', + detailedPlacement: 'detailedPlacement', + useReverb: false, + }); + + const reverbParams = sendBeaconSpy.mock.calls[0][1]; + + expect(sendBeaconSpy).toHaveBeenCalledTimes(1); + expect(reverbParams).toBeNull(); + }); }); }); }); From 07d031eca77b168d2d7b7fba0c13b4fb8dc02fdf Mon Sep 17 00:00:00 2001 From: alex-magana Date: Fri, 7 Feb 2025 17:45:25 +0300 Subject: [PATCH 09/11] Check reverbParams inference when Reverb is disabled --- .../components/ATIAnalytics/index.test.tsx | 27 +++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/src/app/components/ATIAnalytics/index.test.tsx b/src/app/components/ATIAnalytics/index.test.tsx index b185487e641..cd17bb648a7 100644 --- a/src/app/components/ATIAnalytics/index.test.tsx +++ b/src/app/components/ATIAnalytics/index.test.tsx @@ -1201,5 +1201,32 @@ describe('ATI Analytics Container', () => { }, }); }); + + it('should set reverbParams to null when Reverb is disabled', () => { + const mockCanonical = jest.fn().mockReturnValue('canonical-return-value'); + // @ts-expect-error - we need to mock these functions to ensure tests are deterministic + canonical.default = mockCanonical; + + const { + metadata: { atiAnalytics }, + } = articleDataNews; + + render( + , + { + ...defaultRenderProps, + atiData: atiAnalytics, + isAmp: false, + pageData: articleDataNews, + pageType: ARTICLE_PAGE, + service: 'mundo', + isUK: true, + }, + ); + + const { reverbParams } = mockCanonical.mock.calls[0][0]; + + expect(reverbParams).toBeNull() + }); }); }); From 200d5f84440390b1d0e19bab7cc878e4745114b2 Mon Sep 17 00:00:00 2001 From: alex-magana Date: Fri, 7 Feb 2025 17:56:54 +0300 Subject: [PATCH 10/11] Update comment --- src/app/lib/analyticsUtils/sendBeacon/index.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/app/lib/analyticsUtils/sendBeacon/index.test.js b/src/app/lib/analyticsUtils/sendBeacon/index.test.js index 4f24c457403..9fa7395914b 100644 --- a/src/app/lib/analyticsUtils/sendBeacon/index.test.js +++ b/src/app/lib/analyticsUtils/sendBeacon/index.test.js @@ -58,7 +58,7 @@ describe('sendBeacon', () => { }, }; - // Simulates reverbParams set to null in ATIAnalytics and sendEventBeacon + // Simulates reverbBeaconConfig set to null in ATIAnalytics and sendEventBeacon // in the event useReverb resolves to 'false' const reverbConfigWhenReverbIsDisabled = null; From f4c49b4e30eb101f3f37cb8e18544816cd6549b4 Mon Sep 17 00:00:00 2001 From: alex-magana Date: Fri, 7 Feb 2025 18:02:08 +0300 Subject: [PATCH 11/11] Fix linting errors --- .../ATIAnalytics/beacon/index.test.ts | 2 +- .../components/ATIAnalytics/index.test.tsx | 23 ++++++++----------- 2 files changed, 11 insertions(+), 14 deletions(-) diff --git a/src/app/components/ATIAnalytics/beacon/index.test.ts b/src/app/components/ATIAnalytics/beacon/index.test.ts index 4a709708e9c..df2e8dca97b 100644 --- a/src/app/components/ATIAnalytics/beacon/index.test.ts +++ b/src/app/components/ATIAnalytics/beacon/index.test.ts @@ -111,7 +111,7 @@ describe('beacon', () => { }); const reverbParams = sendBeaconSpy.mock.calls[0][1]; - + expect(sendBeaconSpy).toHaveBeenCalledTimes(1); expect(reverbParams).toBeNull(); }); diff --git a/src/app/components/ATIAnalytics/index.test.tsx b/src/app/components/ATIAnalytics/index.test.tsx index cd17bb648a7..0b5f2ace134 100644 --- a/src/app/components/ATIAnalytics/index.test.tsx +++ b/src/app/components/ATIAnalytics/index.test.tsx @@ -1211,22 +1211,19 @@ describe('ATI Analytics Container', () => { metadata: { atiAnalytics }, } = articleDataNews; - render( - , - { - ...defaultRenderProps, - atiData: atiAnalytics, - isAmp: false, - pageData: articleDataNews, - pageType: ARTICLE_PAGE, - service: 'mundo', - isUK: true, - }, - ); + render(, { + ...defaultRenderProps, + atiData: atiAnalytics, + isAmp: false, + pageData: articleDataNews, + pageType: ARTICLE_PAGE, + service: 'mundo', + isUK: true, + }); const { reverbParams } = mockCanonical.mock.calls[0][0]; - expect(reverbParams).toBeNull() + expect(reverbParams).toBeNull(); }); }); });