admiral-migration: add AdmiralScript component and AB test integration - #15355
Conversation
3d400e8 to
8396a15
Compare
|
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. |
8396a15 to
9e07d1e
Compare
| abTestTest, | ||
| userBenefitsApi, | ||
| noAuxiaSignInGate, | ||
| admiralAdblockRecovery, |
There was a problem hiding this comment.
I believe we should be using the new ab test config instead of this file.
@domlander is this a good example to follow?
There was a problem hiding this comment.
There's some documentation on the new AB testing framework here https://github.com/guardian/dotcom-rendering/blob/e8fc9932e1779489fd8c58e8b935dbb10fd2d259/dotcom-rendering/docs/development/ab-testing-in-dcr.md
You essentially just need to add the test definition to abTests.ts
There was a problem hiding this comment.
Having said that, I imagine you may want to use the legacy framework for now until DCR is fully handling this, since otherwise you will have two AB tests doing the same thing on different platforms which could get confusing!
Maybe converting to the new framework could be done as a secondary step after this work is done?
There was a problem hiding this comment.
Yes exactly, I chose the "legacy" framework in order to keep one AB test instead of duplicating the same logic in separate tests
| 'whitelisted' in e && | ||
| 'subscribed' in e; | ||
|
|
||
| if (isMeasureDetectedEvent(event)) { |
There was a problem hiding this comment.
You can unnest this if block by flipping the if/else and then removing the else by using early return.
There was a problem hiding this comment.
thanks, flipped
9e07d1e to
df2c83a
Compare
| const BASE_AJAX_URL = | ||
| window.guardian.config.stage === 'CODE' | ||
| ? 'https://code.api.nextgen.guardianapps.co.uk' | ||
| : 'https://api.nextgen.guardianapps.co.uk'; |
There was a problem hiding this comment.
This is also available on the window config:
guardian.config.page.ajaxUrl
There was a problem hiding this comment.
Neat, I hadn't spotted that before!
There was a problem hiding this comment.
thanks! Updated
- added check if Commercial is handling Admiral - updated logging to use 'dotcom' logger
17176da to
37a62b0
Compare
|
Seen on PROD (merged by @AnastasiiaBalenko 7 minutes and 54 seconds ago) Please check your changes! |
What does this change?
Ticket link
Why?
This PR moves Admiral Adblock Recovery initialization from the commercial bundle to dotcom-rendering (DCR), allowing DCR to own and control Admiral lifecycle. This gives DCR better control over the initialization sequence and event tracking.
Changes:
Notes
The commercial bundle's prepare-admiral.ts is bypassed when dcrOwnsAdmiral = true. It is a temporary measure before code is removed from Commercial
Screenshots
variant-detect => admiral-bootstrap script is loaded, but no modal is shown up for US user


variant-recover => admiral-bootstrap script is loaded, modal is shown up for US user