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

Replace collections on europe front if participating in the Europe beta test #27635

Merged
merged 4 commits into from
Nov 29, 2024
Merged
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
78 changes: 78 additions & 0 deletions common/test/helpers/FaciaTestData.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand All @@ -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

Expand Down Expand Up @@ -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"),
Expand Down Expand Up @@ -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)),
Expand Down
95 changes: 65 additions & 30 deletions facia/app/controllers/FaciaController.scala
Original file line number Diff line number Diff line change
Expand Up @@ -151,9 +151,9 @@ trait FaciaController
Action.async { implicit request =>
if (shouldEditionRedirect(path))
redirectTo(Editionalise(path, Edition(request)))
else if (!ConfigAgent.shouldServeFront(path) || request.getQueryString("page").isDefined)
else if (!ConfigAgent.shouldServeFront(path) || request.getQueryString("page").isDefined) {
applicationsRedirect(path)
else
} else
renderFrontPressResult(path)
}

Expand Down Expand Up @@ -219,40 +219,28 @@ trait FaciaController
targetedTerritories,
)

import PressedPage.pressedPageFormat
private[controllers] def renderFrontPressResult(path: String)(implicit request: RequestHeader): Future[Result] = {
// Europe beta experiment
// Phase 1 prevents users from being able to view the europe-beta front unless opted into the test
val inEuropeBetaTest = ActiveExperiments.isParticipating(EuropeBetaFront)
if (path == "europe-beta" && !inEuropeBetaTest && !context.isPreview) {
return successful(Cached(CacheTime.NotFound)(WithoutRevalidationResult(NotFound)))
}

val futureFaciaPage: Future[Option[(PressedPage, Boolean)]] = frontJsonFapi.get(path, liteRequestType).flatMap {
case Some(faciaPage: PressedPage) =>
val pageContainsTargetedCollections = TargetedCollections.pageContainsTargetedCollections(faciaPage)
val regionalFaciaPage = TargetedCollections.processTargetedCollections(
faciaPage,
request.territories,
context.isPreview,
pageContainsTargetedCollections,
)
if (conf.Configuration.environment.stage == "CODE") {
logInfoWithCustomFields(
s"Rendering front $path, frontjson: ${Json.stringify(Json.toJson(faciaPage)(pressedPageFormat))}",
List(),
)
}
if (faciaPage.collections.isEmpty && liteRequestType == LiteAdFreeType) {
frontJsonFapi.get(path, LiteType).map(_.map(f => (f, false)))
} else Future.successful(Some(regionalFaciaPage, pageContainsTargetedCollections))
case None => Future.successful(None)
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 futureEuropeBetaPage = getFaciaPage("europe-beta")
for {
europePage <- futureFaciaPage
europeBetaPage <- futureEuropeBetaPage
} yield replaceFaciaPageCollections(europePage, europeBetaPage)
} else {
futureFaciaPage
}
}

val networkFrontEdition = Edition.allEditions.find(_.networkFrontId == path)
val deeplyRead = networkFrontEdition.map(deeplyReadAgent.getTrails)

val futureResult = futureFaciaPage.flatMap {
val futureResult = futureFaciaPageWithEuropeBetaTest.flatMap {
case Some((faciaPage, _)) if nonHtmlEmail(request) =>
successful(Cached(CacheTime.RecentlyUpdated)(renderEmail(faciaPage)))
case Some((faciaPage: PressedPage, targetedTerritories))
Expand Down Expand Up @@ -316,6 +304,53 @@ trait FaciaController
futureResult
}

import PressedPage.pressedPageFormat

/** Fetches facia page for path */
private[controllers] def getFaciaPage(path: String)(implicit
request: RequestHeader,
): Future[Option[(PressedPage, Boolean)]] = frontJsonFapi.get(path, liteRequestType).flatMap {
case Some(faciaPage: PressedPage) if faciaPage.collections.isEmpty && liteRequestType == LiteAdFreeType =>
frontJsonFapi.get(path, LiteType).map(_.map(f => (f, false)))
case Some(faciaPage: PressedPage) =>
val pageContainsTargetedCollections = TargetedCollections.pageContainsTargetedCollections(faciaPage)
val regionalFaciaPage = TargetedCollections.processTargetedCollections(
faciaPage,
request.territories,
context.isPreview,
pageContainsTargetedCollections,
)
if (conf.Configuration.environment.stage == "CODE") {
logInfoWithCustomFields(
s"Rendering front $path, frontjson: ${Json.stringify(Json.toJson(faciaPage)(pressedPageFormat))}",
List(),
)
}
Future.successful(Some(regionalFaciaPage, pageContainsTargetedCollections))
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)] = {
for {
(basePage, _) <- baseFaciaPage
(replacementPage, replacementTargetedTerritories) <- replacementFaciaPage
} yield (
PressedPage(
id = basePage.id,
seoData = basePage.seoData,
frontProperties = basePage.frontProperties,
collections = replacementPage.collections,
),
replacementTargetedTerritories,
)
}

private def renderEmail(faciaPage: PressedPage)(implicit request: RequestHeader) = {
if (request.isHeadlineText) {
FrontHeadline.renderEmailHeadline(faciaPage)
Expand Down
30 changes: 29 additions & 1 deletion facia/test/FaciaControllerTest.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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._
Expand Down Expand Up @@ -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()

Expand Down Expand Up @@ -87,6 +91,7 @@ import scala.concurrent.{Await, Future}
"uk" -> frontJson,
"au/media" -> frontJson,
"email/uk/daily" -> frontJson,
"europe" -> frontJson,
),
collections = Map.empty,
),
Expand Down Expand Up @@ -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 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)
}
}