[WS-2817] Refactor toggles url

[hotinglok]

Resolves JIRA: WS-2817

Refactors how Simorgh builds the URL it uses to fetch feature toggles from the toggles BFF

Code changes

• constructTogglesEndpoint: build URL from TOGGLES_BFF_PATH environment variable + application=simorgh|amp, replacing the SIMORGH_CONFIG_URL + origin/__amp_source_origin approach.
• getToggles/withCache: switched to a single options object argument ({ service, cache, isAmp }); withCache now fetches and merges amp toggles on top of simorgh toggles when isAmp is true.
• Updated all call sites (_app.page.tsx, handleLiveRadioRoute.ts, handleOnDemandAudioRoute.ts, handleOnDemandTvRoute.ts, my-news/index.page.tsx, downloads/[[...variant]].page.tsx) to the new call signature.
• handleLiveRadioRoute.ts: moved the !service not-found early-return above the getToggles call (fixes a TS error where service: string | null was passed to a function expecting string, and matches the pattern already used in the on-demand audio/TV route handlers).
• Refactored getToggles() to fetchToggles() to match fetchConfig().
• Updated index file to typescript.
• Removed withCache as this logic has been moved within the index file and more closely follows the pattern of fetchConfig().
• Updated Amp Ad component to allow amp-access to fetch from new endpoint via the /fd/ route.

Testing

