WS-2925 - Migrate inline js code for Piano tracking on Opera Mini - #14258
Open
alex-magana wants to merge 24 commits into
Open
WS-2925 - Migrate inline js code for Piano tracking on Opera Mini#14258alex-magana wants to merge 24 commits into
alex-magana wants to merge 24 commits into
Conversation
alex-magana
changed the title
…ub.com:bbc/simorgh into TRIAGE-2-migrate-inline-js-code-for-opera-mini
Contributor
There was a problem hiding this comment.
Pull request overview
This PR migrates Opera Mini Piano/ATI tracking inline JS from string/eval-style snippets to callable functions, allowing safer composition of inline scripts and reducing reliance on runtime string evaluation.
Changes:
- Converted
sendStaticBeaconand Opera Mini page-view beacon logic from string-returning helpers to real functions invoked viaaddInlineScript. - Extended
addInlineScriptto support passing functions as script parameters (to inline via.toString()), and added/updated unit tests accordingly. - Updated global typing for
window.sendStaticBeaconto reflect the newvoidreturn.
Reviewed changes
Copilot reviewed 8 out of 9 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/global.d.ts | Updates window.sendStaticBeacon typing to void return. |
| src/app/lib/utilities/addInlineScript/index.tsx | Extends inline-script parameter handling to support function parameters. |
| src/app/lib/utilities/addInlineScript/index.test.tsx | Adds coverage for function parameters in inline scripts. |
| src/app/lib/analyticsUtils/staticATITracking/sendStaticBeacon/index.ts | Replaces string-based inline code with functions that attach/call window.sendStaticBeacon. |
| src/app/lib/analyticsUtils/staticATITracking/sendStaticBeacon/index.test.ts | Updates tests to call functions directly instead of eval. |
| src/app/components/ATIAnalytics/canonical/sendPageViewBeaconOperaMini/index.ts | Refactors Opera Mini page-view beacon to a callable function with injected isOperaProxy dependency. |
| src/app/components/ATIAnalytics/canonical/sendPageViewBeaconOperaMini/index.client.test.ts | Updates client test to avoid eval and verify new callable behavior. |
| src/app/components/ATIAnalytics/canonical/index.tsx | Switches Helmet-injected scripts to use function references + parameters. |
| src/app/components/ATIAnalytics/canonical/index.test.tsx | Updates expectations for new inline script output and adds Opera Mini script presence test. |
Comment on lines
1
to
5
| export type InlineScriptProps = { | ||
| script: string | { toString: () => string }; | ||
| parameters?: string | string[]; | ||
| parameters?: string | string[] | (string | { toString: () => string })[]; | ||
| nonce?: string | null; | ||
| }; |
Comment on lines
13
to
16
| beforeEach(() => { | ||
| XMLHttpRequestSpy = jest.spyOn(window, 'XMLHttpRequest'); | ||
| eval(addSendStaticBeaconToWindow()); | ||
| addSendStaticBeaconToWindow(); | ||
| }); |
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: WS-2925
Summary
Update Opera Mini & Lite site Piano inline JS tracking code to meet coding standards.
Code changes
src/app/components/ATIAnalytics/canonical/sendPageViewBeaconOperaMini/index.tso ES5 toenable tracking in Opera Mini 4.5.33867.
src/app/lib/analyticsUtils/staticATITracking/sendStaticBeacon/index.tsto ES5 toenable dispatch of Piano requests in Opera Mini 4.5.33867.
Testing
See WS-2925 for links to the Piano reports for monitoring/testing.
Sample Assets
Expected Behaviour Matrix
Findings
To test tracking on Preview 2, we appended text to the app name property here and here. We then used
appropriate filters in Piano to ascertain that tracking data surfaces in Piano.
Opera Mini 4.5.33867 does not support arrow functions. Tracking on Lite and AMP platforms fails due to
the use of arrow functions in processClientDeviceAndSendStaticBeacon.
We attempted to use the configuration set out in the block below in
ws-nextjs-app/next.config.jsto transpile the ES6 code to ES5. However, this failed with errors. We therefore had to rewrite
the functions manually.
Useful Links