-
Notifications
You must be signed in to change notification settings - Fork 279
WS-2995 - Support variants needed for search experiment #14263
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
louisearchibald
merged 71 commits into
latest
from
WS-2995-support-variants-needed-for-search-experiment
Jul 31, 2026
Merged
Changes from all commits
Commits
Show all changes
71 commits
Select commit
Hold shift + click to select a range
71c37cf
Merge branch 'latest' of ssh://github.com/bbc/simorgh into latest
louisearchibald 115ee9a
Merge branch 'latest' of ssh://github.com/bbc/simorgh into latest
louisearchibald 4471b23
Merge branch 'latest' of ssh://github.com/bbc/simorgh into latest
louisearchibald 2561fc5
Merge branch 'latest' of ssh://github.com/bbc/simorgh into latest
louisearchibald 7f33e83
Merge branch 'latest' of ssh://github.com/bbc/simorgh into latest
louisearchibald fe8ac68
Merge branch 'latest' of ssh://github.com/bbc/simorgh into latest
louisearchibald 2c42c87
Merge branch 'latest' of ssh://github.com/bbc/simorgh into latest
louisearchibald 5ad5e36
Merge branch 'latest' of ssh://github.com/bbc/simorgh into latest
louisearchibald 89f9d4e
Merge branch 'latest' of ssh://github.com/bbc/simorgh into latest
louisearchibald e7d96e5
Merge branch 'latest' of ssh://github.com/bbc/simorgh into latest
louisearchibald 5b39a68
Merge branch 'latest' of ssh://github.com/bbc/simorgh into latest
louisearchibald 1f68fa1
create onward journey component key type
louisearchibald 8c348f7
set up variants with decided oj component ordering
louisearchibald 4a92418
Merge branch 'latest' into WS-2995-support-variants-needed-for-search…
louisearchibald bef8dae
comments
louisearchibald fb5b4c3
Merge branch 'WS-2995-support-variants-needed-for-search-experiment' …
louisearchibald 8ca74b9
make variants params to help with testing
louisearchibald addd268
return variant component order if user is on mobile
louisearchibald c6603e1
Merge branch 'latest' into WS-2995-support-variants-needed-for-search…
louisearchibald c08235b
adds oj reordering logic
holchris efa2edf
adds mobile oj container css
holchris dabfefd
Merge branch 'WS-2995-support-variants-needed-for-search-experiment' …
louisearchibald d676ea0
add switch case for showing mid article ojs for each variant
louisearchibald cd9dfef
add missing brackets to enable reordering to work
louisearchibald 9ebb625
amend switch for variants 1 to 3 as they all return the same component
louisearchibald 29ba715
remove console log
louisearchibald ea7022f
update mobile OJ order to include searchVariant
louisearchibald 99429fe
small change to get switch case working for midarticle OJs
louisearchibald f5874fc
fixes midarticle OJ changing at desktop
louisearchibald de639fa
Merge branch 'latest' into WS-2995-support-variants-needed-for-search…
pvaliani 324577d
hide related topics when topic discovery is in midarticle position
louisearchibald 1702064
Merge branch 'WS-2995-support-variants-needed-for-search-experiment' …
louisearchibald 159eba1
update variant names
holchris 18c506f
update variant names in article page
holchris e8bfe8e
update search variant params
holchris 11c01d8
Merge branch 'WS-2995-support-variants-needed-for-search-experiment' …
louisearchibald bdff19c
Merge branch 'latest' into WS-2995-support-variants-needed-for-search…
louisearchibald 1c19189
adds marginBottom to topicDiscovery when in the midarticle position
louisearchibald 0afc61c
Merge branch 'WS-2995-support-variants-needed-for-search-experiment' …
louisearchibald 21ede71
Merge branch 'latest' into WS-2995-support-variants-needed-for-search…
louisearchibald c266855
Merge branch 'WS-2995-support-variants-needed-for-search-experiment' …
louisearchibald d663bdd
adds 40px marginBottom for location oj also
louisearchibald 57e4441
remove topicDiscovery from variant 5
louisearchibald 29d0668
remove comment
louisearchibald 173fb7e
reuse variant config for debug variant validation
louisearchibald c2a1012
remove redundant styling
louisearchibald 3a7b4b1
revert accidental change
louisearchibald ff416d0
Merge branch 'latest' into WS-2995-support-variants-needed-for-search…
louisearchibald d0d0eb7
Merge branch 'latest' into WS-2995-support-variants-needed-for-search…
Nabeel1276 e576f0c
adds tests for search referrer
holchris 8bc155a
Merge branch 'WS-2995-support-variants-needed-for-search-experiment' …
louisearchibald 501f64b
remove commented code and use media query for the test instead of har…
louisearchibald 53eaa17
use searchVariant type
louisearchibald f7f3ad6
refactor switch case for midarticleOJ
louisearchibald 5de937c
test midarticle OJ to variant mapping
louisearchibald 0aaea8d
Merge branch 'latest' into WS-2995-support-variants-needed-for-search…
louisearchibald 0e45de8
Merge branch 'WS-2995-support-variants-needed-for-search-experiment' …
louisearchibald c7814e9
refactor condition and remove accidental brackets
louisearchibald 1cd3e69
Merge branch 'latest' into WS-2995-support-variants-needed-for-search…
pvaliani ebcac79
fallback to most read midarticle oj when location based oj is not pre…
louisearchibald 94b60dd
Merge branch 'WS-2995-support-variants-needed-for-search-experiment' …
louisearchibald 62a816c
update types to be videoOJ to cater for video curations also
louisearchibald 42abb84
add video oj helper to cater for both pvCarousel and video curations
louisearchibald 1bed8dc
rendered mobile variant tests wip copilot
holchris 05de34c
fallback for related content
louisearchibald 2980b75
Merge branch 'WS-2995-support-variants-needed-for-search-experiment' …
louisearchibald 44c9775
temporarily skip wip tests just added
louisearchibald ca4e4fc
update OJ name
louisearchibald 3ce4988
test change
louisearchibald 02c8712
update describe
louisearchibald fbb791a
Merge branch 'latest' into WS-2995-support-variants-needed-for-search…
louisearchibald File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
Oops, something went wrong.
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The footer versions use the existing checks for whether Topic Discovery and Location OJ (like
showTopicDiscoveryandshowCountryCuration) are enabled, but the mid-article versions bypass those checks I think. This might display a component that is disabled for a service, or leave the mid-article position empty when it is unavailable. If the response is empty it might handle gracefully but thought it might be worth mentioningThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The country curation not being on a service could be a problem here for variant 6. I will ask Gavin.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Answer is to fallback to most read in the mid-article spot (same as control) when the variant defined component is unavailable. This will happen with Location OJ and Related Content.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I take it Topic Discovery is guaranteed to be available for every article in scope, as this mid-article path does not use
showTopicDiscovery? If it is guaranteed then this should be fine otherwise it may need the same fallback so was jwThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have not encountered any articles where it isn't present, however that's not to say that it couldn't happen? The drawback to having the same fallback is the ordering of that particular variant as you would then have mostRead content showing in the Recommendations component in the mid-article slot and then directly followed by mostRead as the first OJ underneath. I can query that tomorrow with product.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think topic discovery is on every service. Let's try not to let it be toggled off for the duration of the experiment! (not sure why it would be. Getting the dedupe fix in as well!)