Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
5d05cd2
WS-2831: init commit, pseudo code comments
Isabella-Mitchell Jul 1, 2026
f9a5b5c
Merge branch 'latest' into WS-2831-spike-reverb-resonance
Isabella-Mitchell Aug 6, 2026
10cbd5e
WS-2831: Install resonance package
Isabella-Mitchell Aug 6, 2026
77aac20
WS-2831: Updates comments
Isabella-Mitchell Aug 6, 2026
b4607e4
WS-2831: Use dummy params to initialise Resonance
Isabella-Mitchell Aug 6, 2026
32491dd
WS-2831: Comments out hardcoded values. Amends Types.
Isabella-Mitchell Aug 7, 2026
bdc3a34
WS-2831: prepare for context provider
Isabella-Mitchell Aug 7, 2026
d1867de
further investigation for option B
victranfield Aug 11, 2026
8221d62
WS-2831: Lint
Isabella-Mitchell Aug 11, 2026
9e5f8dd
WS-2831: Adds site ID look up
Isabella-Mitchell Aug 11, 2026
a5db80f
WS-2831: Trial useResonance serviceContext check in buildParams
Isabella-Mitchell Aug 11, 2026
03913fc
WS-2831: Tidies implementation
Isabella-Mitchell Aug 11, 2026
b10bd2f
WS-2831: Adds to service configs
Isabella-Mitchell Aug 11, 2026
317627e
WS-2831: Tidies comments and imports
Isabella-Mitchell Aug 11, 2026
9a686c9
WS-2831: Configures Resonance as esm for jest
Isabella-Mitchell Aug 12, 2026
987d474
WS-2831: Amend type
Isabella-Mitchell Aug 12, 2026
e7458f9
WS-2831: Adds initial Resonance unit test
Isabella-Mitchell Aug 12, 2026
54b09e5
WS-2831: Updates unit tests based on changes to ReverbContextProvider
Isabella-Mitchell Aug 12, 2026
0f1c960
WS-2831: Handles esm import in jest in NextJS app [copilot]
Isabella-Mitchell Aug 12, 2026
f1957a3
WS-2831: Adds unit tests. Amends types. [copilot]
Isabella-Mitchell Aug 12, 2026
b70bd23
WS-2831: Adds unit tests. [copilot]
Isabella-Mitchell Aug 12, 2026
a6a3e8c
WS-2831: Adds unit test coverage [copilot]
Isabella-Mitchell Aug 13, 2026
f7a2bab
Merge branch 'latest' into WS-2831-spike-reverb-resonance
Isabella-Mitchell Aug 13, 2026
8799bf5
Merge branch 'latest' into WS-2831-spike-reverb-resonance
shayneahchoon Aug 13, 2026
51f4b3a
WS-2831: Adds platform check to limit building ResonanceParams to Can…
Isabella-Mitchell Aug 13, 2026
f89c00a
WS-2831: Removes type import to simplify. Makes Resonance import dynamic
Isabella-Mitchell Aug 13, 2026
763aea9
WS-2831: Updates test readability
Isabella-Mitchell Aug 14, 2026
287adb3
WS-2831: Wraps Resonance call in try catch to ensure Reverb is uneffe…
Isabella-Mitchell Aug 14, 2026
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions jest.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ const unitTests = {
moduleNameMapper: {
...jestDirAlias,
'\\.module\\.(css|scss)$': 'identity-obj-proxy',
'^@bbc/resonance$': '<rootDir>/src/testHelpers/resonanceMock.ts',
},
testEnvironment: 'jsdom',
snapshotSerializers: ['@emotion/jest/serializer'],
Expand Down Expand Up @@ -43,6 +44,7 @@ const clientUnitTests = {
moduleNameMapper: {
...jestDirAlias,
'\\.module\\.(css|scss)$': 'identity-obj-proxy',
'^@bbc/resonance$': '<rootDir>/src/testHelpers/resonanceMock.ts',
},
testEnvironment: '@happy-dom/jest-environment',
snapshotSerializers: ['@emotion/jest/serializer'],
Expand Down
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@
"react-is": "19"
},
"dependencies": {
"@bbc/resonance": "https://mybbc-analytics.files.bbci.co.uk/resonance/resonance-2.7.2.tgz",
"@bbc/reverb-url-helper": "https://mybbc-analytics.files.bbci.co.uk/reverb-url-helper/bbc-reverb-url-helper-2.5.0.tgz",
"@bbc/web-vitals": "2.6.0",
"@emotion/cache": "11.14.0",
Expand Down
78 changes: 77 additions & 1 deletion src/app/components/ATIAnalytics/atiUrl/index.client.test.ts

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As we're renaming things eventually to be more generic, it does seem a little strange that this was always called atiUrl when it doesn't return a URL or even a string.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I. Totally. Agree. Have added to https://bbc.atlassian.net/browse/WS-3129

Original file line number Diff line number Diff line change
@@ -1,6 +1,12 @@
import { ResonanceMode } from '@bbc/resonance';
import { Platforms } from '#app/models/types/global';
import * as getEnvConfigModule from '#app/lib/utilities/getEnvConfig';
import * as genericLabelHelpers from '../../../lib/analyticsUtils';
import { buildReverbAnalyticsModel, buildReverbEventModel } from '.';
import {
buildResonanceAnalyticsModel,
buildReverbAnalyticsModel,
buildReverbEventModel,
} from '.';

const mockAndSet = ({ name, source }, response) => {
source[name] = jest.fn(); // eslint-disable-line no-param-reassign
Expand All @@ -20,6 +26,76 @@ describe('atiUrl', () => {
jest.clearAllMocks();
});

describe('Resonance', () => {
describe('buildResonanceAnalyticsModel', () => {
const input = {
appName: 'news-pidgin',
contentId: 'urn:bbc:optimo:asset:c0000000001o',
contentType: 'article',
language: 'pcm',
statsDestination: 'statsDestination',
siteId: 12345,
hashedId: null,
pageIdentifier: 'pidgin.articles.c0000000001o.page',
producerName: 'PIDGIN',
platform: 'canonical' as Platforms,
};

it('should return the correct Resonance analytics model', () => {
const result = buildResonanceAnalyticsModel(input);

expect(result.resonanceProperties).toEqual({
mode: ResonanceMode.TEST,
});
expect(result.baseProperties).toEqual({
app: { name: 'news-pidgin' },
destination: 'statsDestination',
hashedUserId: undefined,
pageName: 'pidgin.articles.c0000000001o.page',
producer: 'PIDGIN',
siteId: 12345,
});
expect(result.pageviewProperties).toEqual({
contentId: 'urn:bbc:optimo:asset:c0000000001o',
contentType: 'article',
language: 'pcm',
destination: 'statsDestination',
producer: 'PIDGIN',
});
});

it('should suffix app name with "-app" when platform is app', () => {
const result = buildResonanceAnalyticsModel({
...input,
platform: 'app' as Platforms,
});

expect(result.baseProperties.app).toEqual({ name: 'news-pidgin-app' });
});

it('should pass hashedId through as hashedUserId when provided', () => {
const result = buildResonanceAnalyticsModel({
...input,
hashedId: 'abc123hasheduser',
});

expect(result.baseProperties.hashedUserId).toBe('abc123hasheduser');
});

it('should use LIVE mode when SIMORGH_APP_ENV is live', () => {
jest
.spyOn(getEnvConfigModule, 'getEnvConfig')
.mockReturnValue({ SIMORGH_APP_ENV: 'live' } as ReturnType<
typeof getEnvConfigModule.getEnvConfig
>);

const result = buildResonanceAnalyticsModel(input);

expect(result.resonanceProperties.mode).toBe(ResonanceMode.LIVE);
});
});
});

describe('Reverb', () => {
describe('buildReverbAnalyticsModel', () => {
beforeEach(() => {
Expand Down
41 changes: 41 additions & 0 deletions src/app/components/ATIAnalytics/atiUrl/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,54 @@ import {
ATIEventTrackingProps,
ATIPageTrackingProps,
ReverbBeaconConfig,
ResonanceBeaconConfig,
} from '../types';

/*
* For AMP pages, certain browser and device values are determined
* https://github.com/ampproject/amphtml/blob/master/spec/amp-var-substitutions.md#device-and-browser
*/

const RESONANCE_MODE = { LIVE: 'live', TEST: 'test' } as const;

export const buildResonanceAnalyticsModel = ({
appName,
contentId,
contentType,
language,
statsDestination,
siteId,
hashedId,
pageIdentifier,
producerName,
platform,
}: ATIPageTrackingProps): ResonanceBeaconConfig => {
const env = getEnvConfig().SIMORGH_APP_ENV;

return {
resonanceProperties: {
mode: env === 'live' ? RESONANCE_MODE.LIVE : RESONANCE_MODE.TEST,
},
baseProperties: {
app: {
name: platform === 'app' ? `${appName}-app` : appName,
},
destination: statsDestination,
hashedUserId: hashedId ?? undefined,

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Resonance types permits hashedUserId to be a string or undefined. However reverb permits the equivalent value hashedId to be string, undefined or null. Currently, we default this to null. I'll make a note on a follow up ticket: https://bbc.atlassian.net/browse/WS-2949

pageName: pageIdentifier,
producer: producerName,
siteId,
},
pageviewProperties: {
contentId,
contentType,
language,
destination: statsDestination,
producer: producerName,
},
} as ResonanceBeaconConfig;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need the type casting here? I would have expected typescript to infer this ok from the output type defined at the top.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm no it's a bit unhappy if I remove it. The types in the shared ATIPageTrackingProps are different to what ResonanceBeaconConfig is expecting.

E.g. Reverb permits contentId to be string | null | undefined.
But Resonance only permits contentId to be string | undefined.

That as ResonanceBeaconConfig is needed to surpress a lot of TS issues. We could get rid of the output type without any issues. But I don't see the harm in it.

The solution would be typing it ourselves, rather than using the imported types (which I did put a question mark on)

};

export const buildReverbAnalyticsModel = ({
appName,
campaigns,
Expand Down
68 changes: 67 additions & 1 deletion src/app/components/ATIAnalytics/canonical/index.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import { addSendStaticBeaconToWindow } from '#app/lib/analyticsUtils/staticATITr
import processClientDeviceAndSendStaticBeacon from '#app/lib/analyticsUtils/staticATITracking/processClientDeviceAndSendStaticBeacon';
import * as beacon from '../../../lib/analyticsUtils/sendBeacon';
import CanonicalATIAnalytics from '.';
import { ReverbBeaconConfig } from '../types';
import { ResonanceBeaconConfig, ReverbBeaconConfig } from '../types';

describe('Canonical ATI Analytics', () => {
afterEach(() => {
Expand Down Expand Up @@ -53,6 +53,24 @@ describe('Canonical ATI Analytics', () => {
},
} as ReverbBeaconConfig;

const mockResonanceParams = {
resonanceProperties: { mode: 'test' },
baseProperties: {
app: { name: 'news-pidgin' },
destination: 'statsDestination',
pageName: 'pidgin.articles.c0000000001o.page',
producer: 'PIDGIN',
siteId: 12345,
},
pageviewProperties: {
contentId: 'urn:bbc:optimo:asset:c0000000001o',
contentType: 'article',
language: 'pcm',
destination: 'statsDestination',
producer: 'PIDGIN',
},
} as ResonanceBeaconConfig;

const mockSendBeacon = jest.fn().mockReturnValue('beacon-return-value');
// @ts-expect-error - we need to mock these functions to ensure tests are deterministic
beacon.default = mockSendBeacon;
Expand Down Expand Up @@ -125,6 +143,54 @@ describe('Canonical ATI Analytics', () => {
expect(mockSendBeacon).not.toHaveBeenCalled();
});

it('should call sendBeacon with resonanceParams when provided', () => {
jest.spyOn(isOperaProxy, 'default').mockImplementation(() => false);

act(() => {
render(
<CanonicalATIAnalytics
reverbParams={mockReverbParams}
resonanceParams={mockResonanceParams}
/>,
);
});

expect(mockSendBeacon).toHaveBeenCalledWith(
mockReverbParams,
mockResonanceParams,
);
});

it('should call sendBeacon with undefined resonanceParams when not provided', () => {
jest.spyOn(isOperaProxy, 'default').mockImplementation(() => false);

act(() => {
render(
<CanonicalATIAnalytics
reverbParams={mockReverbParams}
resonanceParams={undefined}
/>,
);
});

expect(mockSendBeacon).toHaveBeenCalledWith(mockReverbParams, undefined);
});

it('should call sendBeacon with null resonanceParams when provided as null', () => {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just want to check this is the desired behaviour. Would it make more sense for this and the case above to always return either null or undefined?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes I agree this looks a bit strange.

My logic was that when the page first loads Resonance (and Reverb) is undefined. But once the params are set, they are an object. The different with Resonance is that they can be null if it's not an applicable service or platform.

We don't currently test for when ReverbParams are 'undefined'. But they definitely can be. If I had to bin one of the tests, it would be this one.

(Note this is typed here as undefined because this is also shared by the AmpATIAnalytics component.

I've made a small tweak of the test to improve readability.

jest.spyOn(isOperaProxy, 'default').mockImplementation(() => false);

act(() => {
render(
<CanonicalATIAnalytics
reverbParams={mockReverbParams}
resonanceParams={null}
/>,
);
});

expect(mockSendBeacon).toHaveBeenCalledWith(mockReverbParams, null);
});

it('should render a noscript image for non-JS users', () => {
const { container } = render(
<CanonicalATIAnalytics reverbParams={mockReverbParams} />,
Expand Down
10 changes: 7 additions & 3 deletions src/app/components/ATIAnalytics/canonical/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,10 @@ const addScript = ({ script, parameters, nonce }: InlineScriptProps) => {
return <Helmet>{addInlineScript({ script, parameters, nonce })}</Helmet>;
};

const CanonicalATIAnalytics = ({ reverbParams }: ATIAnalyticsProps) => {
const CanonicalATIAnalytics = ({
reverbParams,
resonanceParams,
}: ATIAnalyticsProps) => {
const { isLite, nonce } = use(RequestContext);

usePWAInstallTracker();
Expand All @@ -48,10 +51,11 @@ const CanonicalATIAnalytics = ({ reverbParams }: ATIAnalyticsProps) => {
usePWAOfflineTracking();

const [reverbBeaconConfig] = useState(reverbParams);
const [resonanceBeaconConfig] = useState(resonanceParams);

useEffect(() => {
if (!isOperaProxy()) sendBeacon(reverbBeaconConfig);
}, [reverbBeaconConfig]);
if (!isOperaProxy()) sendBeacon(reverbBeaconConfig, resonanceBeaconConfig);
}, [reverbBeaconConfig, resonanceBeaconConfig]);

const liteSiteReverbURL = reverbUrlHelper.getLitePageViewUrl(reverbParams);
const operaMiniPageViewReverbURL =
Expand Down
Loading
Loading