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

Remove network fronts banner ads ab test #9047

Merged
merged 3 commits into from
Oct 19, 2023
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
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.