WSTEAM1-1514: Add click tracking at top level #12360

shayneahchoon · 2025-02-03T15:25:15Z

Resolves JIRA WSTEAM1-1514 & WORLDSERVICE-91

Overall changes

Implements changes from #12332

Code changes

Uses immediately invoked function expression instead of a string to outline script.
Adds unit tests.
Adds tracking script at top level of document.
Adds lite tracking on Most Read links only.
IMPORTANT: After this PR is merged, the lite site will be setting cookies on the client side. We may need to add a consent banner in the future if we plan to rollout lite sites to countries that require user consent.

Testing

List the steps used to test this PR.

Helpful Links

Add Links to useful resources related to this PR if applicable.

Coding Standards

Repository use guidelines

src/server/utilities/clickTrackingScript/clickTracking.test.js

src/app/components/MostRead/Canonical/Item/index.tsx

src/app/hooks/useClickTrackerHandler/index.jsx

src/app/components/MostRead/Canonical/Item/index.tsx

src/server/utilities/clickTrackingScript/clickTracking.js

src/server/utilities/clickTrackingScript/clickTracking.test.js

src/app/hooks/useClickTrackerHandler/index.jsx

src/server/utilities/clickTrackingScript/clickTracking.js

karinathomasbbc · 2025-02-03T15:57:13Z

src/server/utilities/clickTrackingScript/clickTracking.js

+          }
+        }
+
+        if (!atUserIdValue && crypto.randomUUID) {


Do we need to worry about Opera Mini + Lite, or does this work? Or is the point of Opera Mini that all users have the same ID, so it would be impossible for us to detect unique users?

We'll check this on Opera Mini on preview

I have a feeling that using the opera mini proxy, the userId was the same, so there was little/no value including this value - in the canonical / full fat solution, we don't bother generating the uuid. We might need to do the same thing here?

simorgh/src/app/lib/analyticsUtils/index.js

Lines 198 to 204 in b340e53

export const getAtUserId = () => {

if (!onClient()) return null;

// Users accessing the site on opera "extreme data saving mode" have the pages rendered by an intermediate service

// Attempting to track these users is just tracking that proxy, causing all opera mini visitors to have the same id

if (isOperaProxy()) return null;

src/app/hooks/useClickTrackerHandler/index.jsx

src/server/Document/Renderers/LiteRenderer.tsx

src/app/components/MostRead/Canonical/Item/index.tsx

src/server/utilities/liteATIClickTracking/index.test.ts

src/app/components/MostRead/Canonical/Item/index.tsx

src/server/Document/__snapshots__/component.test.jsx.snap

src/server/utilities/liteATIClickTracking/index.test.ts

src/app/hooks/useClickTrackerHandler/README.mdx

src/app/components/MostRead/Canonical/Item/index.tsx

src/app/hooks/useClickTrackerHandler/README.mdx

src/app/hooks/useClickTrackerHandler/index.jsx

src/server/Document/__snapshots__/component.test.jsx.snap

src/server/utilities/liteATIClickTracking/index.ts

src/server/Document/Renderers/LiteRenderer.tsx

src/server/utilities/liteATIClickTracking/index.ts

amoore108 · 2025-02-11T15:41:44Z

src/server/utilities/liteATIClickTracking/index.ts

+        }
+
+        if (!atUserIdValue && crypto.randomUUID) {
+          atUserIdValue = crypto.randomUUID();


Similarly here, crypto may not be natively supported in Opera Mini. We have a polyfill from IE here:

simorgh/src/poly/legacy.js

Lines 15 to 21 in 2662a70

import getRandomValues from 'polyfill-crypto.getrandomvalues';

if (!window.crypto && !window.msCrypto) {

window.crypto = {

getRandomValues,

};

}

which Opera could be using.

I've been working on a PR which replaces uuid with crypto - wonder if we could borrow the generateUUID code from there? https://github.com/bbc/simorgh/blob/38995753d1d339c9f1f937d896610094e9f0ea48/src/app/lib/utilities/getUUID/index.ts

Also, crypto.randomUUID doesn't work on http, only https, meaning it wouldn't work properly on local... not a massive issue I suppose?

amoore108 · 2025-02-11T15:43:38Z

Looks like a neat solution!

Just some comments on Opera Mini, as ultimately the Lite site will become its default case. We'll just need to be careful writing plain ol' JS as it won't be getting transpiled down to syntax that Opera Mini supports.

EDIT: The way this is implemented does let it transpile down, so the syntax should be ok 👍

amoore108 · 2025-02-11T15:52:27Z

src/server/utilities/liteATIClickTracking/index.ts

+        if (atiURL == null) {
+          return;
+        }


Will this not prevent links from proceeding if they don't have the ATI data-attribute?

karinathomasbbc · 2025-02-11T12:30:24Z

src/server/utilities/liteATIClickTracking/index.test.ts

@@ -0,0 +1,117 @@
+import { LITE_ATI_TRACKING } from '#app/hooks/useClickTrackerHandler';
+import trackingScript from '.';


Suggested change

import trackingScript from '.';

import liteATIClickTracking from '.';

karinathomasbbc · 2025-02-11T12:31:25Z

src/server/Document/component.test.jsx

+function mockTrackingScript() {
+  return 'Tracking script placeholder';
+}
+jest.mock('#src/server/utilities/liteATIClickTracking', () =>
+  mockTrackingScript.toString(),
+);


Suggested change

function mockTrackingScript() {

return 'Tracking script placeholder';

}

jest.mock('#src/server/utilities/liteATIClickTracking', () =>

mockTrackingScript.toString(),

);

function liteATIClickTracking() {

return 'Tracking script placeholder';

}

jest.mock('#src/server/utilities/liteATIClickTracking', () =>

liteATIClickTracking.toString(),

);

karinathomasbbc · 2025-02-11T12:31:50Z

src/server/utilities/liteATIClickTracking/index.test.ts

+  });
+  beforeEach(() => {


nitpick: newline

Suggested change

});

