-
Notifications
You must be signed in to change notification settings - Fork 566
Add tag page experiment and pull storylines content #28463
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
565483a to
5ab720c
Compare
fredex42
left a comment
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.
LGTM, put in a couple of suggestions for alternate names for the expieriment
| participationGroup = Perc0D, | ||
| ) | ||
|
|
||
| object AITagPageContent |
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.
AIStorylines? TagPageAIStorylines ?
b58a392 to
fa73ec3
Compare
| // These mirror the definitions from DCR: https://github.com/guardian/dotcom-rendering/blob/main/dotcom-rendering/src/types/mainMedia.ts | ||
| // In DCR, we effectively bypass the usual transformations done to Frontend/CAPI data, | ||
| // but need to provide the fields below so multimedia cards can render correctly | ||
| sealed trait TPSGMediaData { |
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.
TPSG ?
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.
Ah yeah, that's a reference to the Tag Page Supercharger tool - wanted to distinguish it from the IIRC existing media data class. Can rename that to storyline though to keep it more consistent 👍
| implicit val mediaDataDecoder: Decoder[TPSGMediaData] = | ||
| Decoder.instance { cursor => | ||
| cursor.get[String]("type").flatMap { | ||
| case "YoutubeVideo" => cursor.as[TPSGVideo] |
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.
Now that we have self-hosted video and vertical formats, will the storylines be supporting any of these?
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.
That's a good question - I think so far the video data has only been youtube ones (i.e. the only video articles getting displayed are ones with youtube videos), but I wouldn't rule it out.
Technically, the type doesn't really matter - the videos aren't meant to be playable where this is getting displayed, mainly we just want the media pill to be displaying correctly. But would definitely be best to make sure it's the correct type. That might be ok for a follow up PR though, I'll probably need to adapt some stuff in the tool for this as well.
What does this change?
Part of the AI tag page project - the tool uploads json content for the AI curated storylines to s3 with the required data for DCR to be able to render it.
On discussion with the WebEx and F&C teams, we agreed that frontend would be pulling this and passing it to DCR. So this PR implements that - we define the relevant classes and objects to be able to provide the StorylinesContent to the DotcomTagPagesRenderingModel.
This will be displayed during an AB test in January, so for the purposes of testing and development, we just set up a 0% experiment (ensuring this logic doesn’t get called unless opted in) without restricting which pages this can appear on. the plan is to just run them on two pages (though possibly slightly more, so still a bit tbd); a subsequent PR will add a check to make sure we’re only displaying this on the desired pages.
The relevant DCR PR is here.
Testing
Can best be tested with the DCR PR and checking the content appears on the desired page.
Checklist
data/databasefiles generated by tests are committed with this PR (the tests will fail in CI if you've forgotten to do this)