diff --git a/facia/app/controllers/FaciaController.scala b/facia/app/controllers/FaciaController.scala index 4ffb55d8632..01ed4cc6050 100644 --- a/facia/app/controllers/FaciaController.scala +++ b/facia/app/controllers/FaciaController.scala @@ -32,6 +32,8 @@ import views.support.FaciaToMicroFormat2Helpers.getCollection import scala.concurrent.Future import scala.concurrent.Future.successful +case class FaciaPage(pressedPage: PressedPage, hasTargetedCollections: Boolean) + trait FaciaController extends BaseController with GuLogging @@ -225,7 +227,7 @@ trait FaciaController /** 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)]] = { + val futureFaciaPageWithEuropeBetaTest: Future[Option[FaciaPage]] = { if (path == "europe" && ActiveExperiments.isParticipating(EuropeBetaFront)) { val futureEuropeBetaPage = getFaciaPage("europe-beta") for { @@ -241,9 +243,9 @@ trait FaciaController val deeplyRead = networkFrontEdition.map(deeplyReadAgent.getTrails) val futureResult = futureFaciaPageWithEuropeBetaTest.flatMap { - case Some((faciaPage, _)) if nonHtmlEmail(request) => + case Some(FaciaPage(faciaPage, _)) if nonHtmlEmail(request) => successful(Cached(CacheTime.RecentlyUpdated)(renderEmail(faciaPage))) - case Some((faciaPage: PressedPage, targetedTerritories)) + case Some(FaciaPage(faciaPage, hasTargetedCollections)) if FaciaPicker.getTier(faciaPage) == RemoteRender && !request.isJson => val pageType = PageType(faciaPage, request, context) @@ -261,16 +263,16 @@ trait FaciaController mostShared = mostViewedAgent.mostShared, deeplyRead = deeplyRead, )(request), - targetedTerritories, + hasTargetedCollections, ) - case Some((faciaPage: PressedPage, targetedTerritories)) if request.isRss => + case Some(FaciaPage(faciaPage, hasTargetedCollections)) if request.isRss => val body = TrailsToRss.fromPressedPage(faciaPage) withVaryHeader( successful(Cached(CacheTime.Facia)(RevalidatableResult(Ok(body).as("text/xml; charset=utf-8"), body))), - targetedTerritories, + hasTargetedCollections, ) - case Some((faciaPage: PressedPage, targetedTerritories)) if request.isJson => + case Some(FaciaPage(faciaPage, hasTargetedCollections)) if request.isJson => val result = if (request.forceDCR) { log.info( s"Front Geo Request (237): ${Edition(request).id} ${request.headers.toSimpleMap @@ -288,13 +290,13 @@ trait FaciaController ), ) } else JsonFront(faciaPage) - resultWithVaryHeader(result, targetedTerritories) - case Some((faciaPage: PressedPage, targetedTerritories)) if request.isEmail || ConfigAgent.isEmailFront(path) => - resultWithVaryHeader(renderEmail(faciaPage), targetedTerritories) - case Some((faciaPage: PressedPage, targetedTerritories)) if TrailsToShowcase.isShowcaseFront(faciaPage) => - resultWithVaryHeader(renderShowcaseFront(faciaPage), targetedTerritories) - case Some((faciaPage: PressedPage, targetedTerritories)) => - resultWithVaryAndPreloadHeader(RevalidatableResult.Ok(FrontHtmlPage.html(faciaPage)), targetedTerritories) + resultWithVaryHeader(result, hasTargetedCollections) + case Some(FaciaPage(faciaPage, hasTargetedCollections)) if request.isEmail || ConfigAgent.isEmailFront(path) => + resultWithVaryHeader(renderEmail(faciaPage), hasTargetedCollections) + case Some(FaciaPage(faciaPage, hasTargetedCollections)) if TrailsToShowcase.isShowcaseFront(faciaPage) => + resultWithVaryHeader(renderShowcaseFront(faciaPage), hasTargetedCollections) + case Some(FaciaPage(faciaPage, hasTargetedCollections)) => + resultWithVaryAndPreloadHeader(RevalidatableResult.Ok(FrontHtmlPage.html(faciaPage)), hasTargetedCollections) case None => { successful(Cached(CacheTime.NotFound)(WithoutRevalidationResult(NotFound))) } @@ -309,16 +311,16 @@ trait FaciaController /** Fetches facia page for path */ private[controllers] def getFaciaPage(path: String)(implicit request: RequestHeader, - ): Future[Option[(PressedPage, Boolean)]] = frontJsonFapi.get(path, liteRequestType).flatMap { + ): Future[Option[FaciaPage]] = frontJsonFapi.get(path, liteRequestType).flatMap { case Some(faciaPage: PressedPage) if faciaPage.collections.isEmpty && liteRequestType == LiteAdFreeType => - frontJsonFapi.get(path, LiteType).map(_.map(f => (f, false))) + frontJsonFapi.get(path, LiteType).map(_.map(f => FaciaPage(f, hasTargetedCollections = false))) case Some(faciaPage: PressedPage) => - val pageContainsTargetedCollections = TargetedCollections.pageContainsTargetedCollections(faciaPage) + val hasTargetedCollections = TargetedCollections.pageContainsTargetedCollections(faciaPage) val regionalFaciaPage = TargetedCollections.processTargetedCollections( faciaPage, request.territories, context.isPreview, - pageContainsTargetedCollections, + hasTargetedCollections, ) if (conf.Configuration.environment.stage == "CODE") { logInfoWithCustomFields( @@ -326,7 +328,7 @@ trait FaciaController List(), ) } - Future.successful(Some(regionalFaciaPage, pageContainsTargetedCollections)) + Future.successful(Some(FaciaPage(regionalFaciaPage, hasTargetedCollections))) case None => Future.successful(None) } @@ -334,21 +336,21 @@ trait FaciaController * return europe-beta collections on the europe front if participating in the test */ private[controllers] def replaceFaciaPageCollections( - baseFaciaPage: Option[(PressedPage, Boolean)], - replacementFaciaPage: Option[(PressedPage, Boolean)], - ): Option[(PressedPage, Boolean)] = { + baseFaciaPage: Option[FaciaPage], + replacementFaciaPage: Option[FaciaPage], + ): Option[FaciaPage] = { for { - (basePage, _) <- baseFaciaPage - (replacementPage, replacementTargetedTerritories) <- replacementFaciaPage - } yield ( - PressedPage( + FaciaPage(basePage, _) <- baseFaciaPage + FaciaPage(replacementPage, replacementHasTargetedCollections) <- replacementFaciaPage + } yield (FaciaPage( + pressedPage = PressedPage( id = basePage.id, seoData = basePage.seoData, frontProperties = basePage.frontProperties, collections = replacementPage.collections, ), - replacementTargetedTerritories, - ) + hasTargetedCollections = replacementHasTargetedCollections, + )) } private def renderEmail(faciaPage: PressedPage)(implicit request: RequestHeader) = { diff --git a/facia/test/FaciaControllerTest.scala b/facia/test/FaciaControllerTest.scala index b69f76367b3..baa240a7e2b 100644 --- a/facia/test/FaciaControllerTest.scala +++ b/facia/test/FaciaControllerTest.scala @@ -5,7 +5,7 @@ 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 controllers.{Assets, FaciaControllerImpl, FaciaPage} import experiments.{ActiveExperiments, EuropeBetaFront, ParticipationGroups} import helpers.FaciaTestData import implicits.FakeRequests @@ -264,25 +264,24 @@ import scala.concurrent.{Await, Future} } "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 europePage: Option[FaciaPage] = Some(FaciaPage(pressedPage = europeFaciaPage, hasTargetedCollections = false)) + val europeBetaPage: Option[FaciaPage] = Some( + FaciaPage( + pressedPage = europeBetaFaciaPageWithTargetedTerritory, + hasTargetedCollections = true, + ), ) val replaceFaciaPageCollections = - PrivateMethod[Option[(PressedPage, Boolean)]](Symbol("replaceFaciaPageCollections")) + PrivateMethod[Option[FaciaPage]](Symbol("replaceFaciaPageCollections")) val result = faciaController invokePrivate replaceFaciaPageCollections(europePage, europeBetaPage) - val (resultPressedPage, targetedTerritories) = result.get + val FaciaPage(resultPressedPage, hasTargetedCollections) = result.get // The page metadata should remain unchanged resultPressedPage.id should be("europe") 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) + // The value for hasTargetedCollections should come from the page with replacement collections + hasTargetedCollections should be(true) } } diff --git a/facia/test/services/dotcomrendering/FaciaPickerTest.scala b/facia/test/services/dotcomrendering/FaciaPickerTest.scala index 7b18ebad329..d742beb0974 100644 --- a/facia/test/services/dotcomrendering/FaciaPickerTest.scala +++ b/facia/test/services/dotcomrendering/FaciaPickerTest.scala @@ -8,7 +8,6 @@ import org.scalatest.flatspec.AnyFlatSpec import org.scalatest.matchers.should.Matchers import org.scalatestplus.mockito.MockitoSugar import helpers.FaciaTestData -import test.FaciaControllerTest import model.facia.PressedCollection import layout.slices.EmailLayouts