From 966f0844e91109ec8aa71d83eeb644299694e1f3 Mon Sep 17 00:00:00 2001 From: Dominik Lander Date: Thu, 19 Oct 2023 14:47:09 +0100 Subject: [PATCH 1/3] Add classname to fronts-banner top container --- dotcom-rendering/src/components/AdSlot.web.tsx | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/dotcom-rendering/src/components/AdSlot.web.tsx b/dotcom-rendering/src/components/AdSlot.web.tsx index 976145bfd53..a0535afb2c9 100644 --- a/dotcom-rendering/src/components/AdSlot.web.tsx +++ b/dotcom-rendering/src/components/AdSlot.web.tsx @@ -578,13 +578,16 @@ export const AdSlot = ({ case 'fronts-banner': { const advertId = `fronts-banner-${index}`; return ( -
+
Date: Thu, 19 Oct 2023 14:58:46 +0100 Subject: [PATCH 2/3] Remove network fronts-banner AB test. --- dotcom-rendering/src/layouts/FrontLayout.tsx | 20 ++------- .../src/layouts/lib/decideAdSlots.test.ts | 11 ----- .../src/layouts/lib/decideAdSlots.tsx | 44 ++++++------------- .../src/lib/commercial-constants.ts | 4 -- 4 files changed, 17 insertions(+), 62 deletions(-) delete mode 100644 dotcom-rendering/src/lib/commercial-constants.ts diff --git a/dotcom-rendering/src/layouts/FrontLayout.tsx b/dotcom-rendering/src/layouts/FrontLayout.tsx index f1f8aebf9bb..baca28d65fc 100644 --- a/dotcom-rendering/src/layouts/FrontLayout.tsx +++ b/dotcom-rendering/src/layouts/FrontLayout.tsx @@ -125,7 +125,6 @@ export const FrontLayout = ({ front, NAV }: Props) => { const { config: { switches, - abTests, isPaidContent, hasPageSkin: hasPageSkinConfig, pageId, @@ -138,15 +137,9 @@ export const FrontLayout = ({ front, NAV }: Props) => { const showBannerAds = !!switches.frontsBannerAds; - const isInFrontsBannerTest = - !!switches.frontsBannerAdsDcr && - abTests.frontsBannerAdsDcrVariant === 'variant' && - Object.keys(frontsBannerAdCollections).includes(pageId); - - const frontsBannerTargetedCollections = - isInFrontsBannerTest || showBannerAds - ? frontsBannerAdCollections[pageId] - : []; + const frontsBannerTargetedCollections = showBannerAds + ? frontsBannerAdCollections[pageId] + : []; const merchHighPosition = getMerchHighPosition( front.pressedPage.collections.length, @@ -162,7 +155,7 @@ export const FrontLayout = ({ front, NAV }: Props) => { const numBannerAdsInserted = useRef(0); - const renderMpuAds = renderAds && !isInFrontsBannerTest && !showBannerAds; + const renderMpuAds = renderAds && !showBannerAds; const showMostPopular = front.config.switches.deeplyRead && @@ -362,7 +355,6 @@ export const FrontLayout = ({ front, NAV }: Props) => { index, pageId, collection.displayName, - isInFrontsBannerTest, frontsBannerTargetedCollections, )} {!!trail.embedUri && ( @@ -426,7 +418,6 @@ export const FrontLayout = ({ front, NAV }: Props) => { index, pageId, collection.displayName, - isInFrontsBannerTest, frontsBannerTargetedCollections, )} { index, pageId, collection.displayName, - isInFrontsBannerTest, frontsBannerTargetedCollections, )} { index, pageId, collection.displayName, - isInFrontsBannerTest, frontsBannerTargetedCollections, )}
{ index, pageId, collection.displayName, - isInFrontsBannerTest, frontsBannerTargetedCollections, )} { const index = 2; const pageId = 'uk'; const collectionName = 'Sport'; - const isInFrontsBannerTest = false; const targetedCollections = [ 'Spotlight', 'Opinion', @@ -63,7 +62,6 @@ describe('decideFrontsBannerAdSlot', () => { index, pageId, collectionName, - isInFrontsBannerTest, targetedCollections, ); @@ -79,7 +77,6 @@ describe('decideFrontsBannerAdSlot', () => { index, pageId, collectionName, - isInFrontsBannerTest, targetedCollections, ); @@ -95,7 +92,6 @@ describe('decideFrontsBannerAdSlot', () => { index, pageId, collectionName, - isInFrontsBannerTest, targetedCollections, ); @@ -111,7 +107,6 @@ describe('decideFrontsBannerAdSlot', () => { index, pageId, collectionName, - isInFrontsBannerTest, targetedCollections, ); @@ -128,7 +123,6 @@ describe('decideFrontsBannerAdSlot', () => { index, pageId, 'Sport', - isInFrontsBannerTest, targetedCollections, ); @@ -144,7 +138,6 @@ describe('decideFrontsBannerAdSlot', () => { index, pageId, 'Not a collection', - isInFrontsBannerTest, targetedCollections, ); @@ -169,7 +162,6 @@ describe('decideFrontsBannerAdSlot', () => { i, 'society', collectionName, - isInFrontsBannerTest, targetedCollections, ); @@ -192,7 +184,6 @@ describe('decideFrontsBannerAdSlot', () => { index, 'myPage', 'Excluded collection', - isInFrontsBannerTest, [], ); @@ -208,7 +199,6 @@ describe('decideFrontsBannerAdSlot', () => { index, 'society', collectionName, - isInFrontsBannerTest, targetedCollections, ); @@ -224,7 +214,6 @@ describe('decideFrontsBannerAdSlot', () => { index, 'society', collectionName, - isInFrontsBannerTest, targetedCollections, ); diff --git a/dotcom-rendering/src/layouts/lib/decideAdSlots.tsx b/dotcom-rendering/src/layouts/lib/decideAdSlots.tsx index cdc1c11be0f..dadfd4fdb30 100644 --- a/dotcom-rendering/src/layouts/lib/decideAdSlots.tsx +++ b/dotcom-rendering/src/layouts/lib/decideAdSlots.tsx @@ -1,10 +1,14 @@ import { Hide } from '@guardian/source-react-components'; import { AdSlot } from '../../components/AdSlot.web'; -import { MAX_FRONTS_BANNER_ADS } from '../../lib/commercial-constants'; import { frontsBannerAdCollections } from '../../lib/frontsBannerAbTestAdPositions'; import { frontsBannerExcludedCollections } from '../../lib/frontsBannerExclusions'; import { getMerchHighPosition } from '../../lib/getAdPositions'; +/** + * The maximum number of fronts-banner ads that can be inserted on any front. + */ +export const MAX_FRONTS_BANNER_ADS = 6; + export const decideMerchHighAndMobileAdSlots = ( renderAds: boolean, index: number, @@ -12,7 +16,7 @@ export const decideMerchHighAndMobileAdSlots = ( isPaidContent: boolean | undefined, mobileAdPositions: (number | undefined)[], hasPageSkin: boolean, - showBannerAds?: boolean, + showBannerAds: boolean, ) => { if (!renderAds) return null; @@ -64,39 +68,18 @@ export const decideFrontsBannerAdSlot = ( index: number, pageId: string, collectionName: string | null, - isInFrontsBannerTest?: boolean, targetedCollections?: string[], ) => { const isFirstContainer = index === 0; - if (!renderAds || hasPageSkin || isFirstContainer) { + if (!renderAds || hasPageSkin || isFirstContainer || !showBannerAds) { return null; } - // The fronts banner 0% AB test has concluded. However, it still exists so that - // the commercial team can opt in and test ad campaigns against the live site. - // In this test, fronts-banner ads are inserted above specific collections and pages. - // - // This code block can be deleted when fronts banner ads are live and the FrontsBannerTest is removed. - if (isInFrontsBannerTest && collectionName) { - if (targetedCollections?.includes(collectionName)) { - numBannerAdsInserted.current = numBannerAdsInserted.current + 1; - - return ( - - ); - } - - // If the showBannerAds feature switch is on and the page was included in the AB test, then - // show ads above the collections as specified in that AB test. One reason is that on /uk - // the "Ukraine invasion" collection is third and we don't want to place an ad above this - // container, which means we don't get a banner ad until the 6th collection. - } else if ( - showBannerAds && + // If the showBannerAds feature switch is on and the page was included in the AB test, then + // show ads above the collections as specified in that AB test. One reason is that on /uk + // the "Ukraine invasion" collection is third and we don't want to place an ad above this + // container, which means we don't get a banner ad until the 6th collection. + if ( collectionName && Object.keys(frontsBannerAdCollections).includes(pageId) ) { @@ -117,9 +100,8 @@ export const decideFrontsBannerAdSlot = ( // Insert an ad after every third collection. Warning: may skip an ad if a collection isn't rendered. // e.g. if the 15th collection doesn't render, an ad is shown above the 12th and the 18th, skipping the 15th. else if ( - showBannerAds && numBannerAdsInserted.current < MAX_FRONTS_BANNER_ADS && - index % 3 === 2 && // Insert above the 3rd, 6th, ..., container + (index + 1) % 3 === 0 && // Insert ad above the 3rd, 6th, ..., container. Index starts at zero. !isCollectionExclusionPresent(pageId, collectionName) ) { numBannerAdsInserted.current = numBannerAdsInserted.current + 1; diff --git a/dotcom-rendering/src/lib/commercial-constants.ts b/dotcom-rendering/src/lib/commercial-constants.ts deleted file mode 100644 index abd46271a23..00000000000 --- a/dotcom-rendering/src/lib/commercial-constants.ts +++ /dev/null @@ -1,4 +0,0 @@ -/** - * The maximum number of fronts-banner ads that can be inserted on any front. - */ -export const MAX_FRONTS_BANNER_ADS = 6; From 23b84bf05572964ae3b276fc0e6913d8927299a6 Mon Sep 17 00:00:00 2001 From: Dominik Lander Date: Thu, 19 Oct 2023 16:52:07 +0100 Subject: [PATCH 3/3] Remove frontsBannerAdsDcr from cypress fixture --- dotcom-rendering/cypress/fixtures/manual/standard-article.js | 1 - 1 file changed, 1 deletion(-) diff --git a/dotcom-rendering/cypress/fixtures/manual/standard-article.js b/dotcom-rendering/cypress/fixtures/manual/standard-article.js index 3f6544c801e..2082cf81800 100644 --- a/dotcom-rendering/cypress/fixtures/manual/standard-article.js +++ b/dotcom-rendering/cypress/fixtures/manual/standard-article.js @@ -1413,7 +1413,6 @@ export const Standard = { twitterUwt: true, prebidAppnexusInvcode: true, ampPrebidPubmatic: true, - frontsBannerAdsDcr: true, a9HeaderBidding: true, prebidAppnexus: true, enableDiscussionSwitch: true,