From 79c5a0ee23862ae02a2f09ebe5c743b98ad41ce3 Mon Sep 17 00:00:00 2001 From: Demetrios Skamiotis Date: Mon, 13 May 2024 19:06:48 +0100 Subject: [PATCH] add the document as a param to shouldAffiliateLinks and related functions --- .../app/views/fragments/galleryBody.scala.html | 2 ++ .../DotcomBlocksRenderingDataModel.scala | 5 ++++- .../dotcomrendering/DotcomRenderingDataModel.scala | 6 +++++- .../model/dotcomrendering/DotcomRenderingUtils.scala | 4 +++- common/app/views/support/HtmlCleaner.scala | 2 ++ .../support/cleaner/AffiliateLinksCleanerTest.scala | 11 +++++++++++ 6 files changed, 27 insertions(+), 3 deletions(-) diff --git a/applications/app/views/fragments/galleryBody.scala.html b/applications/app/views/fragments/galleryBody.scala.html index 44847e6c4804..097f84471e4b 100644 --- a/applications/app/views/fragments/galleryBody.scala.html +++ b/applications/app/views/fragments/galleryBody.scala.html @@ -35,6 +35,8 @@ firstPublishedDate = page.gallery.content.fields.firstPublicationDate, pageUrl = request.uri, contentType = "gallery", + // This is a bit of a hack, but we need to pass the content + document = page.gallery.content ) ) diff --git a/common/app/model/dotcomrendering/DotcomBlocksRenderingDataModel.scala b/common/app/model/dotcomrendering/DotcomBlocksRenderingDataModel.scala index 6edc08e8aa59..a663c3d2b9fa 100644 --- a/common/app/model/dotcomrendering/DotcomBlocksRenderingDataModel.scala +++ b/common/app/model/dotcomrendering/DotcomBlocksRenderingDataModel.scala @@ -11,6 +11,7 @@ import play.api.libs.json._ import play.api.mvc.RequestHeader import views.support.CamelCase import experiments.ActiveExperiments +import org.jsoup.Jsoup // ----------------------------------------------------------------- // DCR Blocks DataModel @@ -69,7 +70,9 @@ object DotcomBlocksRenderingDataModel { bodyBlocks: Seq[APIBlock], ): DotcomBlocksRenderingDataModel = { val content = page.item - val shouldAddAffiliateLinks = DotcomRenderingUtils.shouldAddAffiliateLinks(content) + val htmlBlocks = bodyBlocks.map { block => block.bodyHtml } + val document = Jsoup.parseBodyFragment(htmlBlocks.mkString("")) + val shouldAddAffiliateLinks = DotcomRenderingUtils.shouldAddAffiliateLinks(content, document) 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 f1f280ab99cc..acc248a589ae 100644 --- a/common/app/model/dotcomrendering/DotcomRenderingDataModel.scala +++ b/common/app/model/dotcomrendering/DotcomRenderingDataModel.scala @@ -7,6 +7,7 @@ import common.Maps.RichMap import common.commercial.EditionCommercialProperties import common.{CanonicalLink, Chronos, Edition, Localisation, RichRequestHeader} import conf.Configuration +import org.jsoup.Jsoup import crosswords.CrosswordPageWithContent import experiments.ActiveExperiments import model.dotcomrendering.DotcomRenderingUtils._ @@ -487,7 +488,10 @@ object DotcomRenderingDataModel { blocks.exists(block => DotcomRenderingUtils.stringContainsAffiliateableLinks(block.bodyHtml)) } - val shouldAddAffiliateLinks = DotcomRenderingUtils.shouldAddAffiliateLinks(content) + val htmlBlocks = bodyBlocks.map { block => block.bodyHtml } + val document = Jsoup.parseBodyFragment(htmlBlocks.mkString("")) + + val shouldAddAffiliateLinks = DotcomRenderingUtils.shouldAddAffiliateLinks(content, document) 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 f5f29f1092e9..11039b039cc4 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.nodes.Document import play.api.libs.json._ import play.api.mvc.RequestHeader import views.support.AffiliateLinksCleaner @@ -241,7 +242,7 @@ object DotcomRenderingUtils { } } - def shouldAddAffiliateLinks(content: ContentType): Boolean = { + def shouldAddAffiliateLinks(content: ContentType, document: Document): Boolean = { AffiliateLinksCleaner.shouldAddAffiliateLinks( switchedOn = Switches.AffiliateLinks.isSwitchedOn, section = content.metadata.sectionId, @@ -253,6 +254,7 @@ object DotcomRenderingUtils { firstPublishedDate = content.content.fields.firstPublicationDate, pageUrl = content.metadata.id, contentType = "article", + document, ) } diff --git a/common/app/views/support/HtmlCleaner.scala b/common/app/views/support/HtmlCleaner.scala index 9756c01c8578..6f2561717142 100644 --- a/common/app/views/support/HtmlCleaner.scala +++ b/common/app/views/support/HtmlCleaner.scala @@ -889,6 +889,7 @@ case class AffiliateLinksCleaner( publishedDate, pageUrl, contentType, + document, ) ) { AffiliateLinksCleaner.replaceLinksInHtml(document, pageUrl, skimlinksId) @@ -946,6 +947,7 @@ object AffiliateLinksCleaner { firstPublishedDate: Option[DateTime], pageUrl: String, contentType: String, + document: Document = null, ): Boolean = { val publishedCutOffDate = new DateTime(2020, 8, 14, 0, 0) diff --git a/common/test/views/support/cleaner/AffiliateLinksCleanerTest.scala b/common/test/views/support/cleaner/AffiliateLinksCleanerTest.scala index e0133ebaf90c..9bbed8b0f313 100644 --- a/common/test/views/support/cleaner/AffiliateLinksCleanerTest.scala +++ b/common/test/views/support/cleaner/AffiliateLinksCleanerTest.scala @@ -20,6 +20,7 @@ class AffiliateLinksCleanerTest extends AnyFlatSpec with Matchers { val oldPublishedDate = Some(new DateTime(2020, 8, 13, 0, 0)) val newPublishedDate = Some(new DateTime(2020, 8, 15, 0, 0)) val deniedPageUrl = "/fashion/2024/feb/16/sunscreen-in-winter-yep-spf-moisturiser-is-essential-all-year-round" + val validHtml = """
\n

