Skip to content

Commit

Permalink
Merge pull request #9047 from guardian/doml/remove-network-fronts-ban…
Browse files Browse the repository at this point in the history
…ner-test

Remove network fronts banner ads ab test
  • Loading branch information
domlander authored Oct 19, 2023
2 parents b1c13a8 + 23b84bf commit c591fca
Show file tree
Hide file tree
Showing 6 changed files with 22 additions and 65 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -1413,7 +1413,6 @@ export const Standard = {
twitterUwt: true,
prebidAppnexusInvcode: true,
ampPrebidPubmatic: true,
frontsBannerAdsDcr: true,
a9HeaderBidding: true,
prebidAppnexus: true,
enableDiscussionSwitch: true,
Expand Down
7 changes: 5 additions & 2 deletions dotcom-rendering/src/components/AdSlot.web.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -578,13 +578,16 @@ export const AdSlot = ({
case 'fronts-banner': {
const advertId = `fronts-banner-${index}`;
return (
<div css={frontsBannerAdTopContainerStyles}>
<div
className="top-fronts-banner-ad-container"
css={frontsBannerAdTopContainerStyles}
>
<div
className="ad-slot-container"
css={[
adContainerStyles,
frontsBannerAdContainerStyles,
hasPageskin && frontsBannerCollapseStyles,
adContainerStyles,
]}
>
<div
Expand Down
20 changes: 4 additions & 16 deletions dotcom-rendering/src/layouts/FrontLayout.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,6 @@ export const FrontLayout = ({ front, NAV }: Props) => {
const {
config: {
switches,
abTests,
isPaidContent,
hasPageSkin: hasPageSkinConfig,
pageId,
Expand All @@ -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,
Expand All @@ -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 &&
Expand Down Expand Up @@ -362,7 +355,6 @@ export const FrontLayout = ({ front, NAV }: Props) => {
index,
pageId,
collection.displayName,
isInFrontsBannerTest,
frontsBannerTargetedCollections,
)}
{!!trail.embedUri && (
Expand Down Expand Up @@ -426,7 +418,6 @@ export const FrontLayout = ({ front, NAV }: Props) => {
index,
pageId,
collection.displayName,
isInFrontsBannerTest,
frontsBannerTargetedCollections,
)}
<FrontSection
Expand Down Expand Up @@ -503,7 +494,6 @@ export const FrontLayout = ({ front, NAV }: Props) => {
index,
pageId,
collection.displayName,
isInFrontsBannerTest,
frontsBannerTargetedCollections,
)}
<LabsSection
Expand Down Expand Up @@ -572,7 +562,6 @@ export const FrontLayout = ({ front, NAV }: Props) => {
index,
pageId,
collection.displayName,
isInFrontsBannerTest,
frontsBannerTargetedCollections,
)}
<Section
Expand Down Expand Up @@ -649,7 +638,6 @@ export const FrontLayout = ({ front, NAV }: Props) => {
index,
pageId,
collection.displayName,
isInFrontsBannerTest,
frontsBannerTargetedCollections,
)}
<FrontSection
Expand Down
11 changes: 0 additions & 11 deletions dotcom-rendering/src/layouts/lib/decideAdSlots.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,6 @@ describe('decideFrontsBannerAdSlot', () => {
const index = 2;
const pageId = 'uk';
const collectionName = 'Sport';
const isInFrontsBannerTest = false;
const targetedCollections = [
'Spotlight',
'Opinion',
Expand All @@ -63,7 +62,6 @@ describe('decideFrontsBannerAdSlot', () => {
index,
pageId,
collectionName,
isInFrontsBannerTest,
targetedCollections,
);

Expand All @@ -79,7 +77,6 @@ describe('decideFrontsBannerAdSlot', () => {
index,
pageId,
collectionName,
isInFrontsBannerTest,
targetedCollections,
);

Expand All @@ -95,7 +92,6 @@ describe('decideFrontsBannerAdSlot', () => {
index,
pageId,
collectionName,
isInFrontsBannerTest,
targetedCollections,
);

Expand All @@ -111,7 +107,6 @@ describe('decideFrontsBannerAdSlot', () => {
index,
pageId,
collectionName,
isInFrontsBannerTest,
targetedCollections,
);

Expand All @@ -128,7 +123,6 @@ describe('decideFrontsBannerAdSlot', () => {
index,
pageId,
'Sport',
isInFrontsBannerTest,
targetedCollections,
);

Expand All @@ -144,7 +138,6 @@ describe('decideFrontsBannerAdSlot', () => {
index,
pageId,
'Not a collection',
isInFrontsBannerTest,
targetedCollections,
);

Expand All @@ -169,7 +162,6 @@ describe('decideFrontsBannerAdSlot', () => {
i,
'society',
collectionName,
isInFrontsBannerTest,
targetedCollections,
);

Expand All @@ -192,7 +184,6 @@ describe('decideFrontsBannerAdSlot', () => {
index,
'myPage',
'Excluded collection',
isInFrontsBannerTest,
[],
);

Expand All @@ -208,7 +199,6 @@ describe('decideFrontsBannerAdSlot', () => {
index,
'society',
collectionName,
isInFrontsBannerTest,
targetedCollections,
);

Expand All @@ -224,7 +214,6 @@ describe('decideFrontsBannerAdSlot', () => {
index,
'society',
collectionName,
isInFrontsBannerTest,
targetedCollections,
);

Expand Down
44 changes: 13 additions & 31 deletions dotcom-rendering/src/layouts/lib/decideAdSlots.tsx
Original file line number Diff line number Diff line change
@@ -1,18 +1,22 @@
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,
collectionCount: number,
isPaidContent: boolean | undefined,
mobileAdPositions: (number | undefined)[],
hasPageSkin: boolean,
showBannerAds?: boolean,
showBannerAds: boolean,
) => {
if (!renderAds) return null;

Expand Down Expand Up @@ -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 (
<AdSlot
data-print-layout="hide"
position="fronts-banner"
index={numBannerAdsInserted.current}
hasPageskin={hasPageSkin}
/>
);
}

// 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)
) {
Expand All @@ -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;
Expand Down
4 changes: 0 additions & 4 deletions dotcom-rendering/src/lib/commercial-constants.ts

This file was deleted.

0 comments on commit c591fca

Please sign in to comment.