From 71fe2fade95b17a1996301d3e902f24149fe3997 Mon Sep 17 00:00:00 2001 From: Emma Imber Date: Tue, 14 May 2024 10:36:21 +0100 Subject: [PATCH 1/6] Check if first two elements are paragraphs for DCR articles --- .../DotcomRenderingUtils.scala | 33 ++++++++++++------- 1 file changed, 21 insertions(+), 12 deletions(-) diff --git a/common/app/model/dotcomrendering/DotcomRenderingUtils.scala b/common/app/model/dotcomrendering/DotcomRenderingUtils.scala index f5f29f1092e..35df23eb648 100644 --- a/common/app/model/dotcomrendering/DotcomRenderingUtils.scala +++ b/common/app/model/dotcomrendering/DotcomRenderingUtils.scala @@ -22,6 +22,7 @@ import model.{ Pillar, } import org.joda.time.format.DateTimeFormat +import org.jsoup.Jsoup import play.api.libs.json._ import play.api.mvc.RequestHeader import views.support.AffiliateLinksCleaner @@ -242,18 +243,26 @@ object DotcomRenderingUtils { } def shouldAddAffiliateLinks(content: ContentType): Boolean = { - AffiliateLinksCleaner.shouldAddAffiliateLinks( - switchedOn = Switches.AffiliateLinks.isSwitchedOn, - section = content.metadata.sectionId, - showAffiliateLinks = content.content.fields.showAffiliateLinks, - supportedSections = Configuration.affiliateLinks.affiliateLinkSections, - defaultOffTags = Configuration.affiliateLinks.defaultOffTags, - alwaysOffTags = Configuration.affiliateLinks.alwaysOffTags, - tagPaths = content.content.tags.tags.map(_.id), - firstPublishedDate = content.content.fields.firstPublicationDate, - pageUrl = content.metadata.id, - contentType = "article", - ) + val contentHtml = Jsoup.parse(content.fields.body) + val bodyElements = contentHtml.select("body").first().children() + if (bodyElements.size >= 2) { + val firstEl = bodyElements.get(0) + val secondEl = bodyElements.get(1) + if (firstEl.tagName == "p" && secondEl.tagName == "p") { + AffiliateLinksCleaner.shouldAddAffiliateLinks( + switchedOn = Switches.AffiliateLinks.isSwitchedOn, + section = content.metadata.sectionId, + showAffiliateLinks = content.content.fields.showAffiliateLinks, + supportedSections = Configuration.affiliateLinks.affiliateLinkSections, + defaultOffTags = Configuration.affiliateLinks.defaultOffTags, + alwaysOffTags = Configuration.affiliateLinks.alwaysOffTags, + tagPaths = content.content.tags.tags.map(_.id), + firstPublishedDate = content.content.fields.firstPublicationDate, + pageUrl = content.metadata.id, + contentType = "article", + ) + } else false + } else false } def contentDateTimes(content: ContentType): ArticleDateTimes = { From 6f41a024325cae38a50e567377ef830498c5b400 Mon Sep 17 00:00:00 2001 From: Emma Imber Date: Tue, 14 May 2024 11:56:05 +0100 Subject: [PATCH 2/6] Add 0% test for affiliate links on DCR --- common/app/experiments/Experiments.scala | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/common/app/experiments/Experiments.scala b/common/app/experiments/Experiments.scala index dcad0f1b1d1..a70890d856c 100644 --- a/common/app/experiments/Experiments.scala +++ b/common/app/experiments/Experiments.scala @@ -16,6 +16,7 @@ object ActiveExperiments extends ExperimentsDefinition { DCRVideoPages, UpdatedHeaderDesign, MastheadWithHighlights, + AffiliateLinksDCR, ) implicit val canCheckExperiment: CanCheckExperiment = new CanCheckExperiment(this) } @@ -48,6 +49,15 @@ object DarkModeWeb participationGroup = Perc0D, ) +object AffiliateLinksDCR + extends Experiment( + name = "affiliate-links-dcr", + description = "Display affiliate links on all eligible DCR articles", + owners = Seq(Owner.withGithub("commercial.dev@theguardian.com")), + sellByDate = LocalDate.of(2024, 7, 30), + participationGroup = Perc0E, + ) + object DCRVideoPages extends Experiment( name = "dcr-video-pages", From ddf35b735aa9f85bcd925fd30bb7eb1bb542a172 Mon Sep 17 00:00:00 2001 From: Emma Imber Date: Tue, 14 May 2024 12:11:16 +0100 Subject: [PATCH 3/6] Check if user is in test variant --- .../DotcomBlocksRenderingDataModel.scala | 4 +- .../DotcomRenderingDataModel.scala | 2 +- .../DotcomRenderingUtils.scala | 2 +- common/app/views/support/HtmlCleaner.scala | 10 +++-- .../cleaner/AffiliateLinksCleanerTest.scala | 38 +++++++++++++------ 5 files changed, 38 insertions(+), 18 deletions(-) diff --git a/common/app/model/dotcomrendering/DotcomBlocksRenderingDataModel.scala b/common/app/model/dotcomrendering/DotcomBlocksRenderingDataModel.scala index 6edc08e8aa5..7d6104ea56d 100644 --- a/common/app/model/dotcomrendering/DotcomBlocksRenderingDataModel.scala +++ b/common/app/model/dotcomrendering/DotcomBlocksRenderingDataModel.scala @@ -5,6 +5,7 @@ import com.gu.contentapi.client.model.v1.{Block => APIBlock} import common.Edition import common.commercial.EditionAdTargeting.adTargetParamValueWrites import conf.Configuration +import experiments.{ActiveExperiments, AffiliateLinksDCR} import model.dotcomrendering.pageElements.PageElement import model.{ContentFormat, ContentPage} import play.api.libs.json._ @@ -69,7 +70,8 @@ object DotcomBlocksRenderingDataModel { bodyBlocks: Seq[APIBlock], ): DotcomBlocksRenderingDataModel = { val content = page.item - val shouldAddAffiliateLinks = DotcomRenderingUtils.shouldAddAffiliateLinks(content) + val isInAffiliateLinksTest = ActiveExperiments.isParticipating(AffiliateLinksDCR)(request) + val shouldAddAffiliateLinks = DotcomRenderingUtils.shouldAddAffiliateLinks(content)(request) val contentDateTimes = DotcomRenderingUtils.contentDateTimes(content) val edition = Edition(request) diff --git a/common/app/model/dotcomrendering/DotcomRenderingDataModel.scala b/common/app/model/dotcomrendering/DotcomRenderingDataModel.scala index f1f280ab99c..fe97e8df2d9 100644 --- a/common/app/model/dotcomrendering/DotcomRenderingDataModel.scala +++ b/common/app/model/dotcomrendering/DotcomRenderingDataModel.scala @@ -487,7 +487,7 @@ object DotcomRenderingDataModel { blocks.exists(block => DotcomRenderingUtils.stringContainsAffiliateableLinks(block.bodyHtml)) } - val shouldAddAffiliateLinks = DotcomRenderingUtils.shouldAddAffiliateLinks(content) + val shouldAddAffiliateLinks = DotcomRenderingUtils.shouldAddAffiliateLinks(content)(request) val shouldAddDisclaimer = hasAffiliateLinks(bodyBlocks) val contentDateTimes: ArticleDateTimes = ArticleDateTimes( diff --git a/common/app/model/dotcomrendering/DotcomRenderingUtils.scala b/common/app/model/dotcomrendering/DotcomRenderingUtils.scala index 35df23eb648..7a895d5c1ce 100644 --- a/common/app/model/dotcomrendering/DotcomRenderingUtils.scala +++ b/common/app/model/dotcomrendering/DotcomRenderingUtils.scala @@ -242,7 +242,7 @@ object DotcomRenderingUtils { } } - def shouldAddAffiliateLinks(content: ContentType): Boolean = { + def shouldAddAffiliateLinks(content: ContentType)(implicit request: RequestHeader): Boolean = { val contentHtml = Jsoup.parse(content.fields.body) val bodyElements = contentHtml.select("body").first().children() if (bodyElements.size >= 2) { diff --git a/common/app/views/support/HtmlCleaner.scala b/common/app/views/support/HtmlCleaner.scala index 9756c01c857..9c543d4c310 100644 --- a/common/app/views/support/HtmlCleaner.scala +++ b/common/app/views/support/HtmlCleaner.scala @@ -6,6 +6,7 @@ import common.{Edition, GuLogging, LinkTo} import conf.Configuration.affiliateLinks._ import conf.Configuration.site.host import conf.switches.Switches._ +import experiments.{ActiveExperiments, AffiliateLinksDCR} import layout.ContentWidths import layout.ContentWidths._ import model._ @@ -873,7 +874,8 @@ case class AffiliateLinksCleaner( tags: List[String], publishedDate: Option[DateTime], contentType: String, -) extends HtmlCleaner +)(implicit request: RequestHeader) + extends HtmlCleaner with GuLogging { override def clean(document: Document): Document = { @@ -946,7 +948,7 @@ object AffiliateLinksCleaner { firstPublishedDate: Option[DateTime], pageUrl: String, contentType: String, - ): Boolean = { + )(implicit request: RequestHeader): Boolean = { val publishedCutOffDate = new DateTime(2020, 8, 14, 0, 0) val cleanedPageUrl = if (pageUrl.charAt(0) == '/') { @@ -973,11 +975,11 @@ object AffiliateLinksCleaner { // Never include affiliate links if it is tagged with an always off tag, or if it was published before our cut off date. // The cut off date is temporary while we are working on improving the compliance of affiliate links. - // The cut off date does not apply to any URL on the allow list + // The cut off date does not apply to any URL on the allow list or to galleries if ( !contentHasAlwaysOffTag(tagPaths, alwaysOffTags) && (firstPublishedDate.exists( _.isBefore(publishedCutOffDate), - ) || urlIsInAllowList || contentType == "gallery") + ) || urlIsInAllowList || contentType == "gallery" || ActiveExperiments.isParticipating(AffiliateLinksDCR)) ) { if (showAffiliateLinks.isDefined) { showAffiliateLinks.contains(true) diff --git a/common/test/views/support/cleaner/AffiliateLinksCleanerTest.scala b/common/test/views/support/cleaner/AffiliateLinksCleanerTest.scala index e0133ebaf90..3871559ab7f 100644 --- a/common/test/views/support/cleaner/AffiliateLinksCleanerTest.scala +++ b/common/test/views/support/cleaner/AffiliateLinksCleanerTest.scala @@ -1,8 +1,10 @@ package views.support.cleaner import conf.Configuration +import conf.switches.Switches.ServerSideExperiments import org.joda.time.DateTime import org.scalatest.flatspec.AnyFlatSpec import org.scalatest.matchers.should.Matchers +import play.api.test.FakeRequest import views.support.AffiliateLinksCleaner._ class AffiliateLinksCleanerTest extends AnyFlatSpec with Matchers { @@ -16,6 +18,8 @@ class AffiliateLinksCleanerTest extends AnyFlatSpec with Matchers { } "shouldAddAffiliateLinks" should "correctly determine when to add affiliate links" in { + val fakeTestVariantRequest = FakeRequest().withHeaders("X-GU-Experiment-0perc-E" -> "variant") + val fakeTestControlRequest = FakeRequest().withHeaders("X-GU-Experiment-0perc-E" -> "control") val supportedSections = Set("film", "books", "fashion") val oldPublishedDate = Some(new DateTime(2020, 8, 13, 0, 0)) val newPublishedDate = Some(new DateTime(2020, 8, 15, 0, 0)) @@ -32,7 +36,7 @@ class AffiliateLinksCleanerTest extends AnyFlatSpec with Matchers { oldPublishedDate, deniedPageUrl, "article", - ) should be(false) + )(fakeTestControlRequest) should be(false) shouldAddAffiliateLinks( switchedOn = true, "film", @@ -44,7 +48,7 @@ class AffiliateLinksCleanerTest extends AnyFlatSpec with Matchers { oldPublishedDate, deniedPageUrl, "article", - ) should be(true) + )(fakeTestControlRequest) should be(true) shouldAddAffiliateLinks( switchedOn = true, "film", @@ -56,7 +60,7 @@ class AffiliateLinksCleanerTest extends AnyFlatSpec with Matchers { oldPublishedDate, deniedPageUrl, "article", - ) should be(false) + )(fakeTestControlRequest) should be(false) shouldAddAffiliateLinks( switchedOn = true, "news", @@ -68,7 +72,7 @@ class AffiliateLinksCleanerTest extends AnyFlatSpec with Matchers { oldPublishedDate, deniedPageUrl, "article", - ) should be(true) + )(fakeTestControlRequest) should be(true) shouldAddAffiliateLinks( switchedOn = true, "news", @@ -80,7 +84,7 @@ class AffiliateLinksCleanerTest extends AnyFlatSpec with Matchers { oldPublishedDate, deniedPageUrl, "article", - ) should be(false) + )(fakeTestControlRequest) should be(false) shouldAddAffiliateLinks( switchedOn = true, "news", @@ -92,7 +96,7 @@ class AffiliateLinksCleanerTest extends AnyFlatSpec with Matchers { oldPublishedDate, deniedPageUrl, "article", - ) should be(false) + )(fakeTestControlRequest) should be(false) shouldAddAffiliateLinks( switchedOn = true, "fashion", @@ -104,7 +108,7 @@ class AffiliateLinksCleanerTest extends AnyFlatSpec with Matchers { oldPublishedDate, deniedPageUrl, "article", - ) should be(true) + )(fakeTestControlRequest) should be(true) shouldAddAffiliateLinks( switchedOn = true, "fashion", @@ -116,7 +120,7 @@ class AffiliateLinksCleanerTest extends AnyFlatSpec with Matchers { oldPublishedDate, deniedPageUrl, "article", - ) should be(false) + )(fakeTestControlRequest) should be(false) shouldAddAffiliateLinks( switchedOn = true, "fashion", @@ -128,7 +132,7 @@ class AffiliateLinksCleanerTest extends AnyFlatSpec with Matchers { oldPublishedDate, deniedPageUrl, "article", - ) should be(true) + )(fakeTestControlRequest) should be(true) shouldAddAffiliateLinks( switchedOn = true, "film", @@ -140,7 +144,7 @@ class AffiliateLinksCleanerTest extends AnyFlatSpec with Matchers { newPublishedDate, deniedPageUrl, "article", - ) should be(false) + )(fakeTestControlRequest) should be(false) shouldAddAffiliateLinks( switchedOn = true, "film", @@ -152,6 +156,18 @@ class AffiliateLinksCleanerTest extends AnyFlatSpec with Matchers { newPublishedDate, deniedPageUrl, "gallery", - ) should be(true) + )(fakeTestControlRequest) should be(true) + shouldAddAffiliateLinks( + switchedOn = true, + "film", + None, + supportedSections, + Set.empty, + Set.empty, + List.empty, + newPublishedDate, + deniedPageUrl, + "article", + )(fakeTestVariantRequest) should be(true) } } From 82203304133a7e1f5916c165ce484a57798b1ca3 Mon Sep 17 00:00:00 2001 From: Emma Imber Date: Tue, 14 May 2024 12:36:52 +0100 Subject: [PATCH 4/6] Remove unnecessary var --- .../model/dotcomrendering/DotcomBlocksRenderingDataModel.scala | 2 -- 1 file changed, 2 deletions(-) diff --git a/common/app/model/dotcomrendering/DotcomBlocksRenderingDataModel.scala b/common/app/model/dotcomrendering/DotcomBlocksRenderingDataModel.scala index 7d6104ea56d..65c90d5e409 100644 --- a/common/app/model/dotcomrendering/DotcomBlocksRenderingDataModel.scala +++ b/common/app/model/dotcomrendering/DotcomBlocksRenderingDataModel.scala @@ -5,7 +5,6 @@ import com.gu.contentapi.client.model.v1.{Block => APIBlock} import common.Edition import common.commercial.EditionAdTargeting.adTargetParamValueWrites import conf.Configuration -import experiments.{ActiveExperiments, AffiliateLinksDCR} import model.dotcomrendering.pageElements.PageElement import model.{ContentFormat, ContentPage} import play.api.libs.json._ @@ -70,7 +69,6 @@ object DotcomBlocksRenderingDataModel { bodyBlocks: Seq[APIBlock], ): DotcomBlocksRenderingDataModel = { val content = page.item - val isInAffiliateLinksTest = ActiveExperiments.isParticipating(AffiliateLinksDCR)(request) val shouldAddAffiliateLinks = DotcomRenderingUtils.shouldAddAffiliateLinks(content)(request) val contentDateTimes = DotcomRenderingUtils.contentDateTimes(content) From 0c31a5fe15e6286d7e7bbb78f90fb78683e4616d Mon Sep 17 00:00:00 2001 From: Emma Imber Date: Tue, 14 May 2024 12:39:40 +0100 Subject: [PATCH 5/6] Remove variant test --- .../support/cleaner/AffiliateLinksCleanerTest.scala | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/common/test/views/support/cleaner/AffiliateLinksCleanerTest.scala b/common/test/views/support/cleaner/AffiliateLinksCleanerTest.scala index 3871559ab7f..28f4b9288e5 100644 --- a/common/test/views/support/cleaner/AffiliateLinksCleanerTest.scala +++ b/common/test/views/support/cleaner/AffiliateLinksCleanerTest.scala @@ -18,7 +18,6 @@ class AffiliateLinksCleanerTest extends AnyFlatSpec with Matchers { } "shouldAddAffiliateLinks" should "correctly determine when to add affiliate links" in { - val fakeTestVariantRequest = FakeRequest().withHeaders("X-GU-Experiment-0perc-E" -> "variant") val fakeTestControlRequest = FakeRequest().withHeaders("X-GU-Experiment-0perc-E" -> "control") val supportedSections = Set("film", "books", "fashion") val oldPublishedDate = Some(new DateTime(2020, 8, 13, 0, 0)) @@ -157,17 +156,5 @@ class AffiliateLinksCleanerTest extends AnyFlatSpec with Matchers { deniedPageUrl, "gallery", )(fakeTestControlRequest) should be(true) - shouldAddAffiliateLinks( - switchedOn = true, - "film", - None, - supportedSections, - Set.empty, - Set.empty, - List.empty, - newPublishedDate, - deniedPageUrl, - "article", - )(fakeTestVariantRequest) should be(true) } } From c7b07026f2a9a5fa891f756e6c38b2bb6abe08e4 Mon Sep 17 00:00:00 2001 From: Emma Imber Date: Tue, 14 May 2024 15:02:24 +0100 Subject: [PATCH 6/6] Add minimum character count for second p element --- common/app/model/dotcomrendering/DotcomRenderingUtils.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/common/app/model/dotcomrendering/DotcomRenderingUtils.scala b/common/app/model/dotcomrendering/DotcomRenderingUtils.scala index 7a895d5c1ce..996ef732729 100644 --- a/common/app/model/dotcomrendering/DotcomRenderingUtils.scala +++ b/common/app/model/dotcomrendering/DotcomRenderingUtils.scala @@ -248,7 +248,7 @@ object DotcomRenderingUtils { if (bodyElements.size >= 2) { val firstEl = bodyElements.get(0) val secondEl = bodyElements.get(1) - if (firstEl.tagName == "p" && secondEl.tagName == "p") { + if (firstEl.tagName == "p" && secondEl.tagName == "p" && secondEl.text().length >= 250) { AffiliateLinksCleaner.shouldAddAffiliateLinks( switchedOn = Switches.AffiliateLinks.isSwitchedOn, section = content.metadata.sectionId,