Revert "Revert "[WS-2817] Refactor toggles url"" - #14282
Merged
hotinglok merged 3 commits intoJul 31, 2026
Merged
Conversation
Isabella-Mitchell
approved these changes
Jul 30, 2026
Contributor
There was a problem hiding this comment.
Pull request overview
This PR removes the legacy getToggles (+ withCache) implementation and switches SSR/pages/tests to a new fetchToggles flow that builds a toggles endpoint from TOGGLES_BFF_PATH (and WEB_CDN_URL for AMP), updating related consumers (Next.js routes, Cypress, Playwright, AMP ads, and IDCTA config) accordingly.
Changes:
- Replace
getToggles/withCacheusage withfetchToggles({ service, isAmp })across the Next.js app routes and tests. - Introduce
fetchToggleswith LRU caching and a new endpoint constructor supporting AMP vs canonical. - Add a
getTogglehelper and update ad/IDCTA toggle reads and AMP access paths.
Reviewed changes
Copilot reviewed 38 out of 38 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| ws-nextjs-app/utilities/pageRequests/getPageData.test.ts | Updates toggles mocking to match new toggles fetcher. |
| ws-nextjs-app/playwright/pageTypes/liveRadioPage/liveRadioPage.spec.ts | Switches Playwright toggles fetch to TOGGLES_BFF_PATH. |
| ws-nextjs-app/pages/[service]/onDemandTv/handleOnDemandTvRoute.ts | Uses fetchToggles({ service }) instead of cached getToggles. |
| ws-nextjs-app/pages/[service]/onDemandTv/handleOnDemandTvRoute.test.ts | Updates mocks/imports for toggles fetching. |
| ws-nextjs-app/pages/[service]/onDemandAudio/handleOnDemandAudioRoute.ts | Uses fetchToggles({ service }) and keeps existing page logic. |
| ws-nextjs-app/pages/[service]/my-news/index.page.tsx | Uses fetchToggles({ service }) in SSR gating logic. |
| ws-nextjs-app/pages/[service]/my-news/index.page.test.tsx | Updates mocks and typing for toggles in SSR tests. |
| ws-nextjs-app/pages/[service]/liveRadio/handleLiveRadioRoute.ts | Moves toggles fetch after service validation and uses fetchToggles. |
| ws-nextjs-app/pages/[service]/liveRadio/handleLiveRadioRoute.test.ts | Updates mocks/imports for toggles fetching. |
| ws-nextjs-app/pages/[service]/downloads/[[...variant]].page.tsx | Uses fetchToggles({ service }) in SSR props. |
| ws-nextjs-app/pages/_app.page.tsx | Switches global toggles load in _app to fetchToggles({ service, isAmp }). |
| ws-nextjs-app/cypress/support/config/envs.ts | Removes togglesUrl from Cypress env config shape. |
| ws-nextjs-app/cypress/support/commands.ts | Updates Cypress cy.getToggles to use TOGGLES_BFF_PATH. |
| src/app/utilities/createAdNonce/index.ts | Switches toggle reads to getToggle utility. |
| src/app/lib/utilities/getToggles/withCache.test.js | Removes legacy cache wrapper tests. |
| src/app/lib/utilities/getToggles/withCache.js | Removes legacy LRU wrapper (withCache). |
| src/app/lib/utilities/getToggles/index.js | Removes legacy toggles fetch implementation. |
| src/app/lib/utilities/getToggles/index.client.test.js | Removes legacy toggles client tests. |
| src/app/lib/utilities/getToggleDefinition/index.ts | Refactors toggle-definition helper (now overlaps with getToggle). |
| src/app/lib/utilities/getToggleDefinition/index.test.ts | Removes tests for the old toggle-definition behaviour. |
| src/app/lib/utilities/getToggle/index.ts | Adds getToggle(toggles, name) helper. |
| src/app/lib/utilities/getToggle/index.test.ts | Adds unit tests for getToggle. |
| src/app/lib/utilities/fetchToggles/index.ts | Adds new cached toggles fetcher and logging. |
| src/app/lib/utilities/fetchToggles/index.client.test.ts | Adds tests for the new toggles fetcher. |
| src/app/lib/utilities/fetchConfig/index.ts | Minor formatting change to cached-return block. |
| src/app/lib/logger.const.js | Adds new toggle-related log codes. |
| src/app/lib/idcta/getIdctaConfig/index.ts | Uses getToggle for account toggle lookup. |
| src/app/lib/idcta/getIdctaConfig/index.test.ts | Updates tests to pass toggles directly (no definition mock). |
| src/app/lib/config/toggles/README.md | Updates documentation references to new toggles fetcher. |
| src/app/contexts/ToggleContext/utils/constructTogglesEndpoint/index.ts | Rebuilds endpoint constructor to use BFF path vs AMP web-cdn path. |
| src/app/contexts/ToggleContext/utils/constructTogglesEndpoint/index.test.ts | Updates endpoint constructor tests for new env vars and AMP path. |
| src/app/components/Ad/index.test.tsx | Ensures WEB_CDN_URL is set for AMP toggles endpoint. |
| src/app/components/Ad/Amp/index.tsx | Uses new toggles endpoint constructor and updates AMP access paths. |
| src/app/components/Ad/Amp/index.test.tsx | Updates AMP access fetch expectation for new endpoint. |
| src/app/components/Ad/Amp/snapshots/index.test.tsx.snap | Snapshot updates for AMP access path change. |
| src/app/components/Ad/snapshots/index.test.tsx.snap | Snapshot updates for AMP access path change. |
| scripts/checkSecretEnvVariables.sh | Adds check for TOGGLES_BFF_PATH. |
| cypress.config.ts | Injects TOGGLES_BFF_PATH into the Cypress webpack DefinePlugin env. |
Comments suppressed due to low confidence (1)
src/app/lib/utilities/fetchToggles/index.ts:99
fetchedTogglescan beundefinedif the response shape isn’t{ data: { toggles } }, which leads to cachingundefinedand throwing on object spread. Parsing should be resilient (and avoid relying on exceptions for control flow).
const responseBody = await response.json();
const fetchedToggles = responseBody?.data?.toggles;
cache.set(cacheKey, fetchedToggles);
Comment on lines
50
to
54
| cy.request({ | ||
| url: `${environmentConfig.togglesUrl}?application=simorgh&service=${service}&__amp_source_origin=${environmentConfig.baseUrl}`, | ||
| headers: { | ||
| Origin: 'https://www.bbc.com', | ||
| }, | ||
| url: `${process.env.TOGGLES_BFF_PATH}?service=${service}&application=simorgh`, | ||
| }).then(response => { | ||
| cy.writeFile(togglesFixture, response.body.toggles); | ||
| }); |
Comment on lines
+1
to
+5
| import { Toggles, ToggleDefinition } from '#app/models/types/global'; | ||
|
|
||
| const getToggleDefinitions = ( | ||
| toggles: Toggles = {}, | ||
| ): Record<string, ToggleDefinition> => { | ||
| const { _environment, ...toggleDefinitions } = toggles; | ||
| return toggleDefinitions; | ||
| }; | ||
| export default function getToggleDefinition( | ||
| toggles: Toggles, | ||
| name: string, |
| testToggle: { enabled: true }, | ||
| }; | ||
|
|
||
| describe('getToggles', () => { |
Comment on lines
68
to
70
| const response = await fetch( | ||
| `${togglesUrl}?application=simorgh&service=${service}&__amp_source_origin=${baseURL}`, | ||
| { | ||
| headers: { | ||
| Origin: 'https://www.bbc.com', | ||
| }, | ||
| }, | ||
| `${process.env.TOGGLES_BFF_PATH}?application=simorgh&service=${service}`, | ||
| ); |
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
hotinglok
approved these changes
Jul 31, 2026
hotinglok
deleted the
revert-14281-revert-14226-WS-2817-refactor-toggles-url
branch
July 31, 2026 09:44
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Reverts #14281