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

Use user benefits abtest #13137

Merged
merged 14 commits into from
Jan 21, 2025
35 changes: 23 additions & 12 deletions dotcom-rendering/playwright/tests/user.features.e2e.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,13 @@ import { Standard as standardArticle } from '../../fixtures/generated/fe-article
import { disableCMP } from '../lib/cmp';
import { addCookie } from '../lib/cookies';
import { loadPageNoOkta } from '../lib/load-page';
import { stubResponse } from '../lib/network';

test.describe('User cookies tests', () => {
const userAttributesApiUrl =
'https://members-data-api.theguardian.com/user-attributes';
const userBenefitsApiUrl =
'https://user-benefits.code.dev-guardianapis.com/benefits/me';
test(`Request to user features API is sent when no user features expiry cookie`, async ({
context,
page,
Expand All @@ -17,19 +22,22 @@ test.describe('User cookies tests', () => {

await disableCMP(context);

const membersDataApiPromise = page.waitForRequest(
'https://members-data-api.theguardian.com/user-attributes/me',
);
const userBenefitsApiPromise = stubResponse(page, userBenefitsApiUrl, {
status: 200,
body: JSON.stringify({
benefits: ['adFree', 'hideSupportMessaging'],
}),
});

await loadPageNoOkta(page, standardArticle, {
// user-features expects this config to be present
configOverrides: {
userAttributesApiUrl:
'https://members-data-api.theguardian.com/user-attributes',
userAttributesApiUrl,
userBenefitsApiUrl,
},
});

await membersDataApiPromise;
await userBenefitsApiPromise;

// expect GU_U to still be present so it has not been deleted by user-features
expect(
Expand Down Expand Up @@ -64,19 +72,22 @@ test.describe('User cookies tests', () => {

await disableCMP(context);

const membersDataApiPromise = page.waitForRequest(
'https://members-data-api.theguardian.com/user-attributes/me',
);
const userBenefitsApiPromise = stubResponse(page, userBenefitsApiUrl, {
status: 200,
body: JSON.stringify({
benefits: ['adFree', 'hideSupportMessaging'],
}),
});

await loadPageNoOkta(page, standardArticle, {
// user-features expects this config to be present
configOverrides: {
userAttributesApiUrl:
'https://members-data-api.theguardian.com/user-attributes',
userAttributesApiUrl,
userBenefitsApiUrl,
},
});

await membersDataApiPromise;
await userBenefitsApiPromise;
});

test(`Existing old cookie data is deleted when the user is signed out`, async ({
Expand Down
18 changes: 8 additions & 10 deletions dotcom-rendering/src/client/userFeatures/membersDataApi.ts
Original file line number Diff line number Diff line change
Expand Up @@ -87,16 +87,14 @@ export const syncDataFromMembersDataApi: (
) => Promise<UserBenefits> = async (
signedInAuthStatus: SignedInWithOkta | SignedInWithCookies,
) => {
const response = await fetchJson(
`${
window.guardian.config.page.userAttributesApiUrl ??
'/USER_ATTRIBUTE_API_NOT_FOUND'
}/me`,
{
mode: 'cors',
...getOptionsHeadersWithOkta(signedInAuthStatus),
},
);
const url = window.guardian.config.page.userAttributesApiUrl;
if (!url) {
throw new Error('userAttributesApiUrl is not defined');
}
const response = await fetchJson(url, {
mode: 'cors',
...getOptionsHeadersWithOkta(signedInAuthStatus),
});
if (!validateResponse(response)) {
throw new Error('invalid response');
}
Expand Down
50 changes: 11 additions & 39 deletions dotcom-rendering/src/client/userFeatures/user-features.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,19 +12,14 @@ import {
import { fetchJson } from './fetchJson';
import { deleteAllCookies, refresh } from './user-features';

const fakeUserFeatures = {
showSupportMessaging: false,
contentAccess: {
digitalPack: true,
recurringContributor: false,
paidMember: true,
},
const fakeUserBenefits = {
benefits: ['adFree', 'hideSupportMessaging'],
};

jest.mock('./fetchJson', () => {
return {
fetchJson: jest.fn(() => {
return Promise.resolve(fakeUserFeatures);
return Promise.resolve(fakeUserBenefits);
}),
};
});
Expand Down Expand Up @@ -60,6 +55,8 @@ const setAllFeaturesData = (opts: { isExpired: boolean }) => {

beforeAll(() => {
window.guardian.config.page.userAttributesApiUrl = '';
window.guardian.config.page.userBenefitsApiUrl = 'fake-url';
window.guardian.config.tests['useUserBenefitsApiVariant'] = 'variant';
});

describe('Refreshing the features data', () => {
Expand All @@ -70,7 +67,7 @@ describe('Refreshing the features data', () => {
getAuthStatus.mockResolvedValue({
kind: 'SignedInWithOkta',
} as AuthStatus);
fetchJsonSpy.mockReturnValue(Promise.resolve(fakeUserFeatures));
fetchJsonSpy.mockReturnValue(Promise.resolve(fakeUserBenefits));
});

it('Performs an update if the user has missing data', async () => {
Expand Down Expand Up @@ -124,55 +121,30 @@ describe('If user signed out', () => {

describe('Storing new feature data', () => {
beforeEach(() => {
const mockResponse = {
userId: 'abc',
showSupportMessaging: false,
contentAccess: {
member: false,
paidMember: false,
recurringContributor: false,
digitalPack: false,
paperSubscriber: false,
guardianWeeklySubscriber: false,
},
};

jest.resetAllMocks();
fetchJsonSpy.mockReturnValue(Promise.resolve(mockResponse));
fetchJsonSpy.mockReturnValue(Promise.resolve(fakeUserBenefits));
deleteAllCookies();
isUserLoggedInOktaRefactor.mockResolvedValue(true);
getAuthStatus.mockResolvedValue({
kind: 'SignedInWithOkta',
} as AuthStatus);
});

it('Puts the paying-member state and ad-free state in appropriate cookie', () => {
it('Puts the ad-free state in appropriate cookie', () => {
fetchJsonSpy.mockReturnValueOnce(
Promise.resolve({
showSupportMessaging: false,
contentAccess: {
paidMember: false,
recurringContributor: false,
digitalPack: false,
},
adFree: false,
benefits: [],
}),
);
return refresh().then(() => {
expect(getAdFreeCookie()).toBeNull();
});
});

it('Puts the paying-member state and ad-free state in appropriate cookie', () => {
it('Puts the ad-free state in appropriate cookie', () => {
fetchJsonSpy.mockReturnValueOnce(
Promise.resolve({
showSupportMessaging: false,
contentAccess: {
paidMember: true,
recurringContributor: true,
digitalPack: true,
},
adFree: true,
benefits: ['adFree'],
}),
);
return refresh().then(() => {
Expand Down
31 changes: 18 additions & 13 deletions dotcom-rendering/src/client/userFeatures/user-features.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import {
setUserFeaturesExpiryCookie,
} from './cookies/userFeaturesExpiry';
import { syncDataFromMembersDataApi } from './membersDataApi';
import { syncDataFromUserBenefitsApi } from './userBenefitsApi';

export type UserBenefits = {
adFree: boolean;
Expand Down Expand Up @@ -51,19 +52,23 @@ const refresh = async (): Promise<void> => {
}
};

const requestNewData = () => {
return getAuthStatus()
.then((authStatus) =>
authStatus.kind === 'SignedInWithCookies' ||
authStatus.kind === 'SignedInWithOkta'
? authStatus
: Promise.reject('The user is not signed in'),
)
.then((signedInAuthStatus) => {
return syncDataFromMembersDataApi(signedInAuthStatus).then(
persistResponse,
);
});
const shouldUseUserBenefitsApi = (): boolean => {
return !!window.guardian.config.tests['useUserBenefitsApiVariant'];
};

const requestNewData = async () => {
const authStatus = await getAuthStatus();
if (
authStatus.kind !== 'SignedInWithCookies' &&
authStatus.kind !== 'SignedInWithOkta'
) {
return Promise.reject('The user is not signed in');
}
if (shouldUseUserBenefitsApi()) {
return syncDataFromUserBenefitsApi(authStatus).then(persistResponse);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is neat that both methods return value of the same shape (so can use persistResponse in both cases).

} else {
return syncDataFromMembersDataApi(authStatus).then(persistResponse);
}
};

const timeInDaysFromNow = (daysFromNow: number): string => {
Expand Down
39 changes: 39 additions & 0 deletions dotcom-rendering/src/client/userFeatures/userBenefitsApi.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
import { isObject } from '@guardian/libs';
import {
getOptionsHeadersWithOkta,
type SignedInWithCookies,
type SignedInWithOkta,
} from '../../lib/identity';
import { fetchJson } from './fetchJson';
import type { UserBenefits } from './user-features';

type UserBenefitsResponse = {
benefits: string[];
};
export const syncDataFromUserBenefitsApi = async (
signedInAuthStatus: SignedInWithOkta | SignedInWithCookies,
): Promise<UserBenefits> => {
const url = window.guardian.config.page.userBenefitsApiUrl;
if (!url) {
throw new Error('userBenefitsApiUrl is not defined');
}
const response = await fetchJson(url, {
mode: 'cors',
...getOptionsHeadersWithOkta(signedInAuthStatus),
});
if (!validateResponse(response)) {
throw new Error('invalid response');
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

presumably that will throw an alarm somewhere for us/sentry?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess? Not sure, this is the same as the previous implementation

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

true I guess the more pertinent question is how will you know that everything is working with the new API like it was before across a similar percentage of browsers

}
return {
hideSupportMessaging: response.benefits.includes(
'hideSupportMessaging',
),
adFree: response.benefits.includes('adFree'),
};
};

const validateResponse = (
response: unknown,
): response is UserBenefitsResponse => {
return isObject(response) && Array.isArray(response.benefits);
};
2 changes: 2 additions & 0 deletions dotcom-rendering/src/experiments/ab-tests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import { mpuWhenNoEpic } from './tests/mpu-when-no-epic';
import { optimiseSpacefinderInline } from './tests/optimise-spacefinder-inline';
import { signInGateMainControl } from './tests/sign-in-gate-main-control';
import { signInGateMainVariant } from './tests/sign-in-gate-main-variant';
import { userBenefitsApi } from './tests/user-benefits-api';

// keep in sync with ab-tests in frontend
// https://github.com/guardian/frontend/tree/main/static/src/javascripts/projects/common/modules/experiments/ab-tests.ts
Expand All @@ -19,4 +20,5 @@ export const tests: ABTest[] = [
mpuWhenNoEpic,
adBlockAsk,
optimiseSpacefinderInline,
userBenefitsApi,
];
31 changes: 31 additions & 0 deletions dotcom-rendering/src/experiments/tests/user-benefits-api.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
import type { ABTest } from '@guardian/ab-core';

export const userBenefitsApi: ABTest = {
id: 'UserBenefitsApi',
start: '2020-05-20',
expiry: '2025-12-01',
author: 'Rupert Bates',
description:
'This test is being used to roll out the user benefits API in a gradual manner',
audience: 2 / 100, // 2%
audienceOffset: 0,
successMeasure:
'There are no new client side errors and the user benefits API copes with the load',
audienceCriteria: 'Everyone',
showForSensitive: true,
canRun: () => true,
variants: [
{
id: 'control',
test: (): void => {
/* no-op */
},
},
{
id: 'variant',
test: (): void => {
/* no-op */
},
},
],
};
1 change: 1 addition & 0 deletions dotcom-rendering/src/model/guardian.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ export interface Guardian {
isPaidContent?: boolean;
isDev?: boolean;
userAttributesApiUrl?: string;
userBenefitsApiUrl?: string;
idApiUrl?: string;
isPodcast?: boolean;
};
Expand Down
Loading