From 7f505dc1b198e7983cca83ba7119649960306868 Mon Sep 17 00:00:00 2001 From: Tom Forbes Date: Mon, 7 Oct 2024 10:14:56 +0100 Subject: [PATCH 1/5] Persist landing page test tracking --- .../assets/helpers/abTests/abtest.ts | 18 ++++++++++++++++++ .../twoStepPages/threeTierLanding.tsx | 3 +++ 2 files changed, 21 insertions(+) diff --git a/support-frontend/assets/helpers/abTests/abtest.ts b/support-frontend/assets/helpers/abTests/abtest.ts index 17760ee934..894562f60a 100644 --- a/support-frontend/assets/helpers/abTests/abtest.ts +++ b/support-frontend/assets/helpers/abTests/abtest.ts @@ -8,6 +8,7 @@ import type { Settings } from 'helpers/globalsAndSwitches/settings'; import type { IsoCountry } from 'helpers/internationalisation/country'; import type { CountryGroupId } from 'helpers/internationalisation/countryGroup'; import * as cookie from 'helpers/storage/cookie'; +import * as storage from 'helpers/storage/storage'; import { getQueryParameter } from 'helpers/urls/url'; import type { AmountsTest, @@ -109,6 +110,7 @@ function init({ mvt = getMvtId(), acquisitionDataTests = getTestFromAcquisitionData() ?? [], }: ABtestInitalizerData): Participations { + const sessionParticipations = getParticipationsFromSession(); const participations = getParticipations( abTests, mvt, @@ -120,6 +122,7 @@ function init({ const urlParticipations = getParticipationsFromUrl(); const serverSideParticipations = getServerSideParticipations(); return { + ...sessionParticipations, ...participations, ...serverSideParticipations, ...urlParticipations, @@ -237,6 +240,21 @@ function getParticipationsFromUrl(): Participations | undefined { return; } +function getParticipationsFromSession(): Participations | undefined { + const participations = storage.getSession('abParticipations'); + if (participations) { + try { + return JSON.parse(participations) as Participations; + } catch (error) { + console.error( + 'Failed to parse abParticipations from session storage', + error, + ); + return undefined; + } + } +} + function getServerSideParticipations(): Participations | null | undefined { if (window.guardian.serversideTests) { return window.guardian.serversideTests; diff --git a/support-frontend/assets/pages/supporter-plus-landing/twoStepPages/threeTierLanding.tsx b/support-frontend/assets/pages/supporter-plus-landing/twoStepPages/threeTierLanding.tsx index 358342f4bd..ba0d9e60aa 100644 --- a/support-frontend/assets/pages/supporter-plus-landing/twoStepPages/threeTierLanding.tsx +++ b/support-frontend/assets/pages/supporter-plus-landing/twoStepPages/threeTierLanding.tsx @@ -47,6 +47,7 @@ import { import type { BillingPeriod } from 'helpers/productPrice/billingPeriods'; import type { Promotion } from 'helpers/productPrice/promotions'; import { getPromotion } from 'helpers/productPrice/promotions'; +import * as storage from 'helpers/storage/storage'; import type { GeoId } from 'pages/geoIdConfig'; import { getGeoIdConfig } from 'pages/geoIdConfig'; import { getCampaignSettings } from '../../../helpers/campaigns/campaigns'; @@ -285,6 +286,8 @@ export function ThreeTierLanding({ }; const abParticipations = abTestInit({ countryId, countryGroupId }); + // Persist any tests for tracking in the checkout page + storage.setSession('abParticipations', JSON.stringify(abParticipations)); const campaignSettings = getCampaignSettings(countryGroupId); From 8b207757addfbe9ded40c5af476659e59213ffb9 Mon Sep 17 00:00:00 2001 From: Tom Forbes Date: Wed, 16 Oct 2024 11:42:16 +0100 Subject: [PATCH 2/5] WIP --- .../helpers/abTests/__tests__/abtestTest.ts | 111 ++++++++++++++++++ .../assets/helpers/abTests/abtest.ts | 19 ++- .../helpers/abTests/abtestDefinitions.ts | 22 ++++ 3 files changed, 151 insertions(+), 1 deletion(-) diff --git a/support-frontend/assets/helpers/abTests/__tests__/abtestTest.ts b/support-frontend/assets/helpers/abTests/__tests__/abtestTest.ts index 5086b844ad..7abca7734d 100644 --- a/support-frontend/assets/helpers/abTests/__tests__/abtestTest.ts +++ b/support-frontend/assets/helpers/abTests/__tests__/abtestTest.ts @@ -19,6 +19,7 @@ import { } from '../../internationalisation/countryGroup'; import { _, init as abInit, getAmountsTestVariant } from '../abtest'; import type { Audience, Participations, Test, Variant } from '../abtest'; +import {storage} from "@guardian/libs"; const { targetPageMatches } = _; const { subsDigiSubPages, digiSub } = pageUrlRegexes.subscriptions; @@ -51,6 +52,7 @@ describe('init', () => { afterEach(() => { window.localStorage.clear(); + window.sessionStorage.clear(); }); it('assigns a user to a variant', () => { @@ -400,6 +402,111 @@ describe('init', () => { expect(participations).toEqual({ t1: 'variant' }); }); }); + + describe('path matching', () => { + beforeEach(() => { + window.sessionStorage.clear(); + delete window.location; + Object.defineProperty(window, 'location', { + value: { + // href: 'https://support.theguardian.com/uk/contribute', + // pathname: '/uk/contribute' + }, + }); + }); + + it('does not assign to test if targetPage does not match', () => { + Object.defineProperty(window, 'location', { + value: { + href: 'https://support.theguardian.com/uk/contribute', + pathname: '/uk/contribute' + }, + }); + + const abTests = { + t1: buildTest({ + targetPage: '/us/contribute$', + }), + }; + + const participations: Participations = abInit({ + ...abtestInitalizerData, + abTests, + }); + + expect(participations).toEqual({}); + }); + + it('assign to test if targetPage matches', () => { + Object.defineProperty(window, 'location', { + value: { + href: 'https://support.theguardian.com/uk/contribute', + pathname: '/uk/contribute' + }, + }); + + const abTests = { + t1: buildTest({ + targetPage: '/uk/contribute$', + }), + }; + + const participations: Participations = abInit({ + ...abtestInitalizerData, + abTests, + }); + + expect(participations).toEqual({ t1: 'control' }); + }); + + it('assign to test if persistPage matches and test is in session storage', () => { + window.sessionStorage.setItem('abParticipations', JSON.stringify({t1: 'control'})); + Object.defineProperty(window, 'location', { + value: { + href: 'https://support.theguardian.com/uk/checkout', + pathname: '/uk/checkout' + }, + }); + + const abTests = { + t1: buildTest({ + targetPage: '/uk/contribute$', + persistPage: '/uk/checkout$', + }), + }; + + const participations: Participations = abInit({ + ...abtestInitalizerData, + abTests, + }); + + expect(participations).toEqual({ t1: 'control' }); + }); + + it('does not assign to test if persistPage does not match and test is in session storage', () => { + window.sessionStorage.setItem('abParticipations', JSON.stringify({t1: 'control'})); + Object.defineProperty(window, 'location', { + value: { + href: 'https://support.theguardian.com/uk/blah', + pathname: '/uk/blah' + }, + }); + + const abTests = { + t1: buildTest({ + targetPage: '/uk/contribute$', + persistPage: '/uk/checkout$', + }), + }; + + const participations: Participations = abInit({ + ...abtestInitalizerData, + abTests, + }); + + expect(participations).toEqual({ }); + }); + }); }); it('targetPage matching', () => { @@ -805,6 +912,8 @@ function buildTest({ seed = 0, excludeIfInReferrerControlledTest = false, excludeCountriesSubjectToContributionsOnlyAmounts = true, + targetPage = undefined, + persistPage = undefined, }: Partial): Test { return { variants, @@ -814,6 +923,8 @@ function buildTest({ seed, excludeIfInReferrerControlledTest, excludeCountriesSubjectToContributionsOnlyAmounts, + targetPage, + persistPage, }; } diff --git a/support-frontend/assets/helpers/abTests/abtest.ts b/support-frontend/assets/helpers/abTests/abtest.ts index 894562f60a..7ddf8ff7fe 100644 --- a/support-frontend/assets/helpers/abTests/abtest.ts +++ b/support-frontend/assets/helpers/abTests/abtest.ts @@ -82,6 +82,8 @@ export type Test = { // An optional regex that will be tested against the path of the current page // before activating this test eg. '/(uk|us|au|ca|nz)/subscribe$' targetPage?: string | RegExp; + // Persist this test participation across more pages using this regex + persistPage?: string | RegExp; omitCountries?: IsoCountry[]; // Some users will see a version of the checkout that only offers // the option to make contributions. We won't want to include these @@ -118,11 +120,12 @@ function init({ countryGroupId, acquisitionDataTests, selectedAmountsVariant, + sessionParticipations, ); + const urlParticipations = getParticipationsFromUrl(); const serverSideParticipations = getServerSideParticipations(); return { - ...sessionParticipations, ...participations, ...serverSideParticipations, ...urlParticipations, @@ -159,6 +162,7 @@ function getParticipations( countryGroupId: CountryGroupId, acquisitionDataTests?: AcquisitionABTest[], selectedAmountsVariant?: SelectedAmountsVariant, + sessionParticipations: Participations = {}, ): Participations { const participations: Participations = {}; @@ -175,6 +179,19 @@ function getParticipations( return; } + // Is the user already in this test in the current browser session? + console.log('pathname', window.location.pathname) + console.log('target', test.targetPage) + console.log('persist', test.persistPage) + console.log('window.sessionStorage', window.sessionStorage.getItem('abParticipations')) + if ( + !!sessionParticipations[testId] && + targetPageMatches(window.location.pathname, test.persistPage) + ) { + participations[testId] = sessionParticipations[testId]; + return; + } + if (!targetPageMatches(window.location.pathname, test.targetPage)) { return; } diff --git a/support-frontend/assets/helpers/abTests/abtestDefinitions.ts b/support-frontend/assets/helpers/abTests/abtestDefinitions.ts index 640b1f0bda..789a1441cf 100644 --- a/support-frontend/assets/helpers/abTests/abtestDefinitions.ts +++ b/support-frontend/assets/helpers/abTests/abtestDefinitions.ts @@ -13,6 +13,8 @@ export const pageUrlRegexes = { notUsLandingPage: '/uk|au|eu|int|nz|ca/contribute(/.*)?$', auLandingPage: '/au/contribute(/.*)?$', usLandingPage: '/us/contribute(/.*)?$', + landingPageOnly: '/(uk|us|au|eu|int|nz|ca)/contribute$', + ukCheckout: '/uk/checkout$', }, subscriptions: { subsDigiSubPages: '(/??/subscribe(\\?.*)?$|/??/subscribe/digital(\\?.*)?$)', @@ -171,4 +173,24 @@ export const tests: Tests = { targetPage: pageUrlRegexes.contributions.allLandingPagesAndThankyouPages, excludeCountriesSubjectToContributionsOnlyAmounts: false, }, + landingPageOnlyTest: { + variants: [ + // not really an AB test + { + id: 'variant', + }, + ], + audiences: { + ALL: { + offset: 0, + size: 1, + }, + }, + isActive: true, + referrerControlled: false, + seed: 6, + targetPage: pageUrlRegexes.contributions.landingPageOnly, + persistPage: pageUrlRegexes.contributions.ukCheckout, + excludeCountriesSubjectToContributionsOnlyAmounts: true, + }, }; From 19b93eeb9a40d6fe8f77a7770df0d7ee28a4430e Mon Sep 17 00:00:00 2001 From: Tom Forbes Date: Mon, 21 Oct 2024 10:15:08 +0100 Subject: [PATCH 3/5] fixes --- .../helpers/abTests/__tests__/abtestTest.ts | 196 ++++++++---------- .../assets/helpers/abTests/abtest.ts | 12 +- 2 files changed, 92 insertions(+), 116 deletions(-) diff --git a/support-frontend/assets/helpers/abTests/__tests__/abtestTest.ts b/support-frontend/assets/helpers/abTests/__tests__/abtestTest.ts index 7abca7734d..cf2b40705d 100644 --- a/support-frontend/assets/helpers/abTests/__tests__/abtestTest.ts +++ b/support-frontend/assets/helpers/abTests/__tests__/abtestTest.ts @@ -19,7 +19,6 @@ import { } from '../../internationalisation/countryGroup'; import { _, init as abInit, getAmountsTestVariant } from '../abtest'; import type { Audience, Participations, Test, Variant } from '../abtest'; -import {storage} from "@guardian/libs"; const { targetPageMatches } = _; const { subsDigiSubPages, digiSub } = pageUrlRegexes.subscriptions; @@ -52,7 +51,7 @@ describe('init', () => { afterEach(() => { window.localStorage.clear(); - window.sessionStorage.clear(); + window.sessionStorage.clear(); }); it('assigns a user to a variant', () => { @@ -403,110 +402,87 @@ describe('init', () => { }); }); - describe('path matching', () => { - beforeEach(() => { - window.sessionStorage.clear(); - delete window.location; - Object.defineProperty(window, 'location', { - value: { - // href: 'https://support.theguardian.com/uk/contribute', - // pathname: '/uk/contribute' - }, - }); - }); - - it('does not assign to test if targetPage does not match', () => { - Object.defineProperty(window, 'location', { - value: { - href: 'https://support.theguardian.com/uk/contribute', - pathname: '/uk/contribute' - }, - }); - - const abTests = { - t1: buildTest({ - targetPage: '/us/contribute$', - }), - }; - - const participations: Participations = abInit({ - ...abtestInitalizerData, - abTests, - }); - - expect(participations).toEqual({}); - }); - - it('assign to test if targetPage matches', () => { - Object.defineProperty(window, 'location', { - value: { - href: 'https://support.theguardian.com/uk/contribute', - pathname: '/uk/contribute' - }, - }); - - const abTests = { - t1: buildTest({ - targetPage: '/uk/contribute$', - }), - }; - - const participations: Participations = abInit({ - ...abtestInitalizerData, - abTests, - }); - - expect(participations).toEqual({ t1: 'control' }); - }); - - it('assign to test if persistPage matches and test is in session storage', () => { - window.sessionStorage.setItem('abParticipations', JSON.stringify({t1: 'control'})); - Object.defineProperty(window, 'location', { - value: { - href: 'https://support.theguardian.com/uk/checkout', - pathname: '/uk/checkout' - }, - }); - - const abTests = { - t1: buildTest({ - targetPage: '/uk/contribute$', - persistPage: '/uk/checkout$', - }), - }; - - const participations: Participations = abInit({ - ...abtestInitalizerData, - abTests, - }); - - expect(participations).toEqual({ t1: 'control' }); - }); - - it('does not assign to test if persistPage does not match and test is in session storage', () => { - window.sessionStorage.setItem('abParticipations', JSON.stringify({t1: 'control'})); - Object.defineProperty(window, 'location', { - value: { - href: 'https://support.theguardian.com/uk/blah', - pathname: '/uk/blah' - }, - }); - - const abTests = { - t1: buildTest({ - targetPage: '/uk/contribute$', - persistPage: '/uk/checkout$', - }), - }; - - const participations: Participations = abInit({ - ...abtestInitalizerData, - abTests, - }); - - expect(participations).toEqual({ }); - }); - }); + describe('path matching', () => { + beforeEach(() => { + window.sessionStorage.clear(); + }); + + it('does not assign to test if targetPage does not match', () => { + const abTests = { + t1: buildTest({ + targetPage: '/us/contribute$', + }), + }; + + const participations: Participations = abInit({ + ...abtestInitalizerData, + abTests, + path: '/uk/contribute', + }); + + expect(participations).toEqual({}); + }); + + it('assign to test if targetPage matches', () => { + const abTests = { + t1: buildTest({ + targetPage: '/uk/contribute$', + }), + }; + + const participations: Participations = abInit({ + ...abtestInitalizerData, + abTests, + path: '/uk/contribute', + }); + + expect(participations).toEqual({ t1: 'control' }); + }); + + it('assign to test if persistPage matches and test is in session storage', () => { + window.sessionStorage.setItem( + 'abParticipations', + JSON.stringify({ t1: 'control' }), + ); + + const abTests = { + t1: buildTest({ + targetPage: '/uk/contribute$', + persistPage: '/uk/checkout$', + }), + }; + + const participations: Participations = abInit({ + ...abtestInitalizerData, + abTests, + path: '/uk/checkout', + }); + + expect(participations).toEqual({ t1: 'control' }); + }); + + it('does not assign to test if persistPage does not match and test is in session storage', () => { + window.sessionStorage.setItem( + 'abParticipations', + JSON.stringify({ t1: 'control' }), + ); + + const abTests = { + t1: buildTest({ + targetPage: '/uk/contribute$', + persistPage: '/uk/checkout$', + }), + }; + + const participations: Participations = abInit({ + ...abtestInitalizerData, + abTests, + path: '/uk/blah', + }); + + expect(participations).toEqual({}); + }); + }); }); it('targetPage matching', () => { @@ -912,8 +888,8 @@ function buildTest({ seed = 0, excludeIfInReferrerControlledTest = false, excludeCountriesSubjectToContributionsOnlyAmounts = true, - targetPage = undefined, - persistPage = undefined, + targetPage = undefined, + persistPage = undefined, }: Partial): Test { return { variants, @@ -923,8 +899,8 @@ function buildTest({ seed, excludeIfInReferrerControlledTest, excludeCountriesSubjectToContributionsOnlyAmounts, - targetPage, - persistPage, + targetPage, + persistPage, }; } diff --git a/support-frontend/assets/helpers/abTests/abtest.ts b/support-frontend/assets/helpers/abTests/abtest.ts index 7ddf8ff7fe..132e9566b2 100644 --- a/support-frontend/assets/helpers/abTests/abtest.ts +++ b/support-frontend/assets/helpers/abTests/abtest.ts @@ -102,6 +102,7 @@ type ABtestInitalizerData = { abTests?: Tests; mvt?: number; acquisitionDataTests?: AcquisitionABTest[]; + path?: string; }; function init({ @@ -111,6 +112,7 @@ function init({ abTests = tests, mvt = getMvtId(), acquisitionDataTests = getTestFromAcquisitionData() ?? [], + path = window.location.pathname, }: ABtestInitalizerData): Participations { const sessionParticipations = getParticipationsFromSession(); const participations = getParticipations( @@ -118,6 +120,7 @@ function init({ mvt, countryId, countryGroupId, + path, acquisitionDataTests, selectedAmountsVariant, sessionParticipations, @@ -160,6 +163,7 @@ function getParticipations( mvtId: number, country: IsoCountry, countryGroupId: CountryGroupId, + path: string, acquisitionDataTests?: AcquisitionABTest[], selectedAmountsVariant?: SelectedAmountsVariant, sessionParticipations: Participations = {}, @@ -180,19 +184,15 @@ function getParticipations( } // Is the user already in this test in the current browser session? - console.log('pathname', window.location.pathname) - console.log('target', test.targetPage) - console.log('persist', test.persistPage) - console.log('window.sessionStorage', window.sessionStorage.getItem('abParticipations')) if ( !!sessionParticipations[testId] && - targetPageMatches(window.location.pathname, test.persistPage) + targetPageMatches(path, test.persistPage) ) { participations[testId] = sessionParticipations[testId]; return; } - if (!targetPageMatches(window.location.pathname, test.targetPage)) { + if (!targetPageMatches(path, test.targetPage)) { return; } From 170e5c7f8aa0d3b1b2e1639351bffddfc026f9d9 Mon Sep 17 00:00:00 2001 From: Tom Forbes Date: Mon, 21 Oct 2024 10:31:46 +0100 Subject: [PATCH 4/5] Remove fake ab test --- .../helpers/abTests/abtestDefinitions.ts | 22 ------------------- 1 file changed, 22 deletions(-) diff --git a/support-frontend/assets/helpers/abTests/abtestDefinitions.ts b/support-frontend/assets/helpers/abTests/abtestDefinitions.ts index 789a1441cf..640b1f0bda 100644 --- a/support-frontend/assets/helpers/abTests/abtestDefinitions.ts +++ b/support-frontend/assets/helpers/abTests/abtestDefinitions.ts @@ -13,8 +13,6 @@ export const pageUrlRegexes = { notUsLandingPage: '/uk|au|eu|int|nz|ca/contribute(/.*)?$', auLandingPage: '/au/contribute(/.*)?$', usLandingPage: '/us/contribute(/.*)?$', - landingPageOnly: '/(uk|us|au|eu|int|nz|ca)/contribute$', - ukCheckout: '/uk/checkout$', }, subscriptions: { subsDigiSubPages: '(/??/subscribe(\\?.*)?$|/??/subscribe/digital(\\?.*)?$)', @@ -173,24 +171,4 @@ export const tests: Tests = { targetPage: pageUrlRegexes.contributions.allLandingPagesAndThankyouPages, excludeCountriesSubjectToContributionsOnlyAmounts: false, }, - landingPageOnlyTest: { - variants: [ - // not really an AB test - { - id: 'variant', - }, - ], - audiences: { - ALL: { - offset: 0, - size: 1, - }, - }, - isActive: true, - referrerControlled: false, - seed: 6, - targetPage: pageUrlRegexes.contributions.landingPageOnly, - persistPage: pageUrlRegexes.contributions.ukCheckout, - excludeCountriesSubjectToContributionsOnlyAmounts: true, - }, }; From da91019c02e52d855e1062fcaa7af4a181143d8d Mon Sep 17 00:00:00 2001 From: Tom Forbes Date: Mon, 21 Oct 2024 10:44:00 +0100 Subject: [PATCH 5/5] return --- support-frontend/assets/helpers/abTests/abtest.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/support-frontend/assets/helpers/abTests/abtest.ts b/support-frontend/assets/helpers/abTests/abtest.ts index 132e9566b2..7df326a844 100644 --- a/support-frontend/assets/helpers/abTests/abtest.ts +++ b/support-frontend/assets/helpers/abTests/abtest.ts @@ -270,6 +270,7 @@ function getParticipationsFromSession(): Participations | undefined { return undefined; } } + return undefined; } function getServerSideParticipations(): Participations | null | undefined {