From f9f6325319bef0e0b717e9bd1d238cb6a97b6c83 Mon Sep 17 00:00:00 2001 From: George B <705427+georgeblahblah@users.noreply.github.com> Date: Thu, 28 Sep 2023 11:16:31 +0100 Subject: [PATCH] Deprecate Article Share Counts Part of https://github.com/guardian/frontend/issues/26568. This feature is broken, and the feature switch has been off for some time. --- applications/app/AppLoader.scala | 2 - .../controllers/ApplicationsControllers.scala | 3 - .../controllers/ShareCountController.scala | 27 -------- .../app/services/FacebookGraphApi.scala | 63 ------------------- applications/conf/routes | 3 - .../helpers/FacebookGraphApiTestClient.scala | 28 --------- applications/test/package.scala | 3 +- .../test/services/FacebookGraphApiTest.scala | 31 --------- dev-build/conf/routes | 1 - preview/conf/routes | 2 - 10 files changed, 1 insertion(+), 162 deletions(-) delete mode 100644 applications/app/controllers/ShareCountController.scala delete mode 100644 applications/app/services/FacebookGraphApi.scala delete mode 100644 applications/test/helpers/FacebookGraphApiTestClient.scala delete mode 100644 applications/test/services/FacebookGraphApiTest.scala diff --git a/applications/app/AppLoader.scala b/applications/app/AppLoader.scala index d83fe2d301b7..db26682ab64b 100644 --- a/applications/app/AppLoader.scala +++ b/applications/app/AppLoader.scala @@ -40,8 +40,6 @@ trait ApplicationsServices { lazy val siteMapJob = wire[SiteMapJob] lazy val sectionsLookUp = wire[SectionsLookUp] lazy val ophanApi = wire[OphanApi] - lazy val facebookGraphApiClient = wire[FacebookGraphApiClient] - lazy val facebookGraphApi = wire[FacebookGraphApi] lazy val newsletterApi = wire[NewsletterApi] lazy val newsletterSignupAgent = wire[NewsletterSignupAgent] } diff --git a/applications/app/controllers/ApplicationsControllers.scala b/applications/app/controllers/ApplicationsControllers.scala index 36079b218192..7a13f88e5fa8 100644 --- a/applications/app/controllers/ApplicationsControllers.scala +++ b/applications/app/controllers/ApplicationsControllers.scala @@ -6,7 +6,6 @@ import jobs.SiteMapJob import model.ApplicationContext import play.api.libs.ws.WSClient import play.api.mvc.ControllerComponents -import services.FacebookGraphApi trait ApplicationsControllers { @@ -14,7 +13,6 @@ trait ApplicationsControllers { def siteMapJob: SiteMapJob def sectionsLookUp: SectionsLookUp def wsClient: WSClient - def facebookGraphApi: FacebookGraphApi def controllerComponents: ControllerComponents implicit def appContext: ApplicationContext @@ -38,7 +36,6 @@ trait ApplicationsControllers { lazy val shortUrlsController = wire[ShortUrlsController] lazy val indexController = wire[IndexController] lazy val siteVerificationController = wire[SiteVerificationController] - lazy val shareCountController = wire[ShareCountController] lazy val youtubeController = wire[YoutubeController] lazy val nx1ConfigController = wire[Nx1ConfigController] diff --git a/applications/app/controllers/ShareCountController.scala b/applications/app/controllers/ShareCountController.scala deleted file mode 100644 index 2a939a8eea29..000000000000 --- a/applications/app/controllers/ShareCountController.scala +++ /dev/null @@ -1,27 +0,0 @@ -package controllers - -import common.{ImplicitControllerExecutionContext, JsonComponent, GuLogging} -import model.{ApplicationContext, CacheTime, Cached} -import play.api.mvc.{Action, AnyContent, BaseController, ControllerComponents} -import services.FacebookGraphApi - -class ShareCountController( - facebookGraphAPI: FacebookGraphApi, - val controllerComponents: ControllerComponents, -)(implicit context: ApplicationContext) - extends BaseController - with GuLogging - with ImplicitControllerExecutionContext { - - def fetch(path: String): Action[AnyContent] = - Action.async { implicit request => - facebookGraphAPI.shareCount(path).map { shareCount => - Cached(CacheTime.ShareCount) { - JsonComponent( - ("path", path), - ("share_count", shareCount), - ) - } - } - } -} diff --git a/applications/app/services/FacebookGraphApi.scala b/applications/app/services/FacebookGraphApi.scala deleted file mode 100644 index d40dcdd3ed1b..000000000000 --- a/applications/app/services/FacebookGraphApi.scala +++ /dev/null @@ -1,63 +0,0 @@ -package services - -import java.util.concurrent.TimeoutException - -import common.GuLogging -import conf.Configuration -import play.api.libs.json.Json -import play.api.libs.ws.{WSClient, WSResponse} - -import scala.concurrent.{ExecutionContext, Future} -import scala.concurrent.duration.{Duration, DurationInt} - -object URLResponseDeserializer { - implicit val jsonEngagement = Json.reads[Engagement] - implicit val jsonResponse = Json.reads[URLResponse] -} - -case class Engagement(share_count: Int) -case class URLResponse(id: String, engagement: Engagement) - -class FacebookGraphApiClient(wsClient: WSClient) extends implicits.WSRequests with GuLogging { - val apiRootUrl = s"https://graph.facebook.com/v${Configuration.facebook.graphApi.version}" - - def GET(endpoint: Option[String], timeout: Duration, queryString: (String, String)*)(implicit - executionContext: ExecutionContext, - ): Future[WSResponse] = { - val url = apiRootUrl + endpoint.getOrElse("") - val res = wsClient - .url(url) - .withQueryStringParameters(addAccessToken(queryString): _*) - .withRequestTimeout(timeout) - .getOKResponse() - res.failed.foreach { - case t: TimeoutException => log.warn(s"Timeout when fetching Facebook Graph API: $url", t) - case t => log.error(s"Failed to fetch from Facebook Graph API: $url", t) - } - res - } - - protected def makeUrl(endpoint: Option[String]): String = apiRootUrl + endpoint.getOrElse("") - private def addAccessToken(queryString: Seq[(String, String)]): Seq[(String, String)] = - ("access_token", Configuration.facebook.graphApi.accessToken) +: queryString -} - -class FacebookGraphApi(facebookGraphApiClient: FacebookGraphApiClient) { - - def shareCount(path: String)(implicit executionContext: ExecutionContext): Future[Int] = { - import URLResponseDeserializer._ - - facebookGraphApiClient.GET( - endpoint = None, - timeout = 1.second, - // This has to be http so long as the og:url is (or the API changes again) - queryString = ("id", s"http://www.theguardian.com/$path"), - ("fields", "engagement"), - ) map { response => - response.json - .asOpt[URLResponse] - .map(_.engagement.share_count) - .getOrElse(0) - } - } -} diff --git a/applications/conf/routes b/applications/conf/routes index 093efa853241..b9e9d7638c9f 100644 --- a/applications/conf/routes +++ b/applications/conf/routes @@ -56,9 +56,6 @@ GET /embed/atom/:atomType/:id/nojs/scroll-y POST /story-questions/answers/signup controllers.AtomPageController.signup() OPTIONS /story-questions/answers/signup controllers.AtomPageController.options() -# Share counts -GET /sharecount/*path.json controllers.ShareCountController.fetch(path) - # Preferences GET /preferences controllers.PreferencesController.indexPrefs() diff --git a/applications/test/helpers/FacebookGraphApiTestClient.scala b/applications/test/helpers/FacebookGraphApiTestClient.scala deleted file mode 100644 index 100ba3599f71..000000000000 --- a/applications/test/helpers/FacebookGraphApiTestClient.scala +++ /dev/null @@ -1,28 +0,0 @@ -package helpers - -import java.io.File -import java.net.URLEncoder - -import play.api.libs.ws.{WSClient, WSResponse} -import recorder.DefaultHttpRecorder -import services.FacebookGraphApiClient - -import scala.concurrent.{ExecutionContext, Future} -import scala.concurrent.duration.Duration - -class FacebookGraphApiTestClient(wsClient: WSClient) extends FacebookGraphApiClient(wsClient) { - - val recorder = new DefaultHttpRecorder { - override lazy val baseDir = new File(System.getProperty("user.dir"), "data/facebook-graph-api") - } - - override def GET(endpoint: Option[String], timeout: Duration, params: (String, String)*)(implicit - executionContext: ExecutionContext, - ): Future[WSResponse] = { - val queryString = params.map({ case (key, value) => key + "=" + URLEncoder.encode(value, "UTF-8") }).mkString("&") - - recorder.load(s"${makeUrl(endpoint)}?$queryString", Map.empty) { - super.GET(endpoint, timeout, params: _*) - } - } -} diff --git a/applications/test/package.scala b/applications/test/package.scala index c70e71d37ce6..05ae91bbee48 100644 --- a/applications/test/package.scala +++ b/applications/test/package.scala @@ -2,7 +2,7 @@ package test import java.util.{List => JList} import org.scalatest.Suites -import services.{FacebookGraphApiTest, IndexPageTest, InteractivePickerTest, NewspaperControllerTest} +import services.{IndexPageTest, InteractivePickerTest, NewspaperControllerTest} import scala.jdk.CollectionConverters._ @@ -40,7 +40,6 @@ class ApplicationsTestSuite new CrosswordDataTest, new NewspaperControllerTest, new IndexPageTest, - new FacebookGraphApiTest, new InteractivePickerTest, ) with SingleServerSuite {} diff --git a/applications/test/services/FacebookGraphApiTest.scala b/applications/test/services/FacebookGraphApiTest.scala deleted file mode 100644 index 02b705250c43..000000000000 --- a/applications/test/services/FacebookGraphApiTest.scala +++ /dev/null @@ -1,31 +0,0 @@ -package services - -import helpers.FacebookGraphApiTestClient -import org.scalatest.concurrent.ScalaFutures -import org.scalatest.flatspec.AnyFlatSpec -import org.scalatest.matchers.should.Matchers -import org.scalatest.{BeforeAndAfterAll, DoNotDiscover} -import test.{ConfiguredTestSuite, WithMaterializer, WithTestExecutionContext, WithTestWsClient} - -@DoNotDiscover class FacebookGraphApiTest - extends AnyFlatSpec - with Matchers - with ConfiguredTestSuite - with BeforeAndAfterAll - with WithMaterializer - with WithTestWsClient - with WithTestExecutionContext - with ScalaFutures { - - lazy val facebookGraphApiClient = new FacebookGraphApiTestClient(wsClient) - lazy val facebookGraphApi = new FacebookGraphApi(facebookGraphApiClient) - - it should "return a valid share count" in { - val shareCountRequest = - facebookGraphApi.shareCount("world/2016/dec/30/eight-charts-that-show-2016-wasnt-as-bad-as-you-think") - - whenReady(shareCountRequest) { result => - result should be > 0 - } - } -} diff --git a/dev-build/conf/routes b/dev-build/conf/routes index b0fd0b882690..8e1e323eeff9 100644 --- a/dev-build/conf/routes +++ b/dev-build/conf/routes @@ -339,7 +339,6 @@ GET /opt/reset GET /sitemaps/news.xml controllers.SiteMapController.renderNewsSiteMap() GET /sitemaps/video.xml controllers.SiteMapController.renderVideoSiteMap() GET /$path<[\w\d-]*(/[\w\d-]*)+>/$file controllers.InteractiveController.proxyInteractiveWebWorker(path, file) -GET /sharecount/*path.json controllers.ShareCountController.fetch(path) GET /preferences controllers.PreferencesController.indexPrefs() GET /embed/video/*path controllers.EmbedController.render(path) diff --git a/preview/conf/routes b/preview/conf/routes index caae49a99bd0..3a53595dcfc0 100644 --- a/preview/conf/routes +++ b/preview/conf/routes @@ -171,8 +171,6 @@ GET /$path<[\w\d-]*(/[\w\d-]*)?/(cartoon|graphic|picture)/.*>.json GET /$path<[\w\d-]*(/[\w\d-]*)?/(interactive|ng-interactive)/.*>.json controllers.InteractiveController.renderInteractiveJson(path) -GET /sharecount/*path.json controllers.ShareCountController.fetch(path) - # Email paths GET /email/form/$emailType/$listId<[0-9]+> controllers.EmailSignupController.renderForm(emailType: String, listId: Int) GET /email/form/$emailType/:listName controllers.EmailSignupController.renderFormFromName(emailType: String, listName: String)