[DNM] WS-1097 Referrer based reordering of OJ components#13459
[DNM] WS-1097 Referrer based reordering of OJ components#13459
Conversation
LilyL0u
changed the title
…ly on mobile width
…ve mostRead variable, not sure what it did before...
…ovider but also need useState and useEffect for hydration)
src/app/legacy/containers/PageHandlers/withOptimizelyProvider/index.client.test.tsx
Show resolved
Hide resolved
…thub.com:bbc/simorgh into WS-1097-referrer-based-ordering-of-oj-components
| <RelatedContentSection | ||
| content={articleBlocks} | ||
| {...(referrerVariant && { | ||
| experimentProps: { |
There was a problem hiding this comment.
Is this object being used in the RelatedContentSection? I can see it as an optional prop in the type defintion, but it doesn't appear to be getting used in the view tracker.
There was a problem hiding this comment.
Me and @pvaliani were talking about this on Friday. I wanted to look into it more, because we ARE getting related content clicks through in optimizely despite the experimentProps not being used
There was a problem hiding this comment.
I mean, we were in previous experiments. Anyway, in the branch off of this one that adds the oj-clicks event, Pedram has added code to use the experimentProps
There was a problem hiding this comment.
these things passed to be into components in a few different ways (as we talked about in daily sync). Makes it a bit confusing
There was a problem hiding this comment.
They were used in the time of day experiments, but removed once they were concluded: https://github.com/bbc/simorgh/pull/13589/changes#diff-dd4bcc7b6d8e420fd12a0efc3bd4dc7a8b103d0f6c2c75c7ef6df612dd564c7f
There was a problem hiding this comment.
ah I see. I am going through each component now and making sure they have everything
There was a problem hiding this comment.
Went through each component relavent to this experiment and saw that they have both view and click events from them:
LilyL0u
force-pushed
the
WS-1097-referrer-based-ordering-of-oj-components
branch
from
aa19c62 to
f04331a
Compare
amoore108
left a comment
amoore108
left a comment
There was a problem hiding this comment.
Thanks for all the hard work on this, very complex thing and something we can look at trying to simplify (in general, not this particular one).
There was a problem hiding this comment.
Pull request overview
This PR implements a referrer-based adaptive experiment that reorders onward journey (OJ) components based on how users arrived at an article page. The experiment shows different content recommendations to users from search, social, and direct traffic sources.
Changes:
- Introduces a referrer-based experiment that dynamically selects which OJ component to display mid-article based on referrer variant (Most Read for control, Related Content for search, Top Stories for direct, Features for social)
- Duplicates Top Stories and Features components to render in both secondary column (desktop) and under article content (mobile/tablet), using CSS display properties to show/hide based on viewport
- Enhances event tracking to include experiment metadata and granular item-level tracking data
Reviewed changes
Copilot reviewed 24 out of 25 changed files in this pull request and generated 11 comments.
Show a summary per file
| File | Description |
|---|---|
| ws-nextjs-app/cypress/e2e/testsForAllPages.ts | Filters visible images to avoid testing hidden duplicate components |
| ws-nextjs-app/cypress/e2e/specialFeatures/atiAnalytics/assertions/topStories.ts | Updates selectors to target only visible components for analytics tests |
| ws-nextjs-app/cypress/e2e/specialFeatures/atiAnalytics/assertions/relatedContent.ts | Updates selectors to target only visible components for analytics tests |
| ws-nextjs-app/cypress/e2e/specialFeatures/atiAnalytics/assertions/featuresAnalysis.ts | Updates selectors to target only visible components for analytics tests |
| src/app/pages/ArticlePage/index.test.tsx | Updates test to handle multiple images with same alt text from duplicated components |
| src/app/pages/ArticlePage/helpers/index.tsx | Implements component reordering logic based on referrer variant and referrer type |
| src/app/pages/ArticlePage/helpers/index.test.tsx | Tests component ordering for different referrer variants and edge cases |
| src/app/pages/ArticlePage/SecondaryColumn.tsx | Passes experiment props to Top Stories and Features components |
| src/app/pages/ArticlePage/PagePromoSections/TopStoriesSection/types.ts | Adds type definitions for promo headline blocks and image structures |
| src/app/pages/ArticlePage/PagePromoSections/TopStoriesSection/TopStoriesItem/index.tsx | Removes ts-expect-error now that types are properly defined |
| src/app/pages/ArticlePage/ContinueReadingButton/index.tsx | Accepts and includes experiment props in event tracking data |
| src/app/pages/ArticlePage/ArticlePage.tsx | Implements referrer experiment setup, component duplication, and experiment prop distribution |
| src/app/pages/ArticlePage/ArticlePage.styles.ts | Adds CSS styles to hide/show duplicate components based on viewport width |
| src/app/models/types/global.ts | Adds referrer field to ComponentExperimentProps type |
| src/app/legacy/containers/PageHandlers/withOptimizelyProvider/index.tsx | Exports getReferrer function for use in other components |
| src/app/contexts/RequestContext/index.tsx | Formatting change (blank line) |
| src/app/components/TopBarOJs/Promo/index.tsx | Removes ts-expect-error now that types are properly defined |
| src/app/components/RelatedContentSection/index.tsx | Accepts and includes experiment props in event tracking data |
| src/app/components/Recommendations/index.tsx | Implements adaptive content selection based on referrer variant and adds granular tracking |
| src/app/components/Recommendations/index.test.tsx | Adds comprehensive tests for all referrer variants and data sources |
| src/app/components/Recommendations/helpers/index.tsx | Implements data extraction and mapping functions for different content sources |
| src/app/components/Recommendations/fixtures.ts | Adds fixture data for testing different content sources |
| src/app/components/Recommendations/RecommendationsItem/index.tsx | Accepts event tracking data prop instead of using hardcoded value |
| src/app/components/MostRead/index.tsx | Accepts experiment props and merges them into event tracking data |
| articleBlocks: OptimoBlock[]; | ||
| grey2: string; | ||
| // eslint-disable-next-line @typescript-eslint/no-explicit-any | ||
| pageStyles: Record<string, any>; |
There was a problem hiding this comment.
The any type should be avoided. Consider using a more specific type for pageStyles, such as importing the styles object type or defining an interface that matches the expected structure.
| }; | ||
| images?: { | ||
| defaultPromoImage?: { | ||
| blocks?: any[]; |
There was a problem hiding this comment.
The any type should be avoided. Consider defining a specific type for the blocks array structure based on the OptimoBlock type or the actual structure used.
| // --- Shared utilities for extracting image data from defaultPromoImage --- | ||
|
|
||
| const getAltTextFromDefaultPromoImage = (defaultPromoImage?: { | ||
| blocks?: any[]; |
There was a problem hiding this comment.
The any type should be avoided. Consider defining a specific type for the blocks array parameter based on the structure being accessed.
| ); | ||
| }; | ||
|
|
||
| const getRawImageBlock = (defaultPromoImage?: { blocks?: any[] }) => |
There was a problem hiding this comment.
The any type should be avoided. Consider defining a specific type for the blocks array based on the expected image block structure.
| return slice(0, 4, storyPromoItems); | ||
| }; | ||
|
|
||
| export const getHeadlineFromOptimoBlock = (block: any) => { |
There was a problem hiding this comment.
The any type should be avoided. Consider using the OptimoBlock type or a more specific type for the block parameter.
| return assetUriFirst || assetUriSecond; | ||
| }; | ||
|
|
||
| export const getAltTextFromOptimoBlock = (block: any) => |
There was a problem hiding this comment.
The any type should be avoided. Consider using the OptimoBlock type or a more specific type for the block parameter.
| export const getImageFromOptimoBlock = (block: any) => { | ||
| const imageBlock = block?.model?.blocks?.find((b: any) => b.type === 'image'); | ||
| const rawImageBlock = imageBlock?.model?.blocks?.find( | ||
| (b: any) => b.type === 'rawImage', |
There was a problem hiding this comment.
Multiple uses of any type in this function. Consider using the OptimoBlock type or defining specific types for imageBlock and rawImageBlock.
| }; | ||
| }; | ||
|
|
||
| export const mapOptimoBlockToRecommendation = (block: any): Recommendation => ({ |
There was a problem hiding this comment.
The any type should be avoided. Consider using the OptimoBlock type or a more specific type for the block parameter.
| const { indexImage } = item as any; | ||
|
|
There was a problem hiding this comment.
The as any type assertion should be avoided. Consider extending the TopStoryItem type to include the indexImage property if it's a valid field.
| const { indexImage } = item as any; | |
| type IndexImage = { | |
| href?: string; | |
| altText?: string; | |
| width?: number; | |
| height?: number; | |
| copyrightHolder?: string; | |
| originCode?: string; | |
| }; | |
| type TopStoryItemWithOptionalIndexImage = TopStoryItem & { | |
| indexImage?: IndexImage; | |
| }; | |
| const { indexImage } = item as TopStoryItemWithOptionalIndexImage; |
| Boolean, | ||
| ); | ||
| } | ||
| // Default for when referrerVariant is 'control', 'off', '', or any unknown value and when referrer_variant is 'adaptive_variant' but referrer is 'search'' |
There was a problem hiding this comment.
There is an extra closing quote in the comment. The word "search" has two single quotes at the end instead of one.
| // Default for when referrerVariant is 'control', 'off', '', or any unknown value and when referrer_variant is 'adaptive_variant' but referrer is 'search'' | |
| // Default for when referrerVariant is 'control', 'off', '', or any unknown value and when referrer_variant is 'adaptive_variant' but referrer is 'search' |
5 participants
https://bbc.atlassian.net/browse/WS-1097
This pull request refactors the
Recommendationscomponent to support multiple types of recommendation sources (Most Read, Top Stories, Features, Related Content) based on the user's referrer variant, and introduces new helper functions for extracting and mapping data from various content block formats. It also updates tests to cover these new variants and improves event tracking granularity.It also duplicates the top stories and features components so that they are both inside the secondaryColumn, where they will render on desktop, and outside the secondaryColumn into a reordering function where they can have their order swapped around based on the referrerVariant. There are tests for this function.
display:none is used in order to hide the duplicated under-the-article components when the user is viewing at desktop width, and hiding the secondary column components when on mobile width. When we move this into production, we can decide whether we want to do this in a different way with a media query that decided whether the components render at all, instead of just hiding them with CSS.
Refactoring and Feature Expansion:
Recommendationscomponent now accepts new props (blocks,topStoriesContent,featuresContent,referrerVariant, andexperimentProps) and dynamically selects the data source and section title based on thereferrerVariant, supporting adaptive experiments for search, direct, and social referrers. [1] [2]helpers/index.tsxfor extracting and mapping recommendation data from Optimo blocks, Features, and Top Stories, ensuring consistent structure and alt text extraction for images.Testing Improvements:
index.test.tsxto cover rendering of recommendations for all referrer variants, including checks for correct titles, links, images, and alt text. Also added tests for single-item scenarios and rerendering with different variants.Event Tracking Enhancements:
groupTrackerand per-itemitemTrackerfor more granular analytics, with experiment props merged in if present. This is passed down to eachRecommendationsItem.Summary
A very high-level summary of easily-reproducible changes that can be understood by non-devs, and why these changes where made.
Code changes
Developer Checklist
Testing
Ready-For-Test, Local)Ready-For-Test, Test)Ready-For-Test, Preview)Ready-For-Test, Live)Additional Testing Steps
Useful Links