diff --git a/dotcom-rendering/src/components/EmailSignUpWrapper.importable.tsx b/dotcom-rendering/src/components/EmailSignUpWrapper.importable.tsx index 42511e07716..21b0c41e6a5 100644 --- a/dotcom-rendering/src/components/EmailSignUpWrapper.importable.tsx +++ b/dotcom-rendering/src/components/EmailSignUpWrapper.importable.tsx @@ -25,6 +25,8 @@ interface EmailSignUpWrapperProps extends EmailSignUpProps { idApiUrl: string; /** You should only set this to true if the privacy message will be shown elsewhere on the page */ hidePrivacyMessage?: boolean; + /** Feature flag to enable hiding newsletter signup for already subscribed users */ + hideNewsletterSignupComponentForSubscribers?: boolean; } /** @@ -41,9 +43,14 @@ export const EmailSignUpWrapper = ({ index, listId, idApiUrl, + hideNewsletterSignupComponentForSubscribers = false, ...emailSignUpProps }: EmailSignUpWrapperProps) => { - const isSubscribed = useNewsletterSubscription(listId, idApiUrl); + const isSubscribed = useNewsletterSubscription( + listId, + idApiUrl, + hideNewsletterSignupComponentForSubscribers, + ); // Show placeholder while subscription status is being determined // This prevents layout shift in both subscribed and non-subscribed cases diff --git a/dotcom-rendering/src/components/EmailSignUpWrapper.stories.tsx b/dotcom-rendering/src/components/EmailSignUpWrapper.stories.tsx index 15edf86e418..8b6cd7a29ab 100644 --- a/dotcom-rendering/src/components/EmailSignUpWrapper.stories.tsx +++ b/dotcom-rendering/src/components/EmailSignUpWrapper.stories.tsx @@ -83,14 +83,42 @@ export const SignedInNotSubscribed: Story = { // User is signed in and IS subscribed - component returns null (hidden) // Note: This story will render nothing as the component returns null when subscribed +// Requires hideNewsletterSignupComponentForSubscribers: true to enable the subscription check export const SignedInAlreadySubscribed: Story = { args: { hidePrivacyMessage: false, ...defaultArgs, + hideNewsletterSignupComponentForSubscribers: true, }, async beforeEach() { mocked(useNewsletterSubscription).mockReturnValue(true); }, }; +// Feature flag disabled - always shows signup form regardless of subscription status +// When hideNewsletterSignupComponentForSubscribers is false, the subscription check is skipped +export const FeatureFlagDisabled: Story = { + args: { + hidePrivacyMessage: false, + ...defaultArgs, + hideNewsletterSignupComponentForSubscribers: false, + }, + async beforeEach() { + // Even though we mock this to return true (subscribed), + // the feature flag being disabled means it won't be checked + 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 default meta; diff --git a/dotcom-rendering/src/lib/renderElement.tsx b/dotcom-rendering/src/lib/renderElement.tsx index b49f86a82df..360cb08a780 100644 --- a/dotcom-rendering/src/lib/renderElement.tsx +++ b/dotcom-rendering/src/lib/renderElement.tsx @@ -577,6 +577,8 @@ export const renderElement = ({ successDescription: element.newsletter.successDescription, theme: element.newsletter.theme, idApiUrl: idApiUrl ?? '', + hideNewsletterSignupComponentForSubscribers: + !!switches.hideNewsletterSignupComponentForSubscribers, }; if (isListElement || isTimeline) return null; return ( diff --git a/dotcom-rendering/src/lib/useNewsletterSubscription.test.ts b/dotcom-rendering/src/lib/useNewsletterSubscription.test.ts index c42cb1ed987..a06371ee9a5 100644 --- a/dotcom-rendering/src/lib/useNewsletterSubscription.test.ts +++ b/dotcom-rendering/src/lib/useNewsletterSubscription.test.ts @@ -46,178 +46,266 @@ describe('useNewsletterSubscription', () => { jest.restoreAllMocks(); }); - it('should return undefined while auth status is pending', () => { - mockUseAuthStatus.mockReturnValue({ kind: 'Pending' }); + describe('when feature flag is disabled (enableCheck = false)', () => { + it('should return false immediately without making API call', async () => { + mockUseAuthStatus.mockReturnValue({ + kind: 'SignedIn', + accessToken: {} as never, + idToken: {} as never, + }); + + const { result } = renderHook(() => + useNewsletterSubscription( + mockNewsletterId, + mockIdApiUrl, + false, + ), + ); - const { result } = renderHook(() => - useNewsletterSubscription(mockNewsletterId, mockIdApiUrl), - ); + await waitFor(() => { + expect(result.current).toBe(false); + }); - expect(result.current).toBeUndefined(); - expect(global.fetch).not.toHaveBeenCalled(); + expect(global.fetch).not.toHaveBeenCalled(); + }); }); - it('should return false when user is signed out', async () => { - mockUseAuthStatus.mockReturnValue({ kind: 'SignedOut' }); + describe('when feature flag is enabled (enableCheck = true)', () => { + it('should return undefined while auth status is pending', () => { + mockUseAuthStatus.mockReturnValue({ kind: 'Pending' }); - const { result } = renderHook(() => - useNewsletterSubscription(mockNewsletterId, mockIdApiUrl), - ); + const { result } = renderHook(() => + useNewsletterSubscription(mockNewsletterId, mockIdApiUrl, true), + ); - await waitFor(() => { - expect(result.current).toBe(false); + expect(result.current).toBeUndefined(); + expect(global.fetch).not.toHaveBeenCalled(); }); - expect(global.fetch).not.toHaveBeenCalled(); - }); + it('should return false when user is signed out', async () => { + mockUseAuthStatus.mockReturnValue({ kind: 'SignedOut' }); - it('should return false when idApiUrl is undefined', async () => { - mockUseAuthStatus.mockReturnValue({ - kind: 'SignedIn', - accessToken: {} as never, - idToken: idTokenMock as never, - }); + const { result } = renderHook(() => + useNewsletterSubscription(mockNewsletterId, mockIdApiUrl, true), + ); - const { result } = renderHook(() => - useNewsletterSubscription(mockNewsletterId, undefined), - ); + await waitFor(() => { + expect(result.current).toBe(false); + }); - await waitFor(() => { - expect(result.current).toBe(false); + expect(global.fetch).not.toHaveBeenCalled(); }); - expect(global.fetch).not.toHaveBeenCalled(); - }); + it('should return false when idApiUrl is undefined', async () => { + mockUseAuthStatus.mockReturnValue({ + kind: 'SignedIn', + accessToken: {} as never, + idToken: idTokenMock as never, + }); - it('should return true when user is subscribed to the newsletter', async () => { - mockUseAuthStatus.mockReturnValue({ - kind: 'SignedIn', - accessToken: {} as never, - idToken: idTokenMock as never, - }); + const { result } = renderHook(() => + useNewsletterSubscription(mockNewsletterId, undefined, true), + ); - (global.fetch as jest.Mock).mockResolvedValueOnce({ - ok: true, - json: async () => ({ - result: { - subscriptions: [ - { listId: '1234' }, - { listId: String(mockNewsletterId) }, - { listId: '5678' }, - ], - }, - }), + await waitFor(() => { + expect(result.current).toBe(false); + }); + + expect(global.fetch).not.toHaveBeenCalled(); }); - const { result } = renderHook(() => - useNewsletterSubscription(mockNewsletterId, mockIdApiUrl), - ); + it('should return true when user is subscribed to the newsletter', async () => { + mockUseAuthStatus.mockReturnValue({ + kind: 'SignedIn', + accessToken: {} as never, + idToken: idTokenMock as never, + }); + + (global.fetch as jest.Mock).mockResolvedValueOnce({ + ok: true, + json: async () => ({ + result: { + subscriptions: [ + { listId: '1234' }, + { listId: String(mockNewsletterId) }, + ], + }, + }), + }); + + const { result } = renderHook(() => + useNewsletterSubscription(mockNewsletterId, mockIdApiUrl, true), + ); - await waitFor(() => { - expect(result.current).toBe(true); + await waitFor(() => { + expect(result.current).toBe(true); + }); }); - expect(global.fetch).toHaveBeenCalledWith( - `${mockIdApiUrl}/users/me/newsletters`, - expect.objectContaining({ - method: 'GET', - credentials: 'include', - }), - ); - }); + it('should return false when user is not subscribed to the newsletter', async () => { + mockUseAuthStatus.mockReturnValue({ + kind: 'SignedIn', + accessToken: {} as never, + idToken: idTokenMock as never, + }); + + (global.fetch as jest.Mock).mockResolvedValueOnce({ + ok: true, + json: async () => ({ + result: { + subscriptions: [{ listId: '1234' }, { listId: '5678' }], + }, + }), + }); + + const { result } = renderHook(() => + useNewsletterSubscription(mockNewsletterId, mockIdApiUrl, true), + ); - it('should return false when user is not subscribed to the newsletter', async () => { - mockUseAuthStatus.mockReturnValue({ - kind: 'SignedIn', - accessToken: {} as never, - idToken: idTokenMock as never, + await waitFor(() => { + expect(result.current).toBe(false); + }); }); - (global.fetch as jest.Mock).mockResolvedValueOnce({ - ok: true, - json: async () => ({ - result: { - subscriptions: [{ listId: '1234' }, { listId: '5678' }], + it('should return false when API returns non-ok response', async () => { + const sentryReportErrorMock = jest.fn(); + window.guardian = { + modules: { + sentry: { + reportError: sentryReportErrorMock, + }, }, - }), - }); + } as unknown as typeof window.guardian; + + mockUseAuthStatus.mockReturnValue({ + kind: 'SignedIn', + accessToken: {} as never, + idToken: idTokenMock as never, + }); + + (global.fetch as jest.Mock).mockResolvedValueOnce({ + ok: false, + status: 401, + }); - const { result } = renderHook(() => - useNewsletterSubscription(mockNewsletterId, mockIdApiUrl), - ); + const { result } = renderHook(() => + useNewsletterSubscription(mockNewsletterId, mockIdApiUrl, true), + ); - await waitFor(() => { - expect(result.current).toBe(false); + await waitFor(() => { + expect(result.current).toBe(false); + }); + + expect(sentryReportErrorMock).toHaveBeenCalledWith( + expect.any(Error), + 'errors-fetching-newsletters', + ); }); - }); - it('should return false when API returns non-ok response', async () => { - const sentryReportErrorMock = jest.fn(); - window.guardian = { - modules: { - sentry: { - reportError: sentryReportErrorMock, + it('should return false when fetch throws an error', async () => { + const sentryReportErrorMock = jest.fn(); + window.guardian = { + modules: { + sentry: { + reportError: sentryReportErrorMock, + }, }, - }, - } as unknown as typeof window.guardian; + } as unknown as typeof window.guardian; - mockUseAuthStatus.mockReturnValue({ - kind: 'SignedIn', - accessToken: {} as never, - idToken: idTokenMock as never, - }); + mockUseAuthStatus.mockReturnValue({ + kind: 'SignedIn', + accessToken: {} as never, + idToken: idTokenMock as never, + }); - (global.fetch as jest.Mock).mockResolvedValueOnce({ - ok: false, - status: 401, - }); + (global.fetch as jest.Mock).mockRejectedValueOnce( + new Error('Network error'), + ); - const { result } = renderHook(() => - useNewsletterSubscription(mockNewsletterId, mockIdApiUrl), - ); + const { result } = renderHook(() => + useNewsletterSubscription(mockNewsletterId, mockIdApiUrl, true), + ); - await waitFor(() => { - expect(result.current).toBe(false); - }); + await waitFor(() => { + expect(result.current).toBe(false); + }); - expect(sentryReportErrorMock).toHaveBeenCalledWith( - expect.any(Error), - 'errors-fetching-newsletters', - ); + expect(sentryReportErrorMock).toHaveBeenCalledWith( + expect.any(Error), + 'errors-fetching-newsletters', + ); + }); }); - it('should return false when fetch throws an error', async () => { - const sentryReportErrorMock = jest.fn(); - window.guardian = { - modules: { - sentry: { - reportError: sentryReportErrorMock, + describe('default behavior (enableCheck defaults to true)', () => { + it('should return false when API returns non-ok response', async () => { + const sentryReportErrorMock = jest.fn(); + window.guardian = { + modules: { + sentry: { + reportError: sentryReportErrorMock, + }, }, - }, - } as unknown as typeof window.guardian; + } as unknown as typeof window.guardian; - mockUseAuthStatus.mockReturnValue({ - kind: 'SignedIn', - accessToken: {} as never, - idToken: idTokenMock as never, - }); + mockUseAuthStatus.mockReturnValue({ + kind: 'SignedIn', + accessToken: {} as never, + idToken: idTokenMock as never, + }); + + (global.fetch as jest.Mock).mockResolvedValueOnce({ + ok: false, + status: 401, + }); - (global.fetch as jest.Mock).mockRejectedValueOnce( - new Error('Network error'), - ); + const { result } = renderHook(() => + useNewsletterSubscription(mockNewsletterId, mockIdApiUrl), + ); - const { result } = renderHook(() => - useNewsletterSubscription(mockNewsletterId, mockIdApiUrl), - ); + await waitFor(() => { + expect(result.current).toBe(false); + }); - await waitFor(() => { - expect(result.current).toBe(false); + expect(sentryReportErrorMock).toHaveBeenCalledWith( + expect.any(Error), + 'errors-fetching-newsletters', + ); }); - expect(sentryReportErrorMock).toHaveBeenCalledWith( - expect.any(Error), - 'errors-fetching-newsletters', - ); + it('should return false when fetch throws an error', async () => { + const sentryReportErrorMock = jest.fn(); + window.guardian = { + modules: { + sentry: { + reportError: sentryReportErrorMock, + }, + }, + } as unknown as typeof window.guardian; + + mockUseAuthStatus.mockReturnValue({ + kind: 'SignedIn', + accessToken: {} as never, + idToken: idTokenMock as never, + }); + + (global.fetch as jest.Mock).mockRejectedValueOnce( + new Error('Network error'), + ); + + const { result } = renderHook(() => + useNewsletterSubscription(mockNewsletterId, mockIdApiUrl), + ); + + await waitFor(() => { + expect(result.current).toBe(false); + }); + + expect(sentryReportErrorMock).toHaveBeenCalledWith( + expect.any(Error), + 'errors-fetching-newsletters', + ); + }); }); describe('newsletter subscription data caching', () => { diff --git a/dotcom-rendering/src/lib/useNewsletterSubscription.ts b/dotcom-rendering/src/lib/useNewsletterSubscription.ts index a4bc1ac1d79..13890450c1b 100644 --- a/dotcom-rendering/src/lib/useNewsletterSubscription.ts +++ b/dotcom-rendering/src/lib/useNewsletterSubscription.ts @@ -9,10 +9,15 @@ import { useAuthStatus } from './useAuthStatus'; /** * A hook to check if a user is subscribed to a specific newsletter. + * + * @param newsletterId + * @param idApiUrl + * @param shouldCheckSubscription - Feature flag to enable/disable subscription check. When false, returns false immediately. */ export const useNewsletterSubscription = ( newsletterId: number, idApiUrl: string | undefined, + shouldCheckSubscription: boolean = true, ): boolean | undefined => { const [isSubscribed, setIsSubscribed] = useState( undefined, @@ -21,6 +26,12 @@ export const useNewsletterSubscription = ( const authStatus = useAuthStatus(); useEffect(() => { + // Feature flag is disabled - skip subscription check + if (!shouldCheckSubscription) { + setIsSubscribed(false); + return; + } + // Wait for auth to be determined if (authStatus.kind === 'Pending') { setIsSubscribed(undefined); @@ -74,7 +85,7 @@ export const useNewsletterSubscription = ( }; void fetchNewsletters(); - }, [authStatus, newsletterId, idApiUrl]); + }, [authStatus, newsletterId, idApiUrl, shouldCheckSubscription]); return isSubscribed; };