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

Overlay media image on Twitter with content age #21306

Merged
merged 3 commits into from Mar 29, 2019

Conversation

Projects
None yet
5 participants
@SiAdcock
Copy link
Contributor

SiAdcock commented Mar 29, 2019

What does this change?

Old articles may be shared on Twitter, which could give readers a
misleading picture of a news story.

This change overlays older articles with a PNG that displays the year
the article was published.

This change is limited to a single whitelisted article for the purposes of testing, before we roll out to all news articles. An old Tweet linking to this article can be found here, for testing whether old tweets are updated correctly.

It would be worth considering rolling this out beyond just news to liveblogs and more if it is successful.

Screenshots

Before

Screenshot 2019-03-29 at 15 14 18

After

Screenshot 2019-03-29 at 15 12 39

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

Gives more context to the reader and
allows them to disregard stories that are no longer relevant.

Checklist

Does this affect other platforms?

  • AMP
  • Apps
  • Other (please specify)

Only Twitter

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)
Overlay media image on Twitter with content age
Old articles may be shared on Twitter, which could give readers a
misleading picture of a news story.

This change overlays older articles with a PNG that displays the year
the article was published, giving more context to the reader and
allowing them to disregard stories that are no longer relevant.
@@ -172,6 +173,24 @@ object TwitterImage extends OverlayBase64 {
}
new ShareImage(image, TwitterShareImageLogoOverlay.isSwitchedOn)
}
def getContentAgeFileName(prefix: String, publicationYear: Int): String = {
// WARNING: we have only produced these content age images up to the year 2025

This comment has been minimized.

Copy link
@SiAdcock

SiAdcock Mar 29, 2019

Author Contributor

Greatest caveat here, and probably the dumbest. We have only created images covering publication dates from 1999 to 2025. If you're reading this message in 2026, you will need to create more, or find a programmatic way of applying the overlay.

@PRBuilds

This comment has been minimized.

Copy link

PRBuilds commented Mar 29, 2019

PRbuilds results:

Screenshots
wide.pngdesktop.pngtablet.pngmobile.png

🔥 (failed) Exceptions
thrown-exceptions.js

💚 A11y validation
a11y-report.txt

💚 Microdata Validation
microdata.txt

Apache Benchmark Load Testing
loadtesting.txt

LightHouse Reporting
1553873232.report.html

--automated message

@@ -157,6 +158,13 @@ final case class Content(
else if(isFromTheObserver && tags.isComment) TwitterImage.opinionsObserver
else if(tags.isComment) TwitterImage.opinions
else if(tags.isLiveBlog) TwitterImage.live
else if(tags.tags.exists(_.id == "tone/news") && trail.webPublicationDate.getYear < DateTime.now().getYear()) {
if(isFromTheObserver) {

This comment has been minimized.

Copy link
@MatthewJWalls

MatthewJWalls Mar 29, 2019

Contributor

Out of interest, what is the behaviour if we think we should have an overlay image, but for whatever reason it is missing. For example somebody decides the s3 bucket should have a retention policy of 30 days and all of the overlay images get deleted out by accident. Will this code 500? Or fallback gracefully.

This comment has been minimized.

Copy link
@SiAdcock

SiAdcock Mar 29, 2019

Author Contributor

The fallback will be no image overlay at all. But it won't blow up or anything

This comment has been minimized.

Copy link
@MatthewJWalls

MatthewJWalls Mar 29, 2019

Contributor

Cool that's fine imo

@AWare

AWare approved these changes Mar 29, 2019

Copy link
Contributor

AWare left a comment

We probably should refactor that whole thing and make more generic somehow, but this is 💯 for the use case.

@SiAdcock SiAdcock force-pushed the sa--old-article-overlay branch from e81d038 to 7346b2c Mar 29, 2019

@SiAdcock SiAdcock merged commit ec8798a into master Mar 29, 2019

15 checks passed

continuous-integration/teamcity Finished TeamCity Build dotcom :: frontend : Tests passed: 2072, ignored: 2
Details
license/snyk - admin/pom.xml (guardian-frontend) No new issues
Details
license/snyk - build.sbt (guardian-frontend) No manifest changes detected
license/snyk - dev/eslint-plugin-guardian-frontend/package.json (guardian-frontend) No manifest changes detected
license/snyk - identity/build.sbt (guardian-frontend) No manifest changes detected
license/snyk - package.json (guardian-frontend) No manifest changes detected
license/snyk - sport/pom.xml (guardian-frontend) No new issues
Details
license/snyk - tools/amp-validation/package.json (guardian-frontend) No manifest changes detected
security/snyk - admin/pom.xml (guardian-frontend) No new issues
Details
security/snyk - build.sbt (guardian-frontend) No manifest changes detected
security/snyk - dev/eslint-plugin-guardian-frontend/package.json (guardian-frontend) No manifest changes detected
security/snyk - identity/build.sbt (guardian-frontend) No manifest changes detected
security/snyk - package.json (guardian-frontend) No manifest changes detected
security/snyk - sport/pom.xml (guardian-frontend) No new issues
Details
security/snyk - tools/amp-validation/package.json (guardian-frontend) No manifest changes detected
@prout-bot

This comment has been minimized.

Copy link
Collaborator

prout-bot commented Mar 29, 2019

Seen on PROD (merged by @SiAdcock 17 minutes and 26 seconds ago)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.