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

Add FaciaPage case class to FaciaController #27644

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
58 changes: 30 additions & 28 deletions facia/app/controllers/FaciaController.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 {
Expand All @@ -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)
Expand All @@ -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
Expand All @@ -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)))
}
Expand All @@ -309,46 +311,46 @@ 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(
s"Rendering front $path, frontjson: ${Json.stringify(Json.toJson(faciaPage)(pressedPageFormat))}",
List(),
)
}
Future.successful(Some(regionalFaciaPage, pageContainsTargetedCollections))
Future.successful(Some(FaciaPage(regionalFaciaPage, hasTargetedCollections)))
case None => Future.successful(None)
}

/** 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[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) = {
Expand Down
23 changes: 11 additions & 12 deletions facia/test/FaciaControllerTest.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down