Skip to content

Commit

Permalink
Merge remote-tracking branch 'origin/main' into clj/spike-deploy-with…
Browse files Browse the repository at this point in the history
…-riff-raff
  • Loading branch information
Jakeii committed Nov 15, 2024
2 parents b3af543 + 25f3376 commit 35aaf7c
Show file tree
Hide file tree
Showing 29 changed files with 346 additions and 56 deletions.
5 changes: 0 additions & 5 deletions .changeset/empty-ghosts-film.md

This file was deleted.

28 changes: 28 additions & 0 deletions .github/workflows/stale.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
name: 'Stale PR Handler'

on:
schedule:
# Every morning at 1AM, Mondays to Fridays
- cron: '0 1 * * MON-FRI'

permissions:
pull-requests: write

jobs:
stale:
runs-on: ubuntu-latest
steps:
- uses: actions/stale@v9
id: stale
# Read about options here: https://github.com/actions/stale#all-options
with:
# never automatically mark issues as stale
days-before-issue-stale: -1
days-before-stale: 30
stale-pr-message: >
"This PR is stale because it has been open 30 days with no activity.
Unless a comment is added or the “stale” label removed, this will be closed in 3 days"
days-before-close: 3
close-pr-message: 'This PR was closed because it has been stalled for 3 days with no activity.'
delete-branch: true
exempt-pr-labels: 'dependencies'
27 changes: 27 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,32 @@
# @guardian/commercial

## 23.5.0

### Minor Changes

- 9c0e78d: Fix persisting of forced ab tests to localstorage
- e917f96: Add targeting keyword to indicate teads eligibility

## 23.4.0

### Minor Changes

- 877f516: Setup test for new prebid analytics endpoint

## 23.3.0

### Minor Changes

- 1efa8ca: Re-enable Criteo in Australia and New Zealand
- e8a8717: Filter out static top-above-nav on consentless mobile
- 0cd37f5: block GumGum on fronts

## 23.2.1

### Patch Changes

- d1ab338: add mobile banner to interactives

## 23.2.0

### Minor Changes
Expand Down
12 changes: 7 additions & 5 deletions docs/ab-testing/readme.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,17 +4,17 @@

### Setup