There are so many lipsticks to choose from now, which is fun but can be overwhelming. The longest-wearing formulation is liquid, which forms a sort of seal on the lips, but many people find them drying. The next best thing is a matt lipstick bullet, offering staying power without parched lips.

\n

Start with picking a colour you love. You told me your outfit and colourings: a cream dress suits bright red. If you’re new to red lipstick, I’d recommend a brown-ish red such as Charlotte Tilbury Matte Revolution in Walk of No Shame. Or try something with a hint of orange, such as Chilli, a warm, brick red, or Lady Danger, a vivid coral, from Mac’s reformulated Macximal Silky Matte range.

\n

A neutral shade similar to your own lip colour works for every outfit. Dior Rouge Dior Couture Colour range has incredible staying power, but for a creamier finish, its Make Up Revolution IRL Filter Finish Lip Crème is in between a matt and a cream lipstick, and lasts well. Or try a pigmented balm that really lasts – Kosas and Westman Atelier are my go-tos.

\n

Whatever colour or formulation you pick, always use a lipliner in a similar or slightly darker shade to line the lips and avoid any bleeding or feathering. But my biggest hack is to use the lipliner to also fill in your lips; this gives the lipstick something to “grip” on to and keeps it in place. Whatever you pick, if you like it, it’s perfect – whether it’s green, nude, red or sparkly purple.

\n

Got a beauty question for Anita? Email her at BeautyQandA@theguardian.com

\n
""" shouldAddAffiliateLinks( switchedOn = false, @@ -32,6 +33,7 @@ class AffiliateLinksCleanerTest extends AnyFlatSpec with Matchers { oldPublishedDate, deniedPageUrl, "article", + validHtml, ) should be(false) shouldAddAffiliateLinks( switchedOn = true, @@ -44,6 +46,7 @@ class AffiliateLinksCleanerTest extends AnyFlatSpec with Matchers { oldPublishedDate, deniedPageUrl, "article", + validHtml, ) should be(true) shouldAddAffiliateLinks( switchedOn = true, @@ -56,6 +59,7 @@ class AffiliateLinksCleanerTest extends AnyFlatSpec with Matchers { oldPublishedDate, deniedPageUrl, "article", + validHtml, ) should be(false) shouldAddAffiliateLinks( switchedOn = true, @@ -68,6 +72,7 @@ class AffiliateLinksCleanerTest extends AnyFlatSpec with Matchers { oldPublishedDate, deniedPageUrl, "article", + validHtml, ) should be(true) shouldAddAffiliateLinks( switchedOn = true, @@ -80,6 +85,7 @@ class AffiliateLinksCleanerTest extends AnyFlatSpec with Matchers { oldPublishedDate, deniedPageUrl, "article", + validHtml, ) should be(false) shouldAddAffiliateLinks( switchedOn = true, @@ -92,6 +98,7 @@ class AffiliateLinksCleanerTest extends AnyFlatSpec with Matchers { oldPublishedDate, deniedPageUrl, "article", + validHtml, ) should be(false) shouldAddAffiliateLinks( switchedOn = true, @@ -104,6 +111,7 @@ class AffiliateLinksCleanerTest extends AnyFlatSpec with Matchers { oldPublishedDate, deniedPageUrl, "article", + validHtml, ) should be(true) shouldAddAffiliateLinks( switchedOn = true, @@ -116,6 +124,7 @@ class AffiliateLinksCleanerTest extends AnyFlatSpec with Matchers { oldPublishedDate, deniedPageUrl, "article", + validHtml, ) should be(false) shouldAddAffiliateLinks( switchedOn = true, @@ -128,6 +137,7 @@ class AffiliateLinksCleanerTest extends AnyFlatSpec with Matchers { oldPublishedDate, deniedPageUrl, "article", + validHtml, ) should be(true) shouldAddAffiliateLinks( switchedOn = true, @@ -140,6 +150,7 @@ class AffiliateLinksCleanerTest extends AnyFlatSpec with Matchers { newPublishedDate, deniedPageUrl, "article", + validHtml, ) should be(false) shouldAddAffiliateLinks( switchedOn = true,