Corrresponding resonance values added - #14385
Draft
victranfield wants to merge 2 commits into
Draft
Conversation
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
There are maintainability/documentation issues in the changed code (commented-out production/test code and removed activation-beacon JSDoc) that should be resolved before approval.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR updates ATI Analytics’ Resonance beacon model to include additional parity fields (e.g., page title, published/updated timestamps, LDP tags/IDs, section) and aligns existing unit tests with the expanded model so the suite passes.
Changes:
- Extend
buildResonanceAnalyticsModelto optionally includepageTitle,publicationDate,pubUpdateDate,ldpTags,ldpIds, andsection. - Update ATIAnalytics container/unit tests to expect the additional Resonance properties.
- Update
atiUrlunit tests for the Resonance model to include assertions for the new optional fields.
File summaries
| File | Description |
|---|---|
src/app/components/ATIAnalytics/index.client.test.tsx |
Updates the container test expectations to include the newly-added Resonance pageview properties. |
src/app/components/ATIAnalytics/atiUrl/index.ts |
Adds optional parity fields into the Resonance beacon payload (and removes some inline documentation in the activation model). |
src/app/components/ATIAnalytics/atiUrl/index.client.test.ts |
Updates Resonance model unit tests to validate the new optional fields. |
Review details
Suppressed comments (1)
src/app/components/ATIAnalytics/atiUrl/index.client.test.ts:105
- Avoid committing commented-out tests; this block reads like a pending coverage check for optional fields, but as comments it won’t protect behaviour. Either re-enable it (and ensure the assertions pass) or remove it entirely.
// it('should omit optional parity fields when provided as empty strings', () => {
// const result = buildResonanceAnalyticsModel({
// ...input,
// pageTitle: '',
// timePublished: '',
- Files reviewed: 3/3 changed files
- Comments generated: 3
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+88
to
+100
| // it('should return url and referrerUrl using getHref and getReferrer', () => { | ||
| // const result = buildResonanceAnalyticsModel(input); | ||
|
|
||
| // expect(result.pageviewProperties.url).toBe('getHref'); | ||
| // expect(result.pageviewProperties.referrerUrl).toBe('getReferrer'); | ||
| // }); | ||
|
|
||
| // it('should populate app.type using getAppType', () => { | ||
| // const result = buildResonanceAnalyticsModel(input); | ||
|
|
||
| // expect(result.baseProperties.app.type).toBe('getAppType'); | ||
| // }); | ||
|
|
Comment on lines
53
to
60
| const env = getEnvConfig().SIMORGH_APP_ENV; | ||
| // const href = getHref(platform); | ||
| // const referrer = getReferrer(platform); | ||
|
|
||
| // const aggregatedCampaigns = (Array.isArray(campaigns) ? campaigns : []) | ||
| // .map(({ campaignName }) => campaignName) | ||
| // .join('~'); | ||
|
|
| * Follows the "Activation (v1.0.1) on Web" event-catalogue spec (viewability model), | ||
| * spec ID ACTIVATION_EVENT_SPEC_ID - see https://broxy.tools.bbc.co.uk/bbc-event-catalogue/xbbc/viewability-events/specs/experiment/activation-web/1.0.1/ | ||
| */ | ||
| export const buildActivationEventModel = ({ |
victranfield
changed the title
victranfield
marked this pull request as draft
September 10, 2026 15:05
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Resolves JIRA: https://bbc.atlassian.net/browse/WS-2949
Summary
Unit tests fixed, Resonance values added to ATIAnalytics/atiUrl/index
Code changes
Testing
Useful Links