1. Follow steps 1-6 in [the DCR documentation](https://github.com/guardian/frontend/blob/main/common/app/conf/switches/ABTestSwitches.scala)
1. Create a test in [src/experiments/tests](https://github.com/guardian/commercial-core/blob/main/src/experiments/tests)
1. Add the test to [concurrent tests](https://github.com/guardian/commercial-core/blob/main/src/experiments/ab-tests.ts)
1. Follow steps 1-6 in [the DCR documentation](https://github.com/guardian/dotcom-rendering/blob/main/dotcom-rendering/docs/development/ab-testing-in-dcr.md)
1. Create a test in [src/experiments/tests](https://github.com/guardian/commercial/blob/main/src/experiments/tests)
1. Add the test to [concurrent tests](https://github.com/guardian/commercial/blob/main/src/experiments/ab-tests.ts)

### Example usage

```ts
import { isInVariantSynchronous } from 'experiments/ab';
import { isUserInVariant } from 'experiments/ab';
import { sectionAdDensity } from 'experiments/tests/section-ad-density';

const isInVariant = isInVariantSynchronous(sectionAdDensity, 'variant');
const isInVariant = isUserInVariant(sectionAdDensity, 'variant');
```

### How to test
Expand All @@ -23,6 +23,8 @@ Use the URL opt-in link to force yourself into a particular variant, e.g. `http:

If you test has multiple variants, you can test each one by updating the `yourVariant` part of the above URL.

To remove yourself from the test unset the varaint e.g. `http://localhost:3030/Front/https://www.theguardian.com/uk#ab-yourTest=`

## Server-side tests

### Setup
Expand Down
4 changes: 2 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "@guardian/commercial",
"version": "23.2.0",
"version": "23.5.0",
"description": "Guardian advertising business logic",
"homepage": "https://github.com/guardian/commercial#readme",
"bugs": {
Expand Down Expand Up @@ -104,7 +104,7 @@
},
"dependencies": {
"@changesets/cli": "^2.26.2",
"@guardian/prebid.js": "8.52.0-7",
"@guardian/prebid.js": "8.52.0-8",
"@octokit/core": "^6.1.2",
"fastdom": "^1.0.11",
"lodash-es": "^4.17.21",
Expand Down
2 changes: 1 addition & 1 deletion playwright/lib/gam.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import type { Page, Request, Response } from '@playwright/test';

const gamUrl = /https:\/\/securepubads.g.doubleclick.net\/gampad\/ads/;
const gamUrl = /https:\/\/securepubads\.g\.doubleclick\.net\/gampad\/ads/;

const getEncodedParamsFromRequest = (
request: Request,
Expand Down
8 changes: 4 additions & 4 deletions pnpm-lock.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 6 additions & 0 deletions src/core/lib/ab-localstorage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,3 +19,9 @@ export const getParticipationsFromLocalStorage = (): Participations => {
const participations = storage.local.get(participationsKey);
return isParticipations(participations) ? participations : {};
};

export const setParticipationsInLocalStorage = (
participations: Participations,
): void => {
storage.local.set(participationsKey, participations);
};
8 changes: 8 additions & 0 deletions src/core/messenger/passback-refresh.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,18 @@ import { refreshAdvert } from '../../display/load-advert';
import { getAdvertById } from '../../lib/dfp/get-advert-by-id';
import type { RegisterListener } from '../messenger';

const ineligiblePassbacks = ['teadsdesktop', 'teadsmobile', 'teads'];

const passbackRefresh = (specs: string, adSlot: HTMLElement) => {
const advert = getAdvertById(adSlot.id);
if (advert) {
advert.slot.setTargeting('passback', specs);

// passbacks with these values are not eligible for teads
if (ineligiblePassbacks.includes(specs)) {
advert.slot.setTargeting('teadsEligible', 'false');
}

refreshAdvert(advert);
}
};
Expand Down
97 changes: 97 additions & 0 deletions src/core/targeting/teads-eligibility.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,97 @@
import { isEligibleForTeads } from './teads-eligibility';

// Mocking the IAS keywords
const pubAds = {
getTargeting: jest.fn(() => ['']),
};

window.googletag = {
/* @ts-expect-error -- no way to override types */
pubads() {
return pubAds;
},
};

describe('Teads Eligibility', () => {
it('should be eligible for teads when slot is inline1, on an allowed content type, not sensitive, and there are no banned keywords', () => {
window.guardian = {
config: {
page: {
contentType: 'Article',
isSensitive: false,
pageId: 'lifeandstyle/2024/sep/30/the-electrolytes-boom-a-wonder-supplement-or-an-unnecessary-expense',
} as unknown as typeof window.guardian.config.page,
},
} as typeof window.guardian;

expect(isEligibleForTeads('dfp-ad--inline1')).toBe(true);
});

it('should not be eligible for teads when slot is not inline1', () => {
window.guardian = {
config: {
page: {
contentType: 'Article',
isSensitive: false,
pageId: 'lifeandstyle/2024/sep/30/the-electrolytes-boom-a-wonder-supplement-or-an-unnecessary-expense',
} as unknown as typeof window.guardian.config.page,
},
} as typeof window.guardian;

expect(isEligibleForTeads('dfp-ad--inline2')).toBe(false);
});

it('should not be eligible for teads when content type is not article or liveblog', () => {
window.guardian = {
config: {
page: {
contentType: 'Interactive',
isSensitive: false,
pageId: 'books/ng-interactive/2024/sep/04/this-months-best-paperbacks-stephen-king-anne-michaels-and-more',
} as unknown as typeof window.guardian.config.page,
},
} as typeof window.guardian;

expect(isEligibleForTeads('dfp-ad--inline1')).toBe(false);
});

it('should not be eligible for teads when content is marked as sensitive', () => {
window.guardian = {
config: {
page: {
contentType: 'Article',
isSensitive: true,
pageId: 'society/2020/oct/08/take-life-grateful-alive-surgeon-suicide-attempt',
} as unknown as typeof window.guardian.config.page,
},
} as typeof window.guardian;

expect(isEligibleForTeads('dfp-ad--inline1')).toBe(false);
});

it('should not be eligible for teads when IAS indicates that content is not brand safe', () => {
// Mocking the IAS keywords - need to mock a non brand safe article
const pubAds = {
getTargeting: jest.fn(() => ['IAS_16425_KW']),
};

window.googletag = {
/* @ts-expect-error -- no way to override types */
pubads() {
return pubAds;
},
};

window.guardian = {
config: {
page: {
contentType: 'Article',
isSensitive: false,
pageId: 'us-news/2024/nov/10/trump-putin-ukraine-war',
} as unknown as typeof window.guardian.config.page,
},
} as typeof window.guardian;

expect(isEligibleForTeads('dfp-ad--inline1')).toBe(false);
});
});
24 changes: 24 additions & 0 deletions src/core/targeting/teads-eligibility.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
const allowedContentTypes = ['Article', 'LiveBlog'];

const isEligibleForTeads = (slotId: string) => {
const { contentType, isSensitive } = window.guardian.config.page;

// This IAS value is returned when a page is thought to contain content which is not brand safe
const isBrandSafe = !window.googletag
.pubads()
.getTargeting('ias-kw')
.includes('IAS_16425_KW');

if (
slotId === 'dfp-ad--inline1' &&
allowedContentTypes.includes(contentType) &&
!isSensitive &&
isBrandSafe
) {
return true;
}

return false;
};

export { isEligibleForTeads };
4 changes: 4 additions & 0 deletions src/define/Advert.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,10 @@ jest.mock('core/ad-sizes', () => {
};
});

jest.mock('core/targeting/teads-eligibility', () => ({
isEligibleForTeads: jest.fn(),
}));

describe('Advert', () => {
let googleSlot: googletag.Slot;

Expand Down
4 changes: 4 additions & 0 deletions src/define/define-slot.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,10 @@ jest.mock('define/init-slot-ias', () => ({
initSlotIas: jest.fn(() => Promise.resolve()),
}));

jest.mock('core/targeting/teads-eligibility', () => ({
isEligibleForTeads: jest.fn(),
}));

beforeEach(() => {
const pubAds = {
setTargeting: jest.fn(),
Expand Down
10 changes: 10 additions & 0 deletions src/define/define-slot.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { breakpoints as sourceBreakpoints } from '@guardian/source/foundations';
import { once } from 'lodash-es';
import type { SizeMapping, SlotName } from '../core/ad-sizes';
import { EventTimer } from '../core/index';
import { isEligibleForTeads } from '../core/targeting/teads-eligibility';
import { toGoogleTagSize } from '../utils/googletag-ad-size';
import { getUrlVars } from '../utils/url';
import { initSlotIas } from './init-slot-ias';
Expand Down Expand Up @@ -151,6 +152,15 @@ const defineSlot = (
void slotReady.then(() => {
EventTimer.get().mark('defineSlotEnd', slotTarget);
EventTimer.get().mark('slotReady', slotTarget);

// wait until IAS has initialised before checking teads eligibility
const isTeadsEligible = isEligibleForTeads(id);

if (isTeadsEligible) {
slot.setTargeting('teadsEligible', 'true');
} else {
slot.setTargeting('teadsEligible', 'false');
}
});

const isbn = window.guardian.config.page.isbn;
Expand Down
3 changes: 3 additions & 0 deletions src/display/load-advert.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,9 @@ export const refreshAdvert = (advert: Advert): void => {
.then(() => {
advert.slot.setTargeting('refreshed', 'true');

// slots that have refreshed are not eligible for teads
advert.slot.setTargeting('teadsEligible', 'false');

if (advert.id === 'dfp-ad--top-above-nav') {
// force the slot sizes to be the same as advert.size (current)
// only when advert.size is an array (forget 'fluid' and other specials)
Expand Down
2 changes: 2 additions & 0 deletions src/experiments/ab-tests.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import type { ABTest } from '@guardian/ab-core';
import { mpuWhenNoEpic } from './tests/mpu-when-no-epic';
import { newHeaderBiddingEndpoint } from './tests/new-header-bidding-endpoint';
import { optOutFrequencyCap } from './tests/opt-out-frequency-cap';

/**
Expand All @@ -11,4 +12,5 @@ export const concurrentTests: ABTest[] = [
// one test per line
mpuWhenNoEpic,
optOutFrequencyCap,
newHeaderBiddingEndpoint,
];
14 changes: 10 additions & 4 deletions src/experiments/ab-url.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,18 @@ export const getForcedParticipationsFromUrl = (): Participations => {

return tokens.reduce((obj, token) => {
const [testId, variantId] = token.split('=');
if (testId && variantId) {
if (testId) {
if (variantId) {
return {
...obj,
[testId]: {
variant: variantId,
},
};
}
return {
...obj,
[testId]: {
variant: variantId,
},
[testId]: undefined,
};
}
return obj;
Expand Down
Loading

0 comments on commit 35aaf7c

Please sign in to comment.