From a96d54ca77d80858e761d5546a9b8f8d8223f7b9 Mon Sep 17 00:00:00 2001 From: Charlotte Date: Wed, 27 Nov 2024 13:08:36 +0000 Subject: [PATCH] add test and improve readability --- common/test/helpers/FaciaTestData.scala | 78 +++++++++++++++++++++ facia/app/controllers/FaciaController.scala | 36 +++++----- facia/test/FaciaControllerTest.scala | 30 +++++++- 3 files changed, 125 insertions(+), 19 deletions(-) diff --git a/common/test/helpers/FaciaTestData.scala b/common/test/helpers/FaciaTestData.scala index cc0ee54002e..8229003a2e5 100644 --- a/common/test/helpers/FaciaTestData.scala +++ b/common/test/helpers/FaciaTestData.scala @@ -70,6 +70,22 @@ trait FaciaTestData extends ModelHelper { "/commentisfree/2013/oct/07/feminism-rebranding-man-hater", ) + val europeFrontTrailIds: Seq[String] = Seq( + "/world/2024/nov/27/plan-to-cut-berlin-arts-budget-will-destroy-citys-culture-directors-warn", + "/stage/2024/nov/27/afd-threats-to-german-democracy-on-stage-maximilian-steinbeis-a-citizen-of-the-people", + "/commentisfree/2024/nov/27/france-cannabis-consumption-europe-tax-decriminalisation-crime", + "/football/2024/nov/27/bodo-glimt-manchester-united-europa-league", + "/books/2024/nov/26/freedom-by-angela-merkel-review-settling-scores-with-silence", + ) + + val europeBetaFrontTrailIds: Seq[String] = Seq( + "/business/2024/nov/27/just-eat-to-delist-from-london-stock-exchange-to-cut-complexity-and-costs", + "/world/2024/nov/26/uk-labour-cabinet-ministers-sanctions-russia-storm-shadow-missiles", + "/business/2024/nov/27/fining-budget-airlines-will-make-flying-more-expensive-says-easyjet-boss", + "/world/2024/nov/26/irish-pm-simon-harris-slump-drops-points-polls-election-fine-gael", + "/commentisfree/2024/nov/25/the-guardian-view-on-romanias-presidential-election-a-stable-ukrainian-ally-wobblese", + ) + val cultureTrailIds: Seq[String] = Seq( "/film/2013/oct/08/gravity-science-astrophysicist", @@ -82,6 +98,8 @@ trait FaciaTestData extends ModelHelper { val ukFrontTrails: Seq[PressedContent] = ukFrontTrailIds map TestContent.newFaciaContent val usFrontTrails: Seq[PressedContent] = usFrontTrailIds map TestContent.newFaciaContent val auFrontTrails: Seq[PressedContent] = auFrontTrailIds map TestContent.newFaciaContent + val europeFrontTrails: Seq[PressedContent] = europeFrontTrailIds map TestContent.newFaciaContent + val europeBetaFrontTrails: Seq[PressedContent] = europeBetaFrontTrailIds map TestContent.newFaciaContent val cultureFrontTrails: Seq[PressedContent] = cultureTrailIds map TestContent.newFaciaContent @@ -172,6 +190,64 @@ trait FaciaTestData extends ModelHelper { ), ) + val europeFaciaPage: PressedPage = PressedPage( + id = "europe", + SeoData.fromPath("europe"), + FrontProperties.empty, + collections = List( + PressedCollection( + id = "europe/news/regular-stories", + displayName = "", + curated = europeFrontTrails.toList, + backfill = Nil, + treats = Nil, + lastUpdated = None, + href = None, + description = None, + collectionType = "", + groups = None, + uneditable = false, + showTags = false, + showSections = false, + hideKickers = false, + showDateHeader = false, + showLatestUpdate = false, + config = CollectionConfig.empty, + hasMore = false, + targetedTerritory = None, + ), + ), + ) + + val europeBetaFaciaPageWithTargetedTerritory: PressedPage = PressedPage( + id = "europe-beta", + SeoData.fromPath("europe-beta"), + FrontProperties.empty, + collections = List( + PressedCollection( + id = "europe-beta/news/regular-stories", + displayName = "", + curated = europeBetaFrontTrails.toList, + backfill = Nil, + treats = Nil, + lastUpdated = None, + href = None, + description = None, + collectionType = "", + groups = None, + uneditable = false, + showTags = false, + showSections = false, + hideKickers = false, + showDateHeader = false, + showLatestUpdate = false, + config = CollectionConfig.empty, + hasMore = false, + targetedTerritory = Some(EU27Territory), + ), + ), + ) + val ukCultureFaciaPage: PressedPage = PressedPage( id = "uk/culture", SeoData.fromPath("uk/culture"), @@ -334,6 +410,8 @@ trait FaciaTestData extends ModelHelper { ("uk", new TestPageFront("uk", Uk, ukFaciaPage)), ("us", new TestPageFront("us", Us, usFaciaPage)), ("au", new TestPageFront("au", Au, auFaciaPage)), + ("europe", new TestPageFront("europe", Au, europeFaciaPage)), + ("europe-beta", new TestPageFront("europe-beta", Au, europeBetaFaciaPageWithTargetedTerritory)), ("uk/culture", new TestPageFront("uk/culture", Uk, ukCultureFaciaPage)), ("us/culture", new TestPageFront("us/culture", Us, usCultureFaciaPage)), ("au/culture", new TestPageFront("au/culture", Au, auCultureFaciaPage)), diff --git a/facia/app/controllers/FaciaController.scala b/facia/app/controllers/FaciaController.scala index f185d99b9f5..4ffb55d8632 100644 --- a/facia/app/controllers/FaciaController.scala +++ b/facia/app/controllers/FaciaController.scala @@ -220,20 +220,20 @@ trait FaciaController ) private[controllers] def renderFrontPressResult(path: String)(implicit request: RequestHeader): Future[Result] = { - val fFaciaPage = getFaciaPage(path) + val futureFaciaPage = getFaciaPage(path) /** Europe Beta test: swaps the collections on the Europe network front with those on the hidden europe-beta front * for users participating in the test */ val futureFaciaPageWithEuropeBetaTest: Future[Option[(PressedPage, Boolean)]] = { if (path == "europe" && ActiveExperiments.isParticipating(EuropeBetaFront)) { - val fEuropeBetaPage = getFaciaPage("europe-beta") + val futureEuropeBetaPage = getFaciaPage("europe-beta") for { - europePage <- fFaciaPage - europeBetaPage <- fEuropeBetaPage - } yield swapFaciaPageCollections(europePage, europeBetaPage) + europePage <- futureFaciaPage + europeBetaPage <- futureEuropeBetaPage + } yield replaceFaciaPageCollections(europePage, europeBetaPage) } else { - fFaciaPage + futureFaciaPage } } @@ -330,24 +330,24 @@ trait FaciaController case None => Future.successful(None) } - /** Swaps collections on a facia page with those on a hidden facia page. Originally set up for the Europe beta test - * where we return europe-beta collections on the europe front if participating + /** Swaps collections on a given facia page with those on another facia page. Set up for the Europe beta test where we + * return europe-beta collections on the europe front if participating in the test */ - private def swapFaciaPageCollections( - faciaPage: Option[(PressedPage, Boolean)], - hiddenFaciaPage: Option[(PressedPage, Boolean)], + private[controllers] def replaceFaciaPageCollections( + baseFaciaPage: Option[(PressedPage, Boolean)], + replacementFaciaPage: Option[(PressedPage, Boolean)], ): Option[(PressedPage, Boolean)] = { for { - (pressedPage, _) <- faciaPage - (pressedHiddenPage, hasTargetedCollections) <- hiddenFaciaPage + (basePage, _) <- baseFaciaPage + (replacementPage, replacementTargetedTerritories) <- replacementFaciaPage } yield ( PressedPage( - id = pressedPage.id, - seoData = pressedPage.seoData, - frontProperties = pressedPage.frontProperties, - collections = pressedHiddenPage.collections, + id = basePage.id, + seoData = basePage.seoData, + frontProperties = basePage.frontProperties, + collections = replacementPage.collections, ), - hasTargetedCollections, + replacementTargetedTerritories, ) } diff --git a/facia/test/FaciaControllerTest.scala b/facia/test/FaciaControllerTest.scala index 0af2e567ad5..a750fb9f631 100644 --- a/facia/test/FaciaControllerTest.scala +++ b/facia/test/FaciaControllerTest.scala @@ -4,9 +4,12 @@ import agents.{DeeplyReadAgent, MostViewedAgent} import com.fasterxml.jackson.core.JsonParseException import com.gu.facia.client.models.{ConfigJson, FrontJson} import common.editions.{Uk, Us} +import common.facia.FixtureBuilder import controllers.{Assets, FaciaControllerImpl} +import experiments.{ActiveExperiments, EuropeBetaFront, ParticipationGroups} import helpers.FaciaTestData import implicits.FakeRequests +import model.{FrontProperties, PressedPage, SeoData} import org.mockito.Matchers.{any, anyString} import org.mockito.Mockito.when import org.scalatest._ @@ -35,7 +38,8 @@ import scala.concurrent.{Await, Future} with MockitoSugar with WithTestFrontJsonFapi with WithTestContentApiClient - with WithAssets { + with WithAssets + with PrivateMethodTester { lazy val wsClient = mockWsResponse() @@ -87,6 +91,7 @@ import scala.concurrent.{Await, Future} "uk" -> frontJson, "au/media" -> frontJson, "email/uk/daily" -> frontJson, + "europe" -> frontJson, ), collections = Map.empty, ), @@ -257,4 +262,27 @@ import scala.concurrent.{Await, Future} status(result) should be(200) header("X-GU-Dotcomponents", result) should be(None) } + + "FaciaController.replaceFaciaPageCollections" should "replace the collections of a pressed page with those on another pressed page" in { + val europePage: Option[(PressedPage, Boolean)] = Some( + europeFaciaPage, + false, + ) + val europeBetaPage: Option[(PressedPage, Boolean)] = Some( + europeBetaFaciaPageWithTargetedTerritory, + true, + ) + val replaceFaciaPageCollections = + PrivateMethod[Option[(PressedPage, Boolean)]](Symbol("replaceFaciaPageCollections")) + val result = faciaController invokePrivate replaceFaciaPageCollections(europePage, europeBetaPage) + val (resultPressedPage, targetedTerritories) = result.get + // The page metadata should remain unchanged + resultPressedPage.id should not be "europe-beta" + // The collections should come from the replacement page not the original page + resultPressedPage.collections.exists(_ == europeBetaFaciaPageWithTargetedTerritory.collections(0)) should be(true) + resultPressedPage.collections.exists(_ == europeFaciaPage.collections(0)) should be(false) + // The value for targetedTerritories should come from the page with replacement collections + targetedTerritories should be(true) + + } }