[NASAA-205] - Send top stories experiment analytics events #12075

hotinglok · 2024-10-21T15:34:13Z

Overall changes

Adds an <amp-analytics/> component that is added to the page in <AmpExperiment/> if an analyticsConfig is passed in specifically to handle tracking of experiment events. This is in the same format as the config used for tracking page views in ampAnalyticsJson.ts and is used to track view and click events of the experiment Top Stories section that are viewable in Piano. This PR also includes minor refactors to the experimentTopStories helpers file, as well as the existing <ATIAnalytics/> component that allows the amp experiment variant to come through on the page view event too.

Code changes

Adds an optional <amp-analytics/> component to <AmpExperiment/> if an analyticsConfig is passed to the component.
Created analytics config for the top stories experiment.
- This uses buildATIEventTrackUrl() to format events ready to send to Piano via <amp-analytics/>.
  - Exposes the variant prop that exists in getEventInfo() to use the existing 'Onsite Ads - Variant' metric in Piano (renamed as experimentVariant to avoid it being confused with service variants. These events are registered as publisher.click and publisher.display events which is used to represent component view/click events.
Adds data-experiment attribute to both <TopStoriesSection/> components rendered in the article body and the secondary column. This makes it easier to make selectors in the amp-analytics to trigger tracking events for <TopStoriesSection/>.
- This was also done to account for certain cases where events weren't sending because the selector in the config only targets the first element it finds. So the view event for the <TopStoriesSection/> in the secondary column won't send if the selector was only something like section[aria-labelledby='top-stories-heading'].
- As experimentTopStories blocks are always added to the data on valid assets/services, view events for the control variant which only ever shows the <TopStoriesSection/.> in it's original position in the secondary column didn't send view events at all, hence the need for separate tracking of each one.
Added ampExperimentName to atiData that is added conditionally (only when shouldEnableTopStoriesExperiment is true).
- The active variant name is retrieved by AMP via their Variable Substitution using the experiment name. This is added to the page view event to track when people are still not interacting with or seeing the top stories, even if they are being shown higher up.
- Updated relevant buildATIURL functions/types/tests to include ampExperimentName when it exists.
- Other MVT properties are also added to the page view event (MVT Test, MVT Experiment ID, and MVT Variant).
Refactored block insertion logic.
- Updates the halfway point of inserting the experimentTopStories block to account for blocks in the data that are never rendered (mpu and wsoj). It doesn't need to be exactly 50% of the way through, this approximation will suffice for this experiment since it will only ever be off by one position at worst.
Refactored shouldEnableTopStoriesExperiment conditions.
- Added a minimum blocks length requirement to account for shorter articles that lead to the Top Stories section showing above most of the content.
Added more assets to test with.

Testing

Links

Piano dashboard with filter showing relevant columns

Control (no changes to existing behaviour):

Variant name in code: control

News Test asset
News Test breaking news asset - Article too short, section should not move and no events should be sent. Page view event should also not have variant name.
News live asset
News CPS asset
News live one column asset
News short article asset - Article too short, same result as above.
Sport live asset
Sport short asset - Article too short, same result as above.
Sport one column asset

Variant 1 (show Top Stories section halfway into the article content instead of after it):

Variant name in code: show_at_halfway

News Test asset
News Test breaking news asset - Article too short, same result as above.- News live asset
News CPS asset
News live one column asset
News short article asset - Article too short, same result as above.
Sport live asset
Sport short asset - Article too short, same result as above.
Sport one column asset

Events

Ensure the following events for both variants on a valid articles are being sent through in the ATI tag inspector and are visible in the Piano dashboards.

On page view (type 'page' event in Tag Inspector, 'page.display' in Piano dashboard)

All existing properties in the 'Custom Variables' section should remain the same (see example below):

There should be an 'Other' section with the following properties:

Key	Value
mv_test*	Google Discover
mv_experiment_id (Experiment name)	topStoriesExperiment
mv_creation (Variant)	control

*mv_test is convenient to make relevant results show in the 'Content - MV Testing' section in Piano, so this just has the value 'Google Discover' to make it easier to find.

There should not be a detailed placement id (tag inspector)/ detailedPosition (piano dashboard) property for the page view event.

When viewing the Top Stories section (type 'onSiteAds' event in Tag Inspector, 'publisher.display' in Piano dashboard):

An event with an 'onSiteAds' section should appear with the following properties alongside the mv_ events in the 'Other' section above:

When clicking on any promo in the Top Stories section (promo position irrelevant, type 'onSiteAds' event in Tag Inspector, 'publisher.click' in Piano dashboard):

An event with an 'onSiteAds' section should appear with the following properties alongside the mv_ events in the 'Other' section above:

Helpful Links

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

Coding Standards

Repository use guidelines

hotinglok · 2024-10-23T11:20:08Z

src/app/pages/ArticlePage/experimentTopStories/helpers.test.tsx

+jest.mock('../../../lib/analyticsUtils', () => ({
+  ...jest.requireActual('../../../lib/analyticsUtils'),
+  getAtUserId: jest.fn().mockReturnValue('123-456-789'),
+}));


This block is needed because getAtUserId() always generates a random ID.

src/app/components/ATIAnalytics/types.ts

src/app/components/ATIAnalytics/params/genericPage/buildParams.ts

src/app/components/AmpExperiment/index.tsx

src/app/pages/ArticlePage/experimentTopStories/helpers.tsx

iainrj · 2024-10-28T12:41:38Z

src/app/components/AmpExperiment/index.tsx

+  requests: Record<string, unknown>;
+  triggers: Record<string, unknown>;


Question (non-blocking) Can this be typed any more strictly? I can see that we set base, clicks, trackClicks which we could encode here too.

I've updated this a bit with just the things that we've used for the experiment. There's a lot of other optional properties that I've not included, but those can be added whenever they're needed.

…t for regression

karinathomasbbc · 2024-10-29T15:22:48Z

src/app/components/ATIAnalytics/atiUrl/index.ts

@@ -252,6 +263,7 @@ export const buildATIEventTrackUrl = ({
  advertiserID,
  url,
  detailedPlacement,
+  variant,


Confusingly, we use the term variant for services with dual scripts e.g. serbian with cyrillic script has a variant = cyr

If this is not the intention of the term variant here, can we please rename it to something else, to avoid confusion - perhaps experimentVariant or something similar?

I saw this as well, but had a look at the utility and apparently it was set for A/B testing 3 years ago

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

Line 272 in 275b924

variant = '', // not a service variant - used for A/B testing

We've added this here to use as the experiment variant in our use-case because getEventInfo() uses this as a variant for A/B testing, it just wasn't previously passed in here.
As the value just needs to be in the right place in the string, I'll rename it in there too and remove the comment to make it clearer

karinathomasbbc · 2024-10-29T15:25:09Z

src/app/components/ATIAnalytics/params/genericPage/buildParams.ts

@@ -49,6 +50,7 @@ export const buildPageATIParams = ({
    statsDestination,
    timePublished,
    timeUpdated,
+    ...(ampExperimentName && { ampExperimentName }),


nitpick: Is spreading needed here? If ampExperimentName is undefined, it won't be returned?

If I left it as is in the object, the key ampExperimentName would still be there even if the value was falsey. As it is currently written, it won't work without the spread operator because it wouldn't have a valid key/pair if it evaluated as false.

karinathomasbbc · 2024-10-29T15:26:06Z

src/app/components/ATIAnalytics/types.ts

@@ -109,6 +110,7 @@ export interface ATIEventTrackingProps {
  advertiserID?: string;
  url?: string;
  detailedPlacement?: string;
+  variant?: string;


As per comment above - is this an experiment variant or a dual script variant?

michellebbc

I am not seeing the additional variant data you've added to the page.display event appearing in the analytics portal. I suspect they can only go on publisher.* type events?

michellebbc · 2024-10-29T15:09:23Z

src/app/pages/ArticlePage/experimentTopStories/helpers.tsx

    variants: {
      control: 50,
      show_at_halfway: 50,
    },
  },
 };

+const ARTICLE_LENGTH_THRESHOLD = 10;
 const enableExperimentTopStories = ({


Non-blocking suggestion, expecially as it isn't new to this PR: From reading this function name I was expecting it to do some kind of enablement rather than returning a boolean decision.

Good point. I think I was just struggling for a name since shouldEnableExperimentTopStories is returned in the object from 'getExperimentTopStories()`. Happy to accept any other suggestions.

simorgh/src/app/pages/ArticlePage/experimentTopStories/helpers.tsx

Line 81 in 39bcd02

const shouldEnableExperimentTopStories = enableExperimentTopStories({

michellebbc

Good spot with the MVT properties. I've had a quick check in Piano and can see the data 😄

karinathomasbbc

Thanks for sorting the variant vs experimentVariant issue - it's a lot clearer now.

hotinglok · 2024-11-05T13:14:59Z

src/app/components/ATIAnalytics/atiUrl/index.ts

@@ -215,6 +216,31 @@ export const buildATIPageTrackPath = ({
      value: getATIMarketingString(href, campaignType),
      wrap: false,
    },
+    ...(ampExperimentName


After fixing conflicts, there was an initial concern that this might be overridden by the elections banner experiment that is currently being done, but as that is only run on mundo/portuguese, this should be ok because experimentVariant won't be defined.

Co-authored-by: Michelle <[email protected]>

…nt-more-variants

[NASAA-232] - Add additional variants to top stories experiment

hotinglok added 3 commits October 21, 2024 13:38

Add amp-analytics to AmpExperiment

7625d2d

Add tests

ac4cd70

Remove commented out snapshot

ef8f7f1

hotinglok mentioned this pull request Oct 21, 2024

[NASAA-205] - Send top stories experiment analytics events (Old) #12057

Closed

hotinglok added 6 commits October 21, 2024 17:52

Pass down & use producerId

a7cd6af

Make selectors for event triggers more robust

6905317

Update selectors to account for secondary column section

5607655

Refactor scripts in AmpExperiment to use same component

30f746f

Cleanup of tests/types

558bb5e

Add conditions to account for short articles

fc995db

hotinglok commented Oct 23, 2024

View reviewed changes

hotinglok added 5 commits October 24, 2024 12:04

Refactor block insertion logic, exclude articles too short

9c42c11

Add variant to pageview only for pages impacted by experiment

34973c6

Refactor ATIData to accept ampExperimentName, add tests

150f137

Fix test

b723306

Add more test assets

2b79825

hotinglok commented Oct 28, 2024

View reviewed changes

src/app/components/ATIAnalytics/types.ts Outdated Show resolved Hide resolved

Add CPS asset

cae1f67

hotinglok marked this pull request as ready for review October 28, 2024 12:16

iainrj reviewed Oct 28, 2024

View reviewed changes

src/app/components/ATIAnalytics/params/genericPage/buildParams.ts Outdated Show resolved Hide resolved

Remove unnecessary undefined type from ampExperimentName

4766c64

iainrj reviewed Oct 28, 2024

View reviewed changes

src/app/components/AmpExperiment/index.tsx Outdated Show resolved Hide resolved

iainrj reviewed Oct 28, 2024

View reviewed changes

src/app/pages/ArticlePage/experimentTopStories/helpers.tsx Outdated Show resolved Hide resolved

iainrj reviewed Oct 28, 2024

View reviewed changes

hotinglok added 6 commits October 28, 2024 13:00

Minor cleanup

1792eb4

Update analytics config type

c24cdec

Update data attribute name

d97435f

Add position of top stories section to view events, add cymrufyw asse…

6a14fe1

…t for regression

Add position to click events

5e92f4c

Fix click event selectors

4cd546e

karinathomasbbc reviewed Oct 29, 2024

View reviewed changes

michellebbc reviewed Oct 29, 2024

View reviewed changes

hotinglok added 6 commits October 29, 2024 17:46

Fix page view event, renamed variables

85e6a69

Add buildATIPageTrackPath() test

5eac132

Add MVT test property

3295412

Update snapshots

2f64abe

Fix missed prop name update

f585e09

Add valid sport asset

f3d828a

hotinglok requested review from karinathomasbbc and HarveyPeachey October 30, 2024 16:50

hotinglok and others added 3 commits October 30, 2024 16:50

Merge branch 'latest' into nasaa-amp-experiment-analytics

42f3c53

Refactor block insertion to add more blocks

4439bf6

Add other variants & update tests

a2040e3

Chris-Kay approved these changes Nov 1, 2024

View reviewed changes

hotinglok requested a review from michellebbc November 1, 2024 14:19

michellebbc approved these changes Nov 1, 2024

View reviewed changes

hotinglok added 4 commits November 4, 2024 10:32

Add sick typescript refactor

a6a7619

Remove comments in insertBlockAtPosition()

8c67eba

Add types, refactor css

c49dc8d

Refactor types in helpers

745e9dd

karinathomasbbc approved these changes Nov 4, 2024

View reviewed changes

Fix conflicts

e0d0b7a

hotinglok commented Nov 5, 2024

View reviewed changes

hotinglok and others added 2 commits November 5, 2024 13:29

Update src/app/pages/ArticlePage/experimentTopStories/helpers.tsx

ff62b71

Co-authored-by: Michelle <[email protected]>

Linting

5819495

HarveyPeachey approved these changes Nov 6, 2024

View reviewed changes

hotinglok and others added 4 commits November 6, 2024 10:50

Merge branch 'nasaa-amp-experiment-analytics' into nasaa-amp-experime…

b04d1ab

…nt-more-variants

Merge pull request #12119 from bbc/nasaa-amp-experiment-more-variants

b5e725d

[NASAA-232] - Add additional variants to top stories experiment

Merge branch 'latest' into nasaa-amp-experiment-analytics

82475ce

Update bundle size limits

be4d6cb

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[NASAA-205] - Send top stories experiment analytics events #12075

[NASAA-205] - Send top stories experiment analytics events #12075

hotinglok commented Oct 21, 2024 •

edited

Loading

hotinglok Oct 23, 2024

iainrj Oct 28, 2024

hotinglok Oct 28, 2024

karinathomasbbc Oct 29, 2024

HarveyPeachey Oct 29, 2024

hotinglok Oct 29, 2024 •

edited

Loading

karinathomasbbc Oct 29, 2024

hotinglok Oct 29, 2024

karinathomasbbc Oct 29, 2024

michellebbc left a comment

michellebbc Oct 29, 2024

hotinglok Oct 30, 2024

michellebbc left a comment

karinathomasbbc left a comment

hotinglok Nov 5, 2024

		requests: Record<string, unknown>;
		triggers: Record<string, unknown>;

[NASAA-205] - Send top stories experiment analytics events #12075

Are you sure you want to change the base?

[NASAA-205] - Send top stories experiment analytics events #12075

Conversation

hotinglok commented Oct 21, 2024 • edited Loading

Overall changes

Code changes

Testing

Links

Control (no changes to existing behaviour):

Variant 1 (show Top Stories section halfway into the article content instead of after it):

Events

On page view (type 'page' event in Tag Inspector, 'page.display' in Piano dashboard)

When viewing the Top Stories section (type 'onSiteAds' event in Tag Inspector, 'publisher.display' in Piano dashboard):

When clicking on any promo in the Top Stories section (promo position irrelevant, type 'onSiteAds' event in Tag Inspector, 'publisher.click' in Piano dashboard):

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

hotinglok Oct 29, 2024 • 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

michellebbc left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

michellebbc left a comment

Choose a reason for hiding this comment

karinathomasbbc left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hotinglok commented Oct 21, 2024 •

edited

Loading

hotinglok Oct 29, 2024 •

edited

Loading