frontend icon indicating copy to clipboard operation
frontend copied to clipboard

On-demand DCR rendering for `popular-in-tag` Onwards

Open bryophyta opened this issue 2 years ago • 0 comments

Note PR description is in progress.

Co-authored by: Ashleigh Carr [email protected] Co-authored by: Ioanna Kokkini [email protected] Co-authored by: James Gorrie [email protected]

What does this change?

Adds a new endpoint, based on the existing popular-in-tag endpoint, which returns a DCR-rendered Onward carousel for a given tag.

This is envisaged as a relatively quick fix for part of server-rendering onwards content while we resolve the questions around storing popular-in-tag content in memory. The reason for using a quick-fix approach here is because server-rendering onwards content is blocking our ability to test DCR-rendered Fronts (which depends on an update to how we render images).

This branch is not complete:

  • [ ] Some solution needs to be found for passing the article format from the client to Frontend (and on to DCR).
  • [ ] There is also some refactoring needed, if we use this branch, because it adds mandatory fields to an existing class (OnwardCollectionResponse).

I've noted these in inline comments.

This branch is relatively focused, but it does touch a few different files, so it may make sense to cherry-pick some commits.

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

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

There is a discrepancy between what Frontend is sending and what DCR is expecting, which is causing a bug in the DCR side Onwards/ endpoint. I think the change needs to be made on the DCR side. See this PR.

Screenshots

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

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)

bryophyta avatar Sep 06 '22 16:09 bryophyta