diff --git a/dotcom-rendering/src/components/EmailSignUpWrapper.island.test.tsx b/dotcom-rendering/src/components/EmailSignUpWrapper.island.test.tsx index 834d1da677d..6dfed7691ea 100644 --- a/dotcom-rendering/src/components/EmailSignUpWrapper.island.test.tsx +++ b/dotcom-rendering/src/components/EmailSignUpWrapper.island.test.tsx @@ -1,8 +1,7 @@ import '@testing-library/jest-dom'; import { render, screen } from '@testing-library/react'; import { submitComponentEvent } from '../client/ophan/ophan'; -import { AB_TEST_NAME } from '../lib/newsletterSignupTracking'; -import { useAB } from '../lib/useAB'; +import { NEWSLETTER_SIGNUP_COMPONENT_ID } from '../lib/newsletterSignupTracking'; import { useIsSignedIn } from '../lib/useAuthStatus'; import { useNewsletterSubscription } from '../lib/useNewsletterSubscription'; import { ConfigProvider } from './ConfigContext'; @@ -12,10 +11,6 @@ jest.mock('../client/ophan/ophan', () => ({ submitComponentEvent: jest.fn(() => Promise.resolve()), })); -jest.mock('../lib/useAB', () => ({ - useAB: jest.fn(), -})); - jest.mock('../lib/useAuthStatus', () => ({ useIsSignedIn: jest.fn(), })); @@ -35,10 +30,6 @@ jest.mock('./NewsletterSignupForm.island', () => ({ ), })); -jest.mock('./SecureSignup.island', () => ({ - SecureSignup: () =>
SecureSignup
, -})); - jest.mock('./NewsletterSignupCardContainer', () => ({ NewsletterSignupCardContainer: ({ children, @@ -51,12 +42,6 @@ jest.mock('./NewsletterSignupCardContainer', () => ({ ), })); -jest.mock('./EmailSignup', () => ({ - EmailSignup: ({ children }: { children: React.ReactNode }) => ( -
{children}
- ), -})); - const defaultProps = { index: 0, listId: 4147, @@ -64,7 +49,6 @@ const defaultProps = { name: 'The Recap', description: 'A weekly sports roundup', frequency: 'Weekly', - successDescription: "We'll send you The Recap every week", theme: 'sport', idApiUrl: 'https://idapi.theguardian.com', }; @@ -83,181 +67,52 @@ const renderWrapper = (props = {}, renderingTarget: 'Web' | 'Apps' = 'Web') => , ); -const mockAbTests = (variant: 'control' | 'variantIllustratedCard') => { - (useAB as jest.Mock).mockReturnValue({ - getParticipations: () => ({ - [AB_TEST_NAME]: variant, - }), - }); -}; - describe('EmailSignUpWrapper', () => { beforeEach(() => { jest.resetAllMocks(); - // Default: AB API not yet hydrated - (useAB as jest.Mock).mockReturnValue(undefined); (useIsSignedIn as jest.Mock).mockReturnValue(false); (useNewsletterSubscription as jest.Mock).mockReturnValue(false); }); - describe('flag off (showNewNewsletterSignupCard = false)', () => { - it('shows a placeholder while subscription status is loading', () => { - (useNewsletterSubscription as jest.Mock).mockReturnValue(undefined); + describe('rendering', () => { + it('renders the NewsletterSignupCardContainer', () => { renderWrapper(); - // The Placeholder component renders a div with specific heights - expect( - screen.queryByTestId('email-signup'), - ).not.toBeInTheDocument(); - expect( - screen.queryByTestId('newsletter-signup-card-container'), - ).not.toBeInTheDocument(); - }); - - it('renders nothing when the user is already subscribed', () => { - mockAbTests('control'); - (useNewsletterSubscription as jest.Mock).mockReturnValue(true); - const { container } = renderWrapper({ - hideNewsletterSignupComponentForSubscribers: true, - }); - expect(container).toBeEmptyDOMElement(); - }); - - it('renders the legacy EmailSignup when the user is not subscribed', () => { - mockAbTests('control'); - renderWrapper(); - expect(screen.getByTestId('email-signup')).toBeInTheDocument(); - expect(screen.getByTestId('secure-signup')).toBeInTheDocument(); - expect( - screen.queryByTestId('newsletter-signup-card-container'), - ).not.toBeInTheDocument(); - }); - }); - - describe('flag on (showNewNewsletterSignupCard = true)', () => { - it('shows a placeholder when AB API has not hydrated yet', () => { - // useAB returns undefined before hydration — component shows - // Placeholder rather than committing to control or variant early. - // (default set in outer beforeEach, no override needed) - renderWrapper({ showNewNewsletterSignupCard: true }); - expect( - screen.queryByTestId('email-signup'), - ).not.toBeInTheDocument(); - expect( - screen.queryByTestId('newsletter-signup-card-container'), - ).not.toBeInTheDocument(); - }); - - it('renders NewsletterSignupCardContainer for all web users regardless of AB participation', () => { - mockAbTests('control'); - renderWrapper({ showNewNewsletterSignupCard: true }); expect( screen.getByTestId('newsletter-signup-card-container'), ).toBeInTheDocument(); expect( - screen.queryByTestId('email-signup'), - ).not.toBeInTheDocument(); + screen.getByTestId('newsletter-signup-form'), + ).toBeInTheDocument(); }); - it('renders NewsletterSignupCardContainer for users in the variant group', () => { - mockAbTests('variantIllustratedCard'); - renderWrapper({ showNewNewsletterSignupCard: true }); + it('renders the NewsletterSignupCardContainer on Apps as well as Web', () => { + renderWrapper({}, 'Apps'); expect( screen.getByTestId('newsletter-signup-card-container'), ).toBeInTheDocument(); - expect( - screen.getByTestId('newsletter-signup-form'), - ).toBeInTheDocument(); - expect( - screen.queryByTestId('email-signup'), - ).not.toBeInTheDocument(); }); - it('does not hide the variant for already-subscribed users (subscription handled inside the card)', () => { - mockAbTests('variantIllustratedCard'); + it('still renders the card when the user is already subscribed', () => { (useNewsletterSubscription as jest.Mock).mockReturnValue(true); - renderWrapper({ - showNewNewsletterSignupCard: true, - hideNewsletterSignupComponentForSubscribers: true, - }); + renderWrapper(); expect( screen.getByTestId('newsletter-signup-card-container'), ).toBeInTheDocument(); }); }); - describe('Apps rendering target', () => { - it('renders the legacy EmailSignup without waiting for AB to resolve', () => { - // useAB remains undefined (AB framework never initialises on Apps) - // but the component should not block on it - renderWrapper({ showNewNewsletterSignupCard: true }, 'Apps'); - expect(screen.getByTestId('email-signup')).toBeInTheDocument(); - expect( - screen.queryByTestId('newsletter-signup-card-container'), - ).not.toBeInTheDocument(); - }); - - it('fires a VIEW tracking event without waiting for AB to resolve', () => { - renderWrapper({ showNewNewsletterSignupCard: true }, 'Apps'); - expect(submitComponentEvent).toHaveBeenCalledWith( - expect.objectContaining({ - action: 'VIEW', - abTest: { name: AB_TEST_NAME, variant: 'control' }, - }), - 'Apps', - ); - }); - }); - describe('VIEW tracking', () => { - it('fires a VIEW event with the variantIllustratedCard component id for all web users', () => { - mockAbTests('control'); - renderWrapper({ showNewNewsletterSignupCard: true }); - - expect(submitComponentEvent).toHaveBeenCalledWith( - expect.objectContaining({ - component: { - componentType: 'NEWSLETTER_SUBSCRIPTION', - id: 'AR NewsletterSignupForm the-recap - variantIllustratedCard', - }, - action: 'VIEW', - abTest: { - name: AB_TEST_NAME, - variant: 'variantIllustratedCard', - }, - }), - 'Web', - ); - }); - - it('fires a VIEW event with the correct variant component id and ab metadata', () => { - mockAbTests('variantIllustratedCard'); - renderWrapper({ showNewNewsletterSignupCard: true }); + it('fires a VIEW event with the in-article signup form component id', () => { + renderWrapper(); expect(submitComponentEvent).toHaveBeenCalledWith( expect.objectContaining({ component: { componentType: 'NEWSLETTER_SUBSCRIPTION', - id: 'AR NewsletterSignupForm the-recap - variantIllustratedCard', - }, - action: 'VIEW', - abTest: { - name: AB_TEST_NAME, - variant: 'variantIllustratedCard', + id: NEWSLETTER_SIGNUP_COMPONENT_ID.inArticleSignupForm( + 'the-recap', + ), }, - }), - 'Web', - ); - }); - - it('fires a VIEW event with the control component id when the flag is off', () => { - mockAbTests('control'); - renderWrapper({ showNewNewsletterSignupCard: false }); - - expect(submitComponentEvent).toHaveBeenCalledWith( - expect.objectContaining({ - component: expect.objectContaining({ - id: 'AR SecureSignup the-recap', - }), action: 'VIEW', }), 'Web', @@ -265,45 +120,21 @@ describe('EmailSignUpWrapper', () => { }); it('fires the VIEW event exactly once on mount', () => { - mockAbTests('control'); - renderWrapper({ showNewNewsletterSignupCard: true }); + renderWrapper(); expect(submitComponentEvent).toHaveBeenCalledTimes(1); }); it('does not fire a VIEW event while subscription status is loading', () => { - mockAbTests('control'); (useNewsletterSubscription as jest.Mock).mockReturnValue(undefined); - renderWrapper({ showNewNewsletterSignupCard: true }); - - expect(submitComponentEvent).not.toHaveBeenCalled(); - }); - - it('does not fire a VIEW event while the AB client has not hydrated', () => { - // useAB returns undefined — default set in beforeEach - renderWrapper({ showNewNewsletterSignupCard: true }); - - expect(submitComponentEvent).not.toHaveBeenCalled(); - }); - - it('does not fire a VIEW event for control users who are already subscribed', () => { - mockAbTests('control'); - (useNewsletterSubscription as jest.Mock).mockReturnValue(true); - renderWrapper({ - showNewNewsletterSignupCard: true, - hideNewsletterSignupComponentForSubscribers: true, - }); + renderWrapper(); expect(submitComponentEvent).not.toHaveBeenCalled(); }); - it('does not fire a VIEW event for variant users who are already subscribed', () => { - mockAbTests('variantIllustratedCard'); + it('does not fire a VIEW event when the user is already subscribed', () => { (useNewsletterSubscription as jest.Mock).mockReturnValue(true); - renderWrapper({ - showNewNewsletterSignupCard: true, - hideNewsletterSignupComponentForSubscribers: true, - }); + renderWrapper(); expect(submitComponentEvent).not.toHaveBeenCalled(); }); diff --git a/dotcom-rendering/src/components/EmailSignUpWrapper.island.tsx b/dotcom-rendering/src/components/EmailSignUpWrapper.island.tsx index c4fa4568b32..84b203d8dbf 100644 --- a/dotcom-rendering/src/components/EmailSignUpWrapper.island.tsx +++ b/dotcom-rendering/src/components/EmailSignUpWrapper.island.tsx @@ -1,58 +1,36 @@ -import type { Breakpoint } from '@guardian/source/foundations'; import { useEffect, useRef } from 'react'; import { - AB_TEST_NAME, NEWSLETTER_SIGNUP_COMPONENT_ID, sendNewsletterSignupEvent, } from '../lib/newsletterSignupTracking'; -import { useAB } from '../lib/useAB'; import { useIsSignedIn } from '../lib/useAuthStatus'; import { useNewsletterSubscription } from '../lib/useNewsletterSubscription'; import { useConfig } from './ConfigContext'; import type { EmailSignUpProps } from './EmailSignup'; -import { EmailSignup } from './EmailSignup'; import { InlineSkipToWrapper } from './InlineSkipToWrapper'; import { Island } from './Island'; -import { NewsletterPrivacyMessage } from './NewsletterPrivacyMessage'; import { NewsletterSignupCardContainer } from './NewsletterSignupCardContainer'; import { NewsletterSignupForm } from './NewsletterSignupForm.island'; -import { Placeholder } from './Placeholder'; -import { SecureSignup } from './SecureSignup.island'; - -/** - * Approximate heights of the EmailSignup component at different breakpoints. - */ -const PLACEHOLDER_HEIGHTS = new Map([ - ['mobile', 220], - ['tablet', 180], - ['desktop', 180], -]); +// When the next A/B experiment is added (e.g. preview-button test), import +// useAB and AB_TEST_NAME from their respective modules and thread them through +// sendNewsletterSignupEvent's `abTest` param and NewsletterSignupForm's `abTest` prop. interface EmailSignUpWrapperProps extends EmailSignUpProps { index: number; listId: number; identityName: string; category?: string; - successDescription: string; - /** Illustration image URL (square crop) for the NewsletterSignupCard variant */ + /** Illustration image URL (square crop) for the NewsletterSignupCard */ illustrationSquare?: string; idApiUrl: string; exampleUrl?: string; - /** Feature flag to enable hiding newsletter signup for already subscribed users */ - hideNewsletterSignupComponentForSubscribers?: boolean; - /** Feature flag to show the new NewsletterSignupCard design instead of EmailSignup */ - showNewNewsletterSignupCard?: boolean; } /** * EmailSignUpWrapper as an island component. * * This component needs to be hydrated client-side because it uses - * the useNewsletterSubscription hook which depends on auth status - * to determine if the user is already subscribed to the newsletter. - * - * If the user is signed in and already subscribed, this component - * will return null (hide the signup form). + * client-side hooks for auth status and tracking. */ export const EmailSignUpWrapper = ({ index, @@ -66,46 +44,26 @@ export const EmailSignUpWrapper = ({ illustrationSquare, frequency, theme, - successDescription, - hideNewsletterSignupComponentForSubscribers = false, - showNewNewsletterSignupCard = false, }: EmailSignUpWrapperProps) => { const { renderingTarget } = useConfig(); - - const abTestEnabled = - showNewNewsletterSignupCard && renderingTarget === 'Web'; - - const abTests = useAB(); - const abResolved = abTests !== undefined; - - const getVariantName = () => 'variantIllustratedCard'; - - const abVariant = abTestEnabled ? getVariantName() : 'control'; - - const isSubscribed = useNewsletterSubscription( - listId, - idApiUrl, - hideNewsletterSignupComponentForSubscribers, - ); const isSignedIn = useIsSignedIn(); + const isSubscribed = useNewsletterSubscription(listId, idApiUrl); + + const componentId = + NEWSLETTER_SIGNUP_COMPONENT_ID.inArticleSignupForm(identityName); const viewFiredRef = useRef(false); useEffect(() => { - if (abTestEnabled && !abResolved) { - return; - } - // Wait for subscription status in both branches — we only want to track - // a view of the actual signup form, not a loading state or success message. + // Wait for subscription status — we only want to track a view of the + // actual signup form, not a loading state or success message. if (isSubscribed === undefined) { return; } - // Don't fire if the user is already subscribed: in both branches they - // will see a success/already-subscribed message, not the signup form. + // Don't fire if the user is already subscribed. if (isSubscribed) { return; } - // Guard against double-firing (e.g. if deps change after the first fire) if (viewFiredRef.current) { return; } @@ -113,110 +71,44 @@ export const EmailSignUpWrapper = ({ sendNewsletterSignupEvent({ action: 'VIEW', identityName, - componentId: - abVariant === 'variantIllustratedCard' - ? NEWSLETTER_SIGNUP_COMPONENT_ID.variantIllustratedCard( - identityName, - ) - : abVariant === 'variantNewField' - ? NEWSLETTER_SIGNUP_COMPONENT_ID.variantNewField( - identityName, - ) - : NEWSLETTER_SIGNUP_COMPONENT_ID.control(identityName), + componentId, renderingTarget, value: { eventDescription: 'newsletter-signup-viewed', }, - // Use the standard Ophan abTest field so Ophan can join events - // to the A/B test — not strings encoded inside value. - abTest: { name: AB_TEST_NAME, variant: abVariant }, }); - }, [ - abResolved, - abVariant, - identityName, - isSubscribed, - abTestEnabled, - renderingTarget, - ]); - - if ((abTestEnabled && !abResolved) || isSubscribed === undefined) { - return ; - } - - if (abVariant === 'variantIllustratedCard') { - return ( - - - {(previewAction) => ( - - - - )} - - - ); - } - - // Don't render control if user is already subscribed - if (isSubscribed) { - return null; - } - - const emailInputName = - abVariant === 'variantNewField' ? 'emailAddress' : 'email'; - const emailInputId = - abVariant === 'variantNewField' - ? `emailInput-${identityName}` - : undefined; + }, [componentId, identityName, isSubscribed, renderingTarget]); return ( - - - - - - + {(previewAction) => ( + + + + )} + ); }; diff --git a/dotcom-rendering/src/components/EmailSignUpWrapper.stories.tsx b/dotcom-rendering/src/components/EmailSignUpWrapper.stories.tsx index b3c19fe1ce6..a3e92e981dd 100644 --- a/dotcom-rendering/src/components/EmailSignUpWrapper.stories.tsx +++ b/dotcom-rendering/src/components/EmailSignUpWrapper.stories.tsx @@ -2,30 +2,10 @@ import type { StoryObj } from '@storybook/react-webpack5'; import { mocked, within } from 'storybook/test'; import preview from '../../.storybook/preview'; import { lazyFetchEmailWithTimeout } from '../lib/fetchEmail'; -import { AB_TEST_NAME } from '../lib/newsletterSignupTracking'; -import { useAB } from '../lib/useAB'; import { useIsSignedIn } from '../lib/useAuthStatus'; import { useNewsletterSubscription } from '../lib/useNewsletterSubscription'; import { EmailSignUpWrapper } from './EmailSignUpWrapper.island'; -/** Resolves `useAB` as if the AB framework has hydrated, placing the user in control or variant. */ -const mockAB = ( - variant: 'control' | 'variantNewField' | 'variantIllustratedCard', -) => { - mocked(useAB).mockReturnValue({ - isUserInTestGroup: (_testName: string, group: string) => - group === variant, - isUserInTest: () => true, - getParticipations: () => - (variant !== 'control' - ? { - [AB_TEST_NAME]: variant, - } - : {}) as Record, - trackABTests: () => ({}), - }); -}; - const meta = preview.meta({ title: 'Components/EmailSignUpWrapper', component: EmailSignUpWrapper, @@ -41,32 +21,16 @@ const defaultArgs = { "The best of our sports journalism from the past seven days and a heads-up on the weekend's action", name: 'The Recap', frequency: 'Weekly', - successDescription: "We'll send you The Recap every week", theme: 'sport', idApiUrl: 'https://idapi.theguardian.com', exampleUrl: 'https://www.theguardian.com/email/the-recap', -} satisfies Story['args']; - -const newCardArgs = { - ...defaultArgs, - showNewNewsletterSignupCard: true, - hideNewsletterSignupComponentForSubscribers: true, illustrationSquare: 'https://i.guim.co.uk/img/uploads/2023/11/01/SaturdayEdition_-_5-3.jpg?width=220&dpr=2&s=none&crop=5%3A3', } satisfies Story['args']; -export const Placeholder = meta.story({ - args: { ...defaultArgs }, - beforeEach() { - mockAB('control'); - mocked(useNewsletterSubscription).mockReturnValue(undefined); - }, -}); - -export const DefaultStory = meta.story({ +export const SignedOut = meta.story({ args: { ...defaultArgs }, beforeEach() { - mockAB('control'); mocked(useNewsletterSubscription).mockReturnValue(false); mocked(useIsSignedIn).mockReturnValue(false); mocked(lazyFetchEmailWithTimeout).mockReturnValue(() => @@ -75,10 +39,9 @@ export const DefaultStory = meta.story({ }, }); -export const SignedInNotSubscribed = meta.story({ +export const SignedIn = meta.story({ args: { ...defaultArgs }, beforeEach() { - mockAB('control'); mocked(useNewsletterSubscription).mockReturnValue(false); mocked(useIsSignedIn).mockReturnValue(true); mocked(lazyFetchEmailWithTimeout).mockReturnValue(() => @@ -88,66 +51,8 @@ export const SignedInNotSubscribed = meta.story({ }); export const SignedInAlreadySubscribed = meta.story({ - args: { - ...defaultArgs, - hideNewsletterSignupComponentForSubscribers: true, - }, - beforeEach() { - mockAB('control'); - mocked(useNewsletterSubscription).mockReturnValue(true); - }, -}); - -export const FeatureFlagDisabled = meta.story({ - args: { - ...defaultArgs, - hideNewsletterSignupComponentForSubscribers: false, - }, - beforeEach() { - mockAB('control'); - mocked(useNewsletterSubscription).mockReturnValue(false); - mocked(useIsSignedIn).mockReturnValue(true); - mocked(lazyFetchEmailWithTimeout).mockReturnValue(() => - Promise.resolve('test@example.com'), - ); - }, - parameters: { - docs: { - description: { - story: 'When the hideNewsletterSignupComponentForSubscribers feature flag is disabled, the signup form is always shown regardless of subscription status.', - }, - }, - }, -}); - -export const NewsletterSignupCardSignedInNotSubscribed = meta.story({ - args: newCardArgs, - beforeEach() { - mockAB('variantIllustratedCard'); - mocked(useNewsletterSubscription).mockReturnValue(false); - mocked(useIsSignedIn).mockReturnValue(true); - mocked(lazyFetchEmailWithTimeout).mockReturnValue(() => - Promise.resolve('test@example.com'), - ); - }, -}); - -export const NewsletterSignupCardSignedOutNotSubscribed = meta.story({ - args: newCardArgs, - beforeEach() { - mockAB('variantIllustratedCard'); - mocked(useNewsletterSubscription).mockReturnValue(false); - mocked(useIsSignedIn).mockReturnValue(false); - mocked(lazyFetchEmailWithTimeout).mockReturnValue(() => - Promise.resolve(null), - ); - }, -}); - -export const NewsletterSignupCardSignedInAlreadySubscribed = meta.story({ - args: newCardArgs, + args: { ...defaultArgs }, beforeEach() { - mockAB('variantIllustratedCard'); mocked(useNewsletterSubscription).mockReturnValue(true); mocked(useIsSignedIn).mockReturnValue(true); mocked(lazyFetchEmailWithTimeout).mockReturnValue(() => @@ -156,10 +61,9 @@ export const NewsletterSignupCardSignedInAlreadySubscribed = meta.story({ }, }); -export const NewsletterSignupCardSignedOutAlreadySubscribed = meta.story({ - args: newCardArgs, +export const SignedOutAlreadySubscribed = meta.story({ + args: { ...defaultArgs }, beforeEach() { - mockAB('variantIllustratedCard'); mocked(useNewsletterSubscription).mockReturnValue(true); mocked(useIsSignedIn).mockReturnValue(false); mocked(lazyFetchEmailWithTimeout).mockReturnValue(() => @@ -168,10 +72,9 @@ export const NewsletterSignupCardSignedOutAlreadySubscribed = meta.story({ }, }); -export const NewsletterSignupCardFocused = meta.story({ - args: newCardArgs, +export const EmailInputFocused = meta.story({ + args: { ...defaultArgs }, beforeEach() { - mockAB('variantIllustratedCard'); mocked(useNewsletterSubscription).mockReturnValue(false); mocked(useIsSignedIn).mockReturnValue(false); mocked(lazyFetchEmailWithTimeout).mockReturnValue(() => diff --git a/dotcom-rendering/src/components/Masthead/Newsletter/HighlightsNewsletterSignupModal.tsx b/dotcom-rendering/src/components/Masthead/Newsletter/HighlightsNewsletterSignupModal.tsx index b13592b4cf9..cb5bdb091b1 100644 --- a/dotcom-rendering/src/components/Masthead/Newsletter/HighlightsNewsletterSignupModal.tsx +++ b/dotcom-rendering/src/components/Masthead/Newsletter/HighlightsNewsletterSignupModal.tsx @@ -12,6 +12,7 @@ import { Pillar, } from '../../../lib/articleFormat'; import { generateImageURL } from '../../../lib/image'; +import { NEWSLETTER_SIGNUP_COMPONENT_ID } from '../../../lib/newsletterSignupTracking'; import { useNewsletterSubscription } from '../../../lib/useNewsletterSubscription'; import type { Newsletter } from '../../../types/content'; import { FormatBoundary } from '../../FormatBoundary'; @@ -143,10 +144,9 @@ const HighlightsNewsletterSignupModalContent = ({ frequency={newsletter.frequency} isModal={true} isAlreadySubscribed={isSubscribed === true} - abTest={{ - name: 'highlights-newsletter-card', - variant: 'highlightsCard', - }} + componentId={NEWSLETTER_SIGNUP_COMPONENT_ID.highlightsCard( + newsletter.identityName, + )} /> diff --git a/dotcom-rendering/src/components/NewsletterSignupCardContainer.test.tsx b/dotcom-rendering/src/components/NewsletterSignupCardContainer.test.tsx index f8486e4aab4..d98347f41f9 100644 --- a/dotcom-rendering/src/components/NewsletterSignupCardContainer.test.tsx +++ b/dotcom-rendering/src/components/NewsletterSignupCardContainer.test.tsx @@ -73,7 +73,7 @@ describe('NewsletterSignupCardContainer', () => { expect.objectContaining({ component: { componentType: 'NEWSLETTER_SUBSCRIPTION', - id: NEWSLETTER_SIGNUP_COMPONENT_ID.variantIllustratedCard( + id: NEWSLETTER_SIGNUP_COMPONENT_ID.inArticleSignupForm( defaultProps.identityName, ), }, @@ -102,7 +102,7 @@ describe('NewsletterSignupCardContainer', () => { expect.objectContaining({ component: { componentType: 'NEWSLETTER_SUBSCRIPTION', - id: NEWSLETTER_SIGNUP_COMPONENT_ID.variantIllustratedCard( + id: NEWSLETTER_SIGNUP_COMPONENT_ID.inArticleSignupForm( defaultProps.identityName, ), }, @@ -176,7 +176,7 @@ describe('NewsletterSignupCardContainer', () => { expect.objectContaining({ action: 'EXPAND', component: expect.objectContaining({ - id: NEWSLETTER_SIGNUP_COMPONENT_ID.variantIllustratedCard( + id: NEWSLETTER_SIGNUP_COMPONENT_ID.inArticleSignupForm( defaultProps.identityName, ), }), diff --git a/dotcom-rendering/src/components/NewsletterSignupCardContainer.tsx b/dotcom-rendering/src/components/NewsletterSignupCardContainer.tsx index 9cb2272b6ca..1f374f7c93c 100644 --- a/dotcom-rendering/src/components/NewsletterSignupCardContainer.tsx +++ b/dotcom-rendering/src/components/NewsletterSignupCardContainer.tsx @@ -31,7 +31,7 @@ const sendPreviewTracking = ({ action: eventDescription === 'preview-open' ? 'EXPAND' : 'CLOSE', identityName, componentId: - NEWSLETTER_SIGNUP_COMPONENT_ID.variantIllustratedCard(identityName), + NEWSLETTER_SIGNUP_COMPONENT_ID.inArticleSignupForm(identityName), renderingTarget, value: { eventDescription, renderUrl, isSignedIn }, }); diff --git a/dotcom-rendering/src/components/NewsletterSignupForm.island.stories.tsx b/dotcom-rendering/src/components/NewsletterSignupForm.island.stories.tsx index 8822129fdc8..392ed120914 100644 --- a/dotcom-rendering/src/components/NewsletterSignupForm.island.stories.tsx +++ b/dotcom-rendering/src/components/NewsletterSignupForm.island.stories.tsx @@ -50,6 +50,7 @@ const defaultArgs = { newsletterName: 'Saturday Edition', frequency: 'every Saturday', previewAction: { behaviour: 'modal' as const, onClick: fn() }, + componentId: 'AR NewsletterSignupForm saturday-edition', }; /** Shared no-op handlers — stories that focus on visual state don't need real callbacks. */ diff --git a/dotcom-rendering/src/components/NewsletterSignupForm.island.test.tsx b/dotcom-rendering/src/components/NewsletterSignupForm.island.test.tsx index e4ed1862332..a517597d21e 100644 --- a/dotcom-rendering/src/components/NewsletterSignupForm.island.test.tsx +++ b/dotcom-rendering/src/components/NewsletterSignupForm.island.test.tsx @@ -89,6 +89,7 @@ const renderForm = () => newsletterId="morning-briefing" newsletterName="Morning Briefing" frequency="every day" + componentId="AR NewsletterSignupForm morning-briefing" /> , ); diff --git a/dotcom-rendering/src/components/NewsletterSignupForm.island.tsx b/dotcom-rendering/src/components/NewsletterSignupForm.island.tsx index d5518ee406d..b39b046ab90 100644 --- a/dotcom-rendering/src/components/NewsletterSignupForm.island.tsx +++ b/dotcom-rendering/src/components/NewsletterSignupForm.island.tsx @@ -35,6 +35,9 @@ type Props = { previewAction?: NewsletterPreviewAction; /** When `true`, the success message is shown immediately (user is already subscribed). */ isAlreadySubscribed?: boolean; + /** Ophan component ID for tracking events. Derived by the caller from the active + * component/experiment so that this form is not coupled to test state. */ + componentId: string; /** Ophan A/B test metadata — forwarded to tracking events. */ abTest?: AbTest; /** @@ -290,6 +293,7 @@ const NewsletterSignupFormActive = ({ newsletterName, frequency, previewAction, + componentId, abTest, isModal = false, }: Omit) => { @@ -320,6 +324,7 @@ const NewsletterSignupFormActive = ({ } = useNewsletterSignupForm( newsletterId, renderingTarget, + componentId, abTest, hideMarketingToggle, ); diff --git a/dotcom-rendering/src/components/SecureSignup.island.test.tsx b/dotcom-rendering/src/components/SecureSignup.island.test.tsx index acac253ab99..60452076105 100644 --- a/dotcom-rendering/src/components/SecureSignup.island.test.tsx +++ b/dotcom-rendering/src/components/SecureSignup.island.test.tsx @@ -323,24 +323,12 @@ describe('SecureSignup tracking component id', () => { global.fetch = jest.fn().mockResolvedValue({ ok: true } as Response); }); - it('uses the control component id when no abTest variant is provided', async () => { - const testUser = user.setup(); - renderSecureSignup(); - await submitSignupForm(testUser); - - await waitFor(() => { - expect( - getTrackingCallForEvent('form-submission')?.componentId, - ).toBe('AR SecureSignup morning-briefing'); - }); - }); - - it('uses the variantNewField component id when variant is variantNewField', async () => { + it('always uses the secureSignup component id regardless of abTest variant', async () => { const testUser = user.setup(); renderSecureSignup({ abTest: { name: 'test-ab', - variant: 'variantNewField', + variant: 'someVariant', }, }); await submitSignupForm(testUser); @@ -348,7 +336,7 @@ describe('SecureSignup tracking component id', () => { await waitFor(() => { expect( getTrackingCallForEvent('form-submission')?.componentId, - ).toBe('AR SecureSignup morning-briefing - variantNewField'); + ).toBe('AR SecureSignup morning-briefing'); }); }); }); diff --git a/dotcom-rendering/src/components/SecureSignup.island.tsx b/dotcom-rendering/src/components/SecureSignup.island.tsx index ad2db25b31b..a4844fb8b9e 100644 --- a/dotcom-rendering/src/components/SecureSignup.island.tsx +++ b/dotcom-rendering/src/components/SecureSignup.island.tsx @@ -254,15 +254,10 @@ const sendTracking = ( abTest?: AbTest, extraDetails?: Record, ): void => { - const componentId = - abTest?.variant === 'variantNewField' - ? NEWSLETTER_SIGNUP_COMPONENT_ID.variantNewField(newsletterId) - : NEWSLETTER_SIGNUP_COMPONENT_ID.control(newsletterId); - sendNewsletterSignupEvent({ action: EVENT_DESCRIPTION_TO_ACTION[eventDescription], identityName: newsletterId, - componentId, + componentId: NEWSLETTER_SIGNUP_COMPONENT_ID.secureSignup(newsletterId), renderingTarget, value: { ...extraDetails, diff --git a/dotcom-rendering/src/lib/newsletterSignupTracking.test.ts b/dotcom-rendering/src/lib/newsletterSignupTracking.test.ts index 6491b0ffbe0..f21cc47bf84 100644 --- a/dotcom-rendering/src/lib/newsletterSignupTracking.test.ts +++ b/dotcom-rendering/src/lib/newsletterSignupTracking.test.ts @@ -1,6 +1,5 @@ import { submitComponentEvent } from '../client/ophan/ophan'; import { - AB_TEST_NAME, NEWSLETTER_SIGNUP_COMPONENT_ID, sendNewsletterSignupEvent, } from './newsletterSignupTracking'; @@ -14,20 +13,16 @@ const RENDERING_TARGET = 'Web'; const MOCK_TIMESTAMP = 1_000_000_000_000; describe('NEWSLETTER_SIGNUP_COMPONENT_ID', () => { - it('returns the correct control component id', () => { - expect(NEWSLETTER_SIGNUP_COMPONENT_ID.control(IDENTITY_NAME)).toBe( - `AR SecureSignup ${IDENTITY_NAME}`, - ); + it('returns the in article signup form component id', () => { + expect( + NEWSLETTER_SIGNUP_COMPONENT_ID.inArticleSignupForm(IDENTITY_NAME), + ).toBe(`AR NewsletterSignupForm ${IDENTITY_NAME}`); }); - it('returns the correct variant component id', () => { + it('returns the correct highlights card component id', () => { expect( - NEWSLETTER_SIGNUP_COMPONENT_ID.variantIllustratedCard( - IDENTITY_NAME, - ), - ).toBe( - `AR NewsletterSignupForm ${IDENTITY_NAME} - variantIllustratedCard`, - ); + NEWSLETTER_SIGNUP_COMPONENT_ID.highlightsCard(IDENTITY_NAME), + ).toBe(`HighlightsNewsletterCard ${IDENTITY_NAME}`); }); }); @@ -45,7 +40,10 @@ describe('sendNewsletterSignupEvent', () => { sendNewsletterSignupEvent({ action: 'VIEW', identityName: IDENTITY_NAME, - componentId: NEWSLETTER_SIGNUP_COMPONENT_ID.control(IDENTITY_NAME), + componentId: + NEWSLETTER_SIGNUP_COMPONENT_ID.inArticleSignupForm( + IDENTITY_NAME, + ), renderingTarget: RENDERING_TARGET, value: { eventDescription: 'newsletter-signup-viewed' }, }); @@ -54,7 +52,7 @@ describe('sendNewsletterSignupEvent', () => { { component: { componentType: 'NEWSLETTER_SUBSCRIPTION', - id: `AR SecureSignup ${IDENTITY_NAME}`, + id: `AR NewsletterSignupForm ${IDENTITY_NAME}`, }, action: 'VIEW', value: JSON.stringify({ @@ -75,7 +73,7 @@ describe('sendNewsletterSignupEvent', () => { action: 'EXPAND', identityName: IDENTITY_NAME, componentId: - NEWSLETTER_SIGNUP_COMPONENT_ID.variantIllustratedCard( + NEWSLETTER_SIGNUP_COMPONENT_ID.inArticleSignupForm( IDENTITY_NAME, ), renderingTarget: RENDERING_TARGET, @@ -86,7 +84,7 @@ describe('sendNewsletterSignupEvent', () => { { component: { componentType: 'NEWSLETTER_SUBSCRIPTION', - id: `AR NewsletterSignupForm ${IDENTITY_NAME} - variantIllustratedCard`, + id: `AR NewsletterSignupForm ${IDENTITY_NAME}`, }, action: 'EXPAND', value: JSON.stringify({ @@ -106,7 +104,7 @@ describe('sendNewsletterSignupEvent', () => { action: 'CLOSE', identityName: IDENTITY_NAME, componentId: - NEWSLETTER_SIGNUP_COMPONENT_ID.variantIllustratedCard( + NEWSLETTER_SIGNUP_COMPONENT_ID.inArticleSignupForm( IDENTITY_NAME, ), renderingTarget: RENDERING_TARGET, @@ -130,21 +128,27 @@ describe('sendNewsletterSignupEvent', () => { }); it('passes the abTest object to the top-level abTest field (not encoded in value)', () => { + const abTest = { + name: 'ab-test-name', + variant: 'ab-test-variant', + }; + sendNewsletterSignupEvent({ action: 'VIEW', identityName: IDENTITY_NAME, - componentId: NEWSLETTER_SIGNUP_COMPONENT_ID.control(IDENTITY_NAME), + componentId: + NEWSLETTER_SIGNUP_COMPONENT_ID.highlightsCard(IDENTITY_NAME), renderingTarget: RENDERING_TARGET, value: { eventDescription: 'newsletter-signup-viewed', isAlreadySubscribed: false, }, - abTest: { name: AB_TEST_NAME, variant: 'control' }, + abTest, }); expect(submitComponentEvent).toHaveBeenCalledWith( expect.objectContaining({ - abTest: { name: AB_TEST_NAME, variant: 'control' }, + abTest, value: JSON.stringify({ eventDescription: 'newsletter-signup-viewed', isAlreadySubscribed: false, diff --git a/dotcom-rendering/src/lib/newsletterSignupTracking.ts b/dotcom-rendering/src/lib/newsletterSignupTracking.ts index 2c67124dbb5..623377943bf 100644 --- a/dotcom-rendering/src/lib/newsletterSignupTracking.ts +++ b/dotcom-rendering/src/lib/newsletterSignupTracking.ts @@ -2,8 +2,6 @@ import type { AbTest, TAction } from '@guardian/ophan-tracker-js'; import { submitComponentEvent } from '../client/ophan/ophan'; import type { RenderingTarget } from '../types/renderingTarget'; -export const AB_TEST_NAME = 'newsletters-signup-card-country-illustration'; - export type NewsletterEventDescription = | 'click-button' | 'email-input-focused' @@ -36,20 +34,15 @@ export const EVENT_DESCRIPTION_TO_ACTION = { } as const satisfies Record; /** - * The component ids used by each branch of the newsletter signup A/B test. + * Stable component IDs for newsletter signup tracking. * Keeping them here ensures the VIEW event (fired on mount in EmailSignUpWrapper) - * and subsequent events (EXPAND/CLOSE in NewsletterSignupCardContainer, SUBSCRIBE - * in SecureSignup) all share the same id, so they can be joined in Ophan. - * - * Note: both ids use the `AR` prefix as a shared convention, keeping them - * consistent with the pre-existing `SecureSignup` component id. + * and subsequent interaction events all share the same id so they can be + * joined in Ophan. */ export const NEWSLETTER_SIGNUP_COMPONENT_ID = { - control: (identityName: string) => `AR SecureSignup ${identityName}`, - variantNewField: (identityName: string) => - `AR SecureSignup ${identityName} - variantNewField`, - variantIllustratedCard: (identityName: string) => - `AR NewsletterSignupForm ${identityName} - variantIllustratedCard`, + secureSignup: (identityName: string) => `AR SecureSignup ${identityName}`, + inArticleSignupForm: (identityName: string) => + `AR NewsletterSignupForm ${identityName}`, highlightsCard: (identityName: string) => `HighlightsNewsletterCard ${identityName}`, } as const; @@ -57,9 +50,8 @@ export const NEWSLETTER_SIGNUP_COMPONENT_ID = { /** * Sends a newsletter signup tracking event to Ophan. * - * Used by both branches of the A/B test so that all events share a consistent - * component shape. Import this wherever you need to track an interaction with - * the email signup — e.g. VIEW on mount, EXPAND/CLOSE on preview open/close, + * Import this wherever you need to track an interaction with the email signup — + * e.g. VIEW on mount, EXPAND/CLOSE on preview open/close, * CLICK/ANSWER/SUBSCRIBE on form interactions. */ export const sendNewsletterSignupEvent = ({ diff --git a/dotcom-rendering/src/lib/renderElement.tsx b/dotcom-rendering/src/lib/renderElement.tsx index 0a1593d173c..f877210f059 100644 --- a/dotcom-rendering/src/lib/renderElement.tsx +++ b/dotcom-rendering/src/lib/renderElement.tsx @@ -595,15 +595,10 @@ export const renderElement = ({ description: newsletter.description, name: newsletter.name, frequency: newsletter.frequency, - successDescription: newsletter.successDescription, theme: newsletter.theme, illustrationSquare: newsletter.illustrationSquare, exampleUrl: newsletter.exampleUrl, idApiUrl: idApiUrl ?? '', - hideNewsletterSignupComponentForSubscribers: - !!switches.hideNewsletterSignupComponentForSubscribers, - showNewNewsletterSignupCard: - !!switches.showNewNewsletterSignupCard, }; if (isListElement || isTimeline) { return null; diff --git a/dotcom-rendering/src/lib/useNewsletterSignupForm.ts b/dotcom-rendering/src/lib/useNewsletterSignupForm.ts index 6cfdccc54ac..81d830c2931 100644 --- a/dotcom-rendering/src/lib/useNewsletterSignupForm.ts +++ b/dotcom-rendering/src/lib/useNewsletterSignupForm.ts @@ -12,7 +12,6 @@ import { } from './newsletter-marketing-opt-in'; import { EVENT_DESCRIPTION_TO_ACTION, - NEWSLETTER_SIGNUP_COMPONENT_ID, type NewsletterEventDescription, sendNewsletterSignupEvent, } from './newsletterSignupTracking'; @@ -110,34 +109,15 @@ const postFormData = async ( }); }; -// TODO: when the in-article newsletter signup AB test (newsletters-signup-card-country-illustration) -// is cleaned up, refactor getComponentId (and useNewsletterSignupForm) to accept an explicit -// componentId rather than deriving it from abTest.variant, so tracking is not coupled to test state. -const getComponentId = (newsletterId: string, abTest?: AbTest): string => { - switch (abTest?.variant) { - case 'variantIllustratedCard': - return NEWSLETTER_SIGNUP_COMPONENT_ID.variantIllustratedCard( - newsletterId, - ); - case 'variantNewField': - return NEWSLETTER_SIGNUP_COMPONENT_ID.variantNewField(newsletterId); - case 'highlightsCard': - return NEWSLETTER_SIGNUP_COMPONENT_ID.highlightsCard(newsletterId); - case undefined: - default: - return NEWSLETTER_SIGNUP_COMPONENT_ID.control(newsletterId); - } -}; - const sendTracking = ( newsletterId: string, + componentId: string, eventDescription: NewsletterEventDescription, renderingTarget: RenderingTarget, isSignedIn: boolean | 'Pending', abTest?: AbTest, extraDetails?: Record, ): void => { - const componentId = getComponentId(newsletterId, abTest); sendNewsletterSignupEvent({ action: EVENT_DESCRIPTION_TO_ACTION[eventDescription], identityName: newsletterId, @@ -233,6 +213,7 @@ export type NewsletterSignupFormState = { export const useNewsletterSignupForm = ( newsletterId: string, renderingTarget: RenderingTarget, + componentId: string, abTest?: AbTest, hideMarketingToggle = false, ): NewsletterSignupFormState => { @@ -351,6 +332,7 @@ export const useNewsletterSignupForm = ( sendTracking( newsletterId, + componentId, 'form-submission', renderingTarget, isSignedIn, @@ -388,6 +370,7 @@ export const useNewsletterSignupForm = ( sendTracking( newsletterId, + componentId, response.ok ? 'submission-confirmed' : 'submission-failed', renderingTarget, isSignedIn, @@ -395,7 +378,7 @@ export const useNewsletterSignupForm = ( marketingOptInType ? { marketingOptInType } : undefined, ); }, - [abTest, isSignedIn, newsletterId, renderingTarget], + [abTest, componentId, isSignedIn, newsletterId, renderingTarget], ); const handleCaptchaComplete = useCallback( @@ -403,6 +386,7 @@ export const useNewsletterSignupForm = ( if (!token) { sendTracking( newsletterId, + componentId, 'captcha-not-passed', renderingTarget, isSignedIn, @@ -416,6 +400,7 @@ export const useNewsletterSignupForm = ( } sendTracking( newsletterId, + componentId, 'captcha-passed', renderingTarget, isSignedIn, @@ -430,6 +415,7 @@ export const useNewsletterSignupForm = ( console.error(error); sendTracking( newsletterId, + componentId, 'form-submit-error', renderingTarget, isSignedIn, @@ -445,12 +431,20 @@ export const useNewsletterSignupForm = ( setIsWaitingForResponse(false); }); }, - [abTest, isSignedIn, newsletterId, renderingTarget, submitForm], + [ + abTest, + componentId, + isSignedIn, + newsletterId, + renderingTarget, + submitForm, + ], ); const handleCaptchaLoadError = useCallback((): void => { sendTracking( newsletterId, + componentId, 'captcha-load-error', renderingTarget, isSignedIn, @@ -460,7 +454,7 @@ export const useNewsletterSignupForm = ( setErrorMessage('Sorry, the reCAPTCHA failed to load.'); setIsWaitingForResponse(false); recaptchaRef.current?.reset(); - }, [abTest, isSignedIn, newsletterId, renderingTarget]); + }, [abTest, componentId, isSignedIn, newsletterId, renderingTarget]); const handleSubmit = useCallback( (event: FormEvent): void => { @@ -480,6 +474,7 @@ export const useNewsletterSignupForm = ( setIsWaitingForResponse(true); sendTracking( newsletterId, + componentId, 'open-captcha', renderingTarget, isSignedIn, @@ -489,6 +484,7 @@ export const useNewsletterSignupForm = ( }, [ abTest, + componentId, isSignedIn, isWaitingForResponse, newsletterId, @@ -508,12 +504,13 @@ export const useNewsletterSignupForm = ( setIsInteracted(true); sendTracking( newsletterId, + componentId, 'email-input-focused', renderingTarget, isSignedIn, abTest, ); - }, [abTest, isSignedIn, newsletterId, renderingTarget]); + }, [abTest, componentId, isSignedIn, newsletterId, renderingTarget]); const handleEmailInvalid = useCallback< React.FormEventHandler @@ -540,12 +537,13 @@ export const useNewsletterSignupForm = ( hasAttemptedSubmitRef.current = true; sendTracking( newsletterId, + componentId, 'click-button', renderingTarget, isSignedIn, abTest, ); - }, [abTest, isSignedIn, newsletterId, renderingTarget]); + }, [abTest, componentId, isSignedIn, newsletterId, renderingTarget]); const handleReset = useCallback< ReactEventHandler