beforeEach(() => {

});

beforeEach(() => {

karinathomasbbc · 2025-02-11T12:34:03Z

src/server/utilities/liteATIClickTracking/index.test.ts

+    Object.defineProperty(document, 'cookie', {
+      get() {
+        return mockCookie;
+      },
+      set(cookieValue) {
+        mockCookie = cookieValue;
+      },
+    });
+    Object.defineProperty(window, 'location', {
+      value: {
+        assign: jest.fn(),
+      },
+    });
+  });


Should we use mocking here instead of Object.defineProperty?

karinathomasbbc · 2025-02-11T12:35:10Z

src/server/utilities/liteATIClickTracking/index.test.ts

+    const anchorElement = document.createElement('a');
+    anchorElement.setAttribute(
+      LITE_ATI_TRACKING,
+      'https://logws1363.ati-host.net/?',
+    );


We seem to use this in a number of places - is it worth extracting into a separate function & reusing in the tests which need it?

karinathomasbbc · 2025-02-11T12:47:06Z

src/server/utilities/liteATIClickTracking/index.ts

+        let clientSideAtiURL = atiURL
+          .concat('&', 'r=', rValue)
+          .concat('&', 're=', reValue)
+          .concat('&', 'hl=', hlValue);


Can we use URLSearchParams here? It should be supported on all browsers..? Although we may need to check: https://caniuse.com/?search=URLSearchParams

Suggested change

let clientSideAtiURL = atiURL

.concat('&', 'r=', rValue)

.concat('&', 're=', reValue)

.concat('&', 'hl=', hlValue);

const params = new URLSearchParams();

params.append('r', screenResolutionColourDepth);

params.append('re', browserViewportResolution);

params.append('hl', timestamp);

If not, could we do something which is a bit more readable - using object key values before appending them all at the bottom - this will make it a bit more future proof if we ever decide to add new params.

Suggested change

let clientSideAtiURL = atiURL

.concat('&', 'r=', rValue)

.concat('&', 're=', reValue)

.concat('&', 'hl=', hlValue);

const params = {

r: screenResolutionColourDepth,

re: browserViewportResolution,

...

...(navigator.language && { lng: navigator.language })

}

karinathomasbbc · 2025-02-11T13:05:26Z

src/server/utilities/liteATIClickTracking/index.ts

+          );
+        }
+
+        window.sendBeaconLite(clientSideAtiURL);


Following on from above, if URLSearchParams is supported:

Suggested change

window.sendBeaconLite(clientSideAtiURL);

window.sendBeaconLite(${atiUrl}`&${params.toString()}`)

If not, and we have gone down the params as an object route, then we can do this:

Suggested change

window.sendBeaconLite(clientSideAtiURL);

const paramValues = Object.entries(params).map(([key, value]) => `${key}=${value}`).join('&')

window.sendBeaconLite(${atiUrl}`&${paramValues}`)

src/server/Document/__snapshots__/component.test.jsx.snap

karinathomasbbc · 2025-02-11T16:24:19Z

src/server/utilities/clickTrackingScript/clickTracking.js

+          }
+        }
+
+        if (!atUserIdValue && crypto.randomUUID) {


I have a feeling that using the opera mini proxy, the userId was the same, so there was little/no value including this value - in the canonical / full fat solution, we don't bother generating the uuid. We might need to do the same thing here?

simorgh/src/app/lib/analyticsUtils/index.js

Lines 198 to 204 in b340e53

export const getAtUserId = () => {

if (!onClient()) return null;

// Users accessing the site on opera "extreme data saving mode" have the pages rendered by an intermediate service

// Attempting to track these users is just tracking that proxy, causing all opera mini visitors to have the same id

if (isOperaProxy()) return null;

karinathomasbbc · 2025-02-11T16:31:08Z

src/server/utilities/liteATIClickTracking/index.ts

+        }
+
+        if (!atUserIdValue && crypto.randomUUID) {
+          atUserIdValue = crypto.randomUUID();


I've been working on a PR which replaces uuid with crypto - wonder if we could borrow the generateUUID code from there? https://github.com/bbc/simorgh/blob/38995753d1d339c9f1f937d896610094e9f0ea48/src/app/lib/utilities/getUUID/index.ts

Also, crypto.randomUUID doesn't work on http, only https, meaning it wouldn't work properly on local... not a massive issue I suppose?

WSTEAM1-1514: Add click tracking at top level

67888e2

github-advanced-security bot found potential problems Feb 3, 2025

View reviewed changes

src/server/utilities/clickTrackingScript/clickTracking.test.js Fixed Show fixed Hide fixed

src/server/utilities/clickTrackingScript/clickTracking.test.js Fixed Show fixed Hide fixed

WSTEAM1-1514: Update

1e894cf

shayneahchoon commented Feb 3, 2025

View reviewed changes

src/app/components/MostRead/Canonical/Item/index.tsx Outdated Show resolved Hide resolved

karinathomasbbc reviewed Feb 3, 2025

View reviewed changes

WSTEAM1-1514: Update

975419f

karinathomasbbc reviewed Feb 3, 2025

View reviewed changes

src/app/components/MostRead/Canonical/Item/index.tsx Outdated Show resolved Hide resolved

shayneahchoon added 2 commits February 3, 2025 17:00

WSTEAM1-1514: Change to TS:

8c64c73

WSTEAM1-1514: rename

7839f99

github-advanced-security bot found potential problems Feb 3, 2025

View reviewed changes

src/server/utilities/liteATIClickTracking/index.test.ts Fixed Show fixed Hide fixed

src/server/utilities/liteATIClickTracking/index.test.ts Fixed Show fixed Hide fixed

shayneahchoon added 3 commits February 4, 2025 08:53

WSTEAM1-1514: Update

3dcc093

WSTEAM1-1514: Update

e95925f

WSTEAM1-1514: Update

b59c095

karinathomasbbc reviewed Feb 4, 2025

View reviewed changes

shayneahchoon and others added 6 commits February 7, 2025 16:45

WSTEAM1-1514: Update

537508a

WSTEAM1-1514: Update

640e1da

WSTEAM1-1514: Update

9453f07

Merge branch 'latest' into WSTEAM1-1514-LITE-TRACKER-V2

86f0854

Merge branch 'latest' into WSTEAM1-1514-LITE-TRACKER-V2

a12ff7c

Merge branch 'latest' into WSTEAM1-1514-LITE-TRACKER-V2

30ae145

karinathomasbbc reviewed Feb 11, 2025

View reviewed changes

shayneahchoon and others added 6 commits February 11, 2025 10:56

WSTEAM1-1514: Update

6a68ef9

WSTEAM1-1514: Update

a355db9

WS-91: Lint

8e8825d

Merge branch 'latest' into WSTEAM1-1514-LITE-TRACKER-V2

e05c1f9

WS-91: Lint

7d987d7

WS-91: Removes comments

21adf6e

amoore108 reviewed Feb 11, 2025

View reviewed changes

src/server/utilities/liteATIClickTracking/index.ts Show resolved Hide resolved

amoore108 reviewed Feb 11, 2025

View reviewed changes

karinathomasbbc reviewed Feb 11, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

WSTEAM1-1514: Add click tracking at top level #12360

WSTEAM1-1514: Add click tracking at top level #12360

shayneahchoon commented Feb 3, 2025 •

edited by Isabella-Mitchell

Loading

karinathomasbbc Feb 3, 2025

Isabella-Mitchell Feb 11, 2025

karinathomasbbc Feb 11, 2025

amoore108 Feb 11, 2025

karinathomasbbc Feb 11, 2025

amoore108 commented Feb 11, 2025 •

edited

Loading

amoore108 Feb 11, 2025 •

edited

Loading

karinathomasbbc Feb 11, 2025

karinathomasbbc Feb 11, 2025

karinathomasbbc Feb 11, 2025

karinathomasbbc Feb 11, 2025

karinathomasbbc Feb 11, 2025

karinathomasbbc Feb 11, 2025

karinathomasbbc Feb 11, 2025

karinathomasbbc Feb 11, 2025

karinathomasbbc Feb 11, 2025

	export const getAtUserId = () => {
	if (!onClient()) return null;

	// Users accessing the site on opera "extreme data saving mode" have the pages rendered by an intermediate service
	// Attempting to track these users is just tracking that proxy, causing all opera mini visitors to have the same id
	if (isOperaProxy()) return null;

	import getRandomValues from 'polyfill-crypto.getrandomvalues';

	if (!window.crypto && !window.msCrypto) {
	window.crypto = {
	getRandomValues,
	};
	}

		@@ -0,0 +1,117 @@
		import { LITE_ATI_TRACKING } from '#app/hooks/useClickTrackerHandler';
		import trackingScript from '.';

	import trackingScript from '.';
	import liteATIClickTracking from '.';

	window.sendBeaconLite(clientSideAtiURL);
	window.sendBeaconLite(${atiUrl}`&${params.toString()}`)

	window.sendBeaconLite(clientSideAtiURL);
	const paramValues = Object.entries(params).map(([key, value]) => `${key}=${value}`).join('&')
	window.sendBeaconLite(${atiUrl}`&${paramValues}`)

WSTEAM1-1514: Add click tracking at top level #12360

Are you sure you want to change the base?

WSTEAM1-1514: Add click tracking at top level #12360

Conversation

shayneahchoon commented Feb 3, 2025 • edited by Isabella-Mitchell Loading

Overall changes

Code changes

Testing

Helpful Links

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

amoore108 commented Feb 11, 2025 • edited Loading

amoore108 Feb 11, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

shayneahchoon commented Feb 3, 2025 •

edited by Isabella-Mitchell

Loading

amoore108 commented Feb 11, 2025 •

edited

Loading

amoore108 Feb 11, 2025 •

edited

Loading