1. Before merging: confirm TOGGLES_BFF_PATH are added to all envConfig/*.env files and to getEnvConfig's getProcessEnvAppVariables() so the new URL construction works in deployed environments and on the client.
2. Merge branch to preview and visit any page (e.g. a live radio page).

Useful Links

• Coding Standards
• Repository use guidelines
• Add Links to useful resources related to this PR if applicable.

[Copilot]

Pull request overview

Refactors how Simorgh constructs and fetches feature toggles by switching to a TOGGLES_BFF_PATH-based endpoint and updating the toggles fetching API to use a single options object (including AMP support via merging AMP toggles over canonical toggles).

Changes:

• Updates toggle URL construction to use TOGGLES_BFF_PATH + application=simorgh|amp.
• Refactors getToggles and withCache to accept a single options object and (when isAmp: true) fetch + merge AMP toggles on top.
• Updates Next.js route/page call sites and adds/updates unit tests for the new behaviour.

Reviewed changes

Copilot reviewed 12 out of 12 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
ws-nextjs-app/pages/[service]/onDemandTv/handleOnDemandTvRoute.ts	Updates toggles call signature to the new options-object API.
ws-nextjs-app/pages/[service]/onDemandAudio/handleOnDemandAudioRoute.ts	Updates toggles call signature; adjusts formatting around podcast external-links fetch.
ws-nextjs-app/pages/[service]/my-news/index.page.tsx	Updates toggles call signature to the new options-object API.
ws-nextjs-app/pages/[service]/liveRadio/handleLiveRadioRoute.ts	Moves not-found early return before toggles fetch; updates toggles call signature.
ws-nextjs-app/pages/[service]/downloads/[[...variant]].page.tsx	Updates toggles call signature to the new options-object API.
ws-nextjs-app/pages/_app.page.tsx	Passes isAmp into toggles fetch to support AMP toggle merging.
src/app/lib/utilities/getToggles/withCache.test.js	Updates tests for new withCache({ service, isAmp }) behaviour and AMP merge precedence.
src/app/lib/utilities/getToggles/withCache.js	Implements AMP toggle fetching + merge when isAmp: true; switches to options object signature.
src/app/lib/utilities/getToggles/index.js	Refactors getToggles to accept options object and uses constructTogglesEndpoint(service, isAmp).
src/app/lib/utilities/getToggles/index.client.test.js	Updates tests for new endpoint format and new getToggles({ ... }) signature.
src/app/contexts/ToggleContext/utils/constructTogglesEndpoint/index.ts	Switches endpoint construction to TOGGLES_BFF_PATH and `application=simorgh
src/app/contexts/ToggleContext/utils/constructTogglesEndpoint/index.test.ts	Updates tests for the new endpoint construction and AMP application selection.

[Isabella-Mitchell]

Am I right in thinking this won't be an issue here since this isn't clientside code? Or I have I misunderstood this? Sorry I got totally muddled and thought this was an infra PR.

Is this going to be an issue?

[hotinglok]

Yes you're correct @Isabella-Mitchell, TOGGLES_BFF_PATH was added here in simorgh-infrastructure so it should be pulled through without being exposed.

[Isabella-Mitchell]

Thanks - I'm not too hot on how env variables work.

Do we need add anything to https://github.com/bbc/simorgh/blob/WS-2817-refactor-toggles-url/src/app/lib/utilities/getEnvConfig/index.ts#L7?

[hotinglok]

The above location shouldn't need it since those are only for the env variables exposed in the simorgh envConfig files.

[Isabella-Mitchell]

Do we need to do anything for the cy.getToggles() method used in the e2e tests? E.g. https://github.com/bbc/simorgh/blob/WS-2817-refactor-toggles-url/cypress/README.md#toggles

[Isabella-Mitchell]

Thanks - I agree with the suggested testing approach of releasing this to preview and checking it over there.

[Copilot]

Pull request overview

Copilot reviewed 36 out of 36 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (6)

src/app/lib/utilities/fetchConfig/index.ts:62

• Avoid console.log in the hot cache-hit path; it’s noisy and can materially increase log volume. The structured logger call above already records cache hits via the cached flag.

  if (cachedResponse) {
    console.log('[fetchConfig] cache hit, returning cached response');
    return cachedResponse as T;
  }

src/app/lib/utilities/fetchConfig/index.ts:80

• Logging the entire config response body can leak data into logs and is likely to be very high volume. Prefer removing this or logging only safe metadata.

      console.log('[fetchConfig] response body:', res);

src/app/contexts/ToggleContext/utils/constructTogglesEndpoint/index.test.ts:10

• Reassigning process.env to a new object is brittle in Jest/Node (it can drop non-enumerable properties and makes it easy to accidentally lose env var mutations from other tests). It’s safer to set/restore only the specific env vars under test.

const originalEnv = process.env;

beforeEach(() => {
  process.env = {
    ...originalEnv,
    TOGGLES_BFF_PATH: 'https://some-url.co.uk/toggles',
    WEB_CDN_URL: 'https://web-cdn.test',
  };

src/app/lib/utilities/fetchToggles/index.ts:53

• constructTogglesEndpoint() can throw (e.g. when TOGGLES_BFF_PATH/WEB_CDN_URL is unset), but it’s called outside the try block here. That means missing/invalid env will crash the request instead of falling back to localToggles (which the script messaging suggests is the intended behaviour).

  const togglesEndpoint = constructTogglesEndpoint({ service, isAmp });

  const isLocal = appEnvironment === 'local';
  const serviceEnv = isLocal ? 'test' : appEnvironment;
  const cacheKey = `${togglesEndpoint}:${serviceEnv}`;

src/app/lib/utilities/fetchConfig/index.ts:49

• The console.log debug statement here will add noisy logs in production/server output and bypasses the structured logger used elsewhere in this function.

This issue also appears in the following locations of the same file:

• line 59
• line 80

  console.log('[fetchConfig] endpoint:', bffReqPath);

scripts/checkSecretEnvVariables.sh:8

• This script has a duplicate shebang mid-file, which can confuse editors/tooling and is unnecessary. Keep a single shebang at the top and include both checks in the same script body.

#!/bin/sh
if [ -z "$TOGGLES_BFF_PATH" ]; then 
    echo No TOGGLES_BFF_PATH set as environment variable, local toggles values will be used; 

[Isabella-Mitchell]

Can we do this here -
const { enabled: isAccountEnabled, value: accountService = '' } = toggles.account || {};

I can see a few places in this PR where we've updated it to something like "'x' in toggles". I'm not so used to seeing this. So wondering is there any benefit to doing it this way?

[emilysaffron]

+1

[hotinglok]

I think this was rewritten as part of folding getToggleDefinitions() into fetchToggles(). It's a little less readable now that I look at it again. I might undo this change and keep getToggleDefinitions() as a separate utility, will look into some alternatives too.