Skip to content
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

Render Showcase formatted feeds for pressed fronts #24038

Merged
merged 65 commits into from
Sep 8, 2021
Merged

Render Showcase formatted feeds for pressed fronts #24038

merged 65 commits into from
Sep 8, 2021

Conversation

tonytw1
Copy link
Contributor

@tonytw1 tonytw1 commented Jul 28, 2021

Render Showcase formatted feeds on a /showcase suffix for pressed fronts.

Showcase is an extension of RSS.
We use a Rome RSS module to render the new g and atom fields.

Adds a new lastModified optional field to PressedCards.

Fronts tool collections are used to populate Single Story and Rundown Panels.

Each Showcase feed will have it's own front in the showcase priority in the Fronts tool.
Only fronts which have the magic Standalone and Rundown containers will render is Showcase feeds.

Screenshot 2021-08-09 at 13 05 01

What does this change?

Does this change need to be reproduced in dotcom-rendering ?

  • No
  • Yes (please indicate your plans for DCR Implementation)

Screenshots

What is the value of this and can you measure success?

Can pass a live guardian showcase feed url to Showcase feed validator and have it pass.

Checklist

Does this affect other platforms?

  • AMP
  • Apps
  • Other (please specify)

Does this affect GLabs Paid Content Pages? Should it have support for Paid Content?

  • No
  • Yes (please give details)

Does this change break ad-free?

  • No
  • It did, but tests caught it and I fixed it
  • It did, but there was no test coverage so I added that then fixed it

Does this change update the version of CAPI we're using?

Accessibility test checklist

Tested

  • Locally
  • On CODE (optional)

@tonytw1 tonytw1 changed the title Work in progress. Create a Rome RSS module to allow us to render Show… Render Showcase formatted feeds Aug 9, 2021
@tonytw1 tonytw1 changed the title Render Showcase formatted feeds Render Showcase formatted feeds for pressed fronts Aug 13, 2021
@tonytw1 tonytw1 force-pushed the showcase-rss branch 2 times, most recently from eebab9b to f1465f4 Compare August 24, 2021 11:00
@tonytw1 tonytw1 force-pushed the showcase-rss branch 3 times, most recently from 8273256 to c57d6ae Compare September 1, 2021 15:01
Tony McCrae added 17 commits September 2, 2021 10:37
…case modules while still using Rome as the framework.
This is almost certainly not what we what but shows we can get through the frontend ingress.
…e which contains Collections and Cards from the Fronts editor.

Pivot to /showcase end point in facia.
Begin sourcing Showcase fields from the PressedContent model rather than Trails.
Provide a rough fixture for creating test PressedContent.
Unroll changes to TrailsRSS which are no longer of interest to us.
… best place to pickup panel images from a Card.
…e to populate Showcase update fields.

Last updated is a mandatory field in Showcase so we should be attempting to populate with good data.
…which cannot be made valid from the give content item.
Copy link
Member

@davidfurey davidfurey left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are quite a few TODOs, are you planning to resolve them in a separate PR?

@@ -0,0 +1,2 @@
rss_1.0.item.ModuleGenerator.classes=common.GModuleGenerator,common.RssAtomModuleGenerator
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this need to be added to dev-build and preview too?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oddly it does not appear to be needed; the classpath is probably leaking across into dev build and preview.

Preview might need to be revisited in a real environment but we well probably be doing something visual rather than rendering RSS in real preview. So not a blocker/

facia/app/controllers/FaciaController.scala Show resolved Hide resolved
rss.getNamespace("media") should be("http://search.yahoo.com/mrss/")
}

"TrailsToShowcase" can "render feed with Single Story and Rundown panels" in {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test seems to be covering a lot of things, would it make sense to break it up?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll ok with this; this test is acting an 'yeah but does it actually render' integration test and I'm fining it useful in this form.

common/test/common/TrailsToShowcaseTest.scala Outdated Show resolved Hide resolved
common/test/common/TrailsToShowcaseTest.scala Outdated Show resolved Hide resolved
@@ -35,6 +35,7 @@ GET /most-relevant-container/*path.json
GET /*path/show-more/*id.json controllers.FaciaController.renderShowMore(path, id)
GET /rss controllers.FaciaController.renderRootFrontRss()
GET /*path/rss controllers.FaciaController.renderFrontRss(path)
GET /*path/showcase controllers.FaciaController.renderFrontShowcase(path)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This might also need to be added to dev-build routes

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in preview and dev-build.

Tony McCrae added 9 commits September 3, 2021 12:59
…inclusion in the author tag rather than dc:creator

Showcase's usage of the author tag is slightly off spec here.
We will almost certainly end up trying to override this with some visual.
…operties.maybeContent.metadata.webUrl

Which is a full url rather than a URI.
@tonytw1 tonytw1 requested a review from davidfurey September 6, 2021 09:39
…ain objects.

Rather than Rome feed clases; sets up for preview of panels.
@tonytw1 tonytw1 merged commit 139874b into main Sep 8, 2021
@tonytw1 tonytw1 deleted the showcase-rss branch September 8, 2021 08:35
@prout-bot
Copy link
Collaborator

Seen on PROD (merged by @tonytw1 18 minutes and 31 seconds ago)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants