-
Notifications
You must be signed in to change notification settings - Fork 30
add force particiaptions to AB test participations #14880
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
base: main
Are you sure you want to change the base?
Conversation
|
Hello 👋! When you're ready to run Chromatic, please apply the You will need to reapply the label each time you want to run Chromatic. |
f2a97cf to
48f1822
Compare
48f1822 to
73df669
Compare
|
This will work for client side code but not server side code - may lead to confusion? hash fragments aren't visible to the server so would need a different way to opt in |
934fc7c to
3051ace
Compare
a8f9528 to
c1854cf
Compare
216d6e9 to
22dfe45
Compare
22dfe45 to
2e573bc
Compare
2e573bc to
87633f9
Compare
|
Could we add to the readme's with how to opt in locally? |
| }; | ||
| req.body = frontendData; | ||
| } catch (error) { | ||
| return next(); |
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.
We should log the error here, to help resolve issues if people don't format their params quite right!
|
|
||
| export const getABTestsFromQueryParams: Handler = async (req, res, next) => { | ||
| try { | ||
| const frontendData = validateAsFEArticle(req.body); |
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 don't think this will work for other page types, Fronts etc.
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.
This needs a re-review as this has changed since I first reviewed. See Jake's comments
What does this change?
Adds URL parameter support to the beta AB testing framework, allowing developers to force themselves into AB tests using
?ab-TestName=variantImplementation:
New Middleware (add-queryparams-to-abtests.ts)
Middleware runs after getContentFromURLMiddleware (which fetches article JSON)
Middleware runs before handleArticle (which renders the HTML)
Why?
The beta AB framework had no way to force test participation during development, making it difficult to test features gated behind AB tests locally.
Essentially the
getParticipationsfunction checks the participations object of the beta ab framework to see if the user is in the test. We are circumventing this by forcing the test name in to thewindow.guardian.config.serverSideABTestslocallyWe currently do this in the Legacy
ab-coreand would like to include this useful development feature in the beta ab testing onlocalhost:3030/✅ Force participation on localhost:3030/article#ab-MyTest=variant
✅ Override production assignments for debugging
✅ Multiple tests:
?ab-Test1=variant,?ab-Test2=control✅ Priority: URL > Server > Cookie
✅ No breaking changes to existing code
Screenshots
To Test I added console logs to see the participations:
using Localhost with
#ab-MyTest=control: http://localhost:3030/Article/https://www.theguardian.com/sport/2023/nov/22/ronnie-osullivan-warns-he-will-quit-snooker-if-he-cannot-play-in-china#ab-MyTest=control#ab-MyTest=control#ab-MyTest=variantdocument.cookie = "gu_client_ab_tests=TestA:control,TestB:variant; path=/";
#ab-TestA=variant,ab-TestB=variant