From d3a3e6ded3fec325e16a7ac9d887277e3da08268 Mon Sep 17 00:00:00 2001 From: Talisson Costa Date: Thu, 3 Sep 2026 15:13:13 -0300 Subject: [PATCH] refactor(routes): give the route table a module without components web/routes imports App, so every component reading a path from it forms a cycle. That already cost us once: reading the map at module level in App got an empty object and the app failed to boot, worked around by reading it lazily inside the function. The table moves to web/routePaths, which imports nothing but react-router. web/routes re-exports it, so nothing outside changes, and the three components that wanted a path now take it from there. The lazy read goes with the cycle. isAllowedWhileBlocked moves too, and is now testable without pulling in the app. jest gains the web/ alias webpack already resolves, without which it cannot follow those imports. Co-Authored-By: Claude Opus 5 (1M context) --- frontend/jest.config.js | 2 + frontend/web/__tests__/routePaths.test.ts | 29 ++++++ .../web/components/AnnouncementPerPage.tsx | 2 +- frontend/web/components/App.js | 9 +- .../components/pages/OrganisationsPage.tsx | 2 +- frontend/web/routePaths.ts | 98 +++++++++++++++++++ frontend/web/routes.js | 83 +--------------- 7 files changed, 134 insertions(+), 91 deletions(-) create mode 100644 frontend/web/__tests__/routePaths.test.ts create mode 100644 frontend/web/routePaths.ts diff --git a/frontend/jest.config.js b/frontend/jest.config.js index 0d44ad00a7df..cf20584638f9 100644 --- a/frontend/jest.config.js +++ b/frontend/jest.config.js @@ -13,6 +13,8 @@ module.exports = { '^common/(.*)$': '/common/$1', '^components/(.*)$': '/web/components/$1', '^project/(.*)$': '/web/project/$1', + // webpack resolves this one too; without it jest cannot follow the app. + '^web/(.*)$': '/web/$1', }, preset: 'ts-jest', roots: [''], diff --git a/frontend/web/__tests__/routePaths.test.ts b/frontend/web/__tests__/routePaths.test.ts new file mode 100644 index 000000000000..ea573e6f4f8b --- /dev/null +++ b/frontend/web/__tests__/routePaths.test.ts @@ -0,0 +1,29 @@ +import { isAllowedWhileBlocked } from 'web/routePaths' + +// App renders for a blocked organisation on everything this +// returns false for, so a wrong answer here either locks them out of the page +// that explains the block or lets them back into the app. +describe('isAllowedWhileBlocked', () => { + it.each` + pathname | allowed + ${'/organisation/7528/usage'} | ${true} + ${'/organisations'} | ${true} + ${'/organisation/7528/projects'} | ${false} + ${'/organisation/7528/settings'} | ${false} + ${'/organisation-settings'} | ${false} + ${'/project/1/environment/abc/features'} | ${false} + ${'/account'} | ${false} + `( + '$pathname is reachable while blocked: $allowed', + ({ allowed, pathname }) => { + expect(isAllowedWhileBlocked(pathname)).toBe(allowed) + }, + ) + + // The usage route is allowed by pattern, not by prefix. + it('does not open anything nested under the usage page', () => { + expect(isAllowedWhileBlocked('/organisation/7528/usage/breakdown')).toBe( + false, + ) + }) +}) diff --git a/frontend/web/components/AnnouncementPerPage.tsx b/frontend/web/components/AnnouncementPerPage.tsx index 67b4dca6b6f0..ab6835728698 100644 --- a/frontend/web/components/AnnouncementPerPage.tsx +++ b/frontend/web/components/AnnouncementPerPage.tsx @@ -4,7 +4,7 @@ import flagsmith from '@flagsmith/flagsmith' import Utils from 'common/utils/utils' import { AnnouncementValueType } from './Announcement' import { matchPath } from 'react-router-dom' -import { routes } from 'web/routes' +import { routes } from 'web/routePaths' type AnnouncementPerPageValueType = AnnouncementValueType & { pages: string[] diff --git a/frontend/web/components/App.js b/frontend/web/components/App.js index 4b47ac5a1f38..f7621f6c2a12 100644 --- a/frontend/web/components/App.js +++ b/frontend/web/components/App.js @@ -30,16 +30,9 @@ import Announcement from './Announcement' import { getBuildVersion } from 'common/services/useBuildVersion' import AccountProvider from 'common/providers/AccountProvider' import Nav from './navigation/Nav' -import { routes } from 'web/routes' +import { isAllowedWhileBlocked } from 'web/routePaths' import 'project/darkMode' -// The usage page explains the block, so it stays reachable. Read inside the -// function: web/routes imports this file, so routes is empty at module level. -const isAllowedWhileBlocked = (pathname) => - [routes.organisations, routes['organisation-usage']].some((path) => - matchPath(pathname, { exact: true, path, strict: false }), - ) - const App = class extends Component { static propTypes = { children: propTypes.element.isRequired, diff --git a/frontend/web/components/pages/OrganisationsPage.tsx b/frontend/web/components/pages/OrganisationsPage.tsx index a4fbf1b282f4..c8e3bc08c743 100644 --- a/frontend/web/components/pages/OrganisationsPage.tsx +++ b/frontend/web/components/pages/OrganisationsPage.tsx @@ -10,7 +10,7 @@ import PanelSearch from 'components/PanelSearch' import AppActions from 'common/dispatcher/app-actions' import { useHistory } from 'react-router-dom' import ConfigProvider from 'common/providers/ConfigProvider' -import { routes } from 'web/routes' +import { routes } from 'web/routePaths' const OrganisationsPage: FC = () => { const history = useHistory() diff --git a/frontend/web/routePaths.ts b/frontend/web/routePaths.ts new file mode 100644 index 000000000000..85d92f82445f --- /dev/null +++ b/frontend/web/routePaths.ts @@ -0,0 +1,98 @@ +import { matchPath } from 'react-router-dom' + +// The route table on its own, so anything needing a path does not have to +// import the component tree. web/routes imports App, so reading it from a +// component is a cycle. +export const routes = { + 'account': '/account', + 'account-settings': '/project/:projectId/environment/:environmentId/account', + 'admin-dashboard': '/admin/dashboard', + 'audit-log': '/project/:projectId/audit-log', + 'audit-log-item': '/project/:projectId/audit-log/:id', + 'broken': '/broken', + 'change-request': + '/project/:projectId/environment/:environmentId/change-requests/:id', + 'change-request-project': '/project/:projectId/change-requests/:id', + 'change-requests': + '/project/:projectId/environment/:environmentId/change-requests', + 'change-requests-project': '/project/:projectId/change-requests', + 'compare': '/project/:projectId/compare', + 'create-environment': '/project/:projectId/environment/create', + 'create-organisation': '/create', + 'create-release-pipeline': '/project/:projectId/release-pipelines/create', + 'dev-view': '/organisation/:organisationId/dev-view', + 'environment-settings': + '/project/:projectId/environment/:environmentId/settings', + 'executive-view': '/organisation/:organisationId/executive-view', + 'experiment-detail': + '/project/:projectId/environment/:environmentId/experiments/:experimentId', + 'experiments': '/project/:projectId/environment/:environmentId/experiments', + 'feature-history': '/project/:projectId/environment/:environmentId/history', + 'feature-history-detail': + '/project/:projectId/environment/:environmentId/history/:id/', + 'features': '/project/:projectId/environment/:environmentId/features', + 'flag-environments': '/project/:projectId/flag/:flagId/environments', + 'gettingStarted': '/getting-started', + 'github-setup': '/github-setup', + 'home': '/home', + 'identities': '/project/:projectId/environment/:environmentId/identities', + 'identity': + '/project/:projectId/environment/:environmentId/identities/:identity/:id', + 'identity-id': + '/project/:projectId/environment/:environmentId/identities/:identity', + 'integrations': '/project/:projectId/integrations', + 'invite': '/invite/:id', + 'invite-link': '/invite-link/:id', + 'legacy-identities': '/project/:projectId/environment/:environmentId/users', + 'legacy-identity': + '/project/:projectId/environment/:environmentId/users/:identity/:id', + 'legacy-identity-id': + '/project/:projectId/environment/:environmentId/users/:identity', + 'lifecycle': '/project/:projectId/lifecycle/:section?', + 'login': '/login', + 'maintenance': '/maintenance', + 'metrics': '/project/:projectId/environment/:environmentId/metrics', + 'not-found': '/404', + 'oauth': '/oauth/:type', + 'oauth-authorize': '/oauth/authorize', + 'organisation-integrations': '/organisation/:organisationId/integrations', + 'organisation-permissions': '/organisation/:organisationId/permissions', + 'organisation-projects': '/organisation/:organisationId/projects', + 'organisation-settings': '/organisation/:organisationId/settings', + 'organisation-settings-redirect': '/organisation-settings', + 'organisation-usage': '/organisation/:organisationId/usage', + 'organisations': '/organisations', + 'password-reset': '/password-reset/confirm/:uid/:token/', + 'permissions': '/project/:projectId/permissions', + 'project-redirect': '/project/:projectId', + 'project-settings': '/project/:projectId/settings', + 'project-settings-in-environment': + '/project/:projectId/environment/:environmentId/project-settings', + 'release-manager': '/organisation/:organisationId/release-manager', + 'release-pipelines': '/project/:projectId/release-pipelines', + 'release-pipelines-detail': '/project/:projectId/release-pipelines/:id', + 'release-pipelines-detail-edit': + '/project/:projectId/release-pipelines/:id/edit', + 'root': '/', + 'saml': '/saml', + 'scheduled-change': + '/project/:projectId/environment/:environmentId/scheduled-changes/:id', + 'scheduled-changes': + '/project/:projectId/environment/:environmentId/scheduled-changes', + 'sdk-keys': '/project/:projectId/environment/:environmentId/sdk-keys', + 'segment': '/project/:projectId/segments/:id', + 'segments': '/project/:projectId/segments', + 'signup': '/signup', +} + +// A blocked organisation keeps the organisations list, to switch away, and the +// usage page, which explains the block. +export const ALLOWED_WHILE_BLOCKED = [ + routes.organisations, + routes['organisation-usage'], +] + +export const isAllowedWhileBlocked = (pathname: string): boolean => + ALLOWED_WHILE_BLOCKED.some((path) => + matchPath(pathname, { exact: true, path, strict: false }), + ) diff --git a/frontend/web/routes.js b/frontend/web/routes.js index 7486352dd8c5..111e9f64d095 100644 --- a/frontend/web/routes.js +++ b/frontend/web/routes.js @@ -56,89 +56,10 @@ import DevViewPage from './components/pages/DevViewPage' import AdminDashboardPage from './components/pages/admin-dashboard/AdminDashboardPage' import CleanupPage from './components/pages/feature-lifecycle' import OAuthAuthorizePage from './components/pages/OAuthAuthorizePage' +import { routes } from './routePaths' import { Provider } from 'react-redux' import { getStore } from 'common/store' -export const routes = { - 'account': '/account', - 'account-settings': '/project/:projectId/environment/:environmentId/account', - 'admin-dashboard': '/admin/dashboard', - 'audit-log': '/project/:projectId/audit-log', - 'audit-log-item': '/project/:projectId/audit-log/:id', - 'broken': '/broken', - 'change-request': - '/project/:projectId/environment/:environmentId/change-requests/:id', - 'change-request-project': '/project/:projectId/change-requests/:id', - 'change-requests': - '/project/:projectId/environment/:environmentId/change-requests', - 'change-requests-project': '/project/:projectId/change-requests', - 'compare': '/project/:projectId/compare', - 'create-environment': '/project/:projectId/environment/create', - 'create-organisation': '/create', - 'create-release-pipeline': '/project/:projectId/release-pipelines/create', - 'dev-view': '/organisation/:organisationId/dev-view', - 'environment-settings': - '/project/:projectId/environment/:environmentId/settings', - 'executive-view': '/organisation/:organisationId/executive-view', - 'experiment-detail': - '/project/:projectId/environment/:environmentId/experiments/:experimentId', - 'experiments': '/project/:projectId/environment/:environmentId/experiments', - 'feature-history': '/project/:projectId/environment/:environmentId/history', - 'feature-history-detail': - '/project/:projectId/environment/:environmentId/history/:id/', - 'features': '/project/:projectId/environment/:environmentId/features', - 'flag-environments': '/project/:projectId/flag/:flagId/environments', - 'gettingStarted': '/getting-started', - 'github-setup': '/github-setup', - 'home': '/home', - 'identities': '/project/:projectId/environment/:environmentId/identities', - 'identity': - '/project/:projectId/environment/:environmentId/identities/:identity/:id', - 'identity-id': - '/project/:projectId/environment/:environmentId/identities/:identity', - 'integrations': '/project/:projectId/integrations', - 'invite': '/invite/:id', - 'invite-link': '/invite-link/:id', - 'legacy-identities': '/project/:projectId/environment/:environmentId/users', - 'legacy-identity': - '/project/:projectId/environment/:environmentId/users/:identity/:id', - 'legacy-identity-id': - '/project/:projectId/environment/:environmentId/users/:identity', - 'lifecycle': '/project/:projectId/lifecycle/:section?', - 'login': '/login', - 'maintenance': '/maintenance', - 'metrics': '/project/:projectId/environment/:environmentId/metrics', - 'not-found': '/404', - 'oauth': '/oauth/:type', - 'oauth-authorize': '/oauth/authorize', - 'organisation-integrations': '/organisation/:organisationId/integrations', - 'organisation-permissions': '/organisation/:organisationId/permissions', - 'organisation-projects': '/organisation/:organisationId/projects', - 'organisation-settings': '/organisation/:organisationId/settings', - 'organisation-settings-redirect': '/organisation-settings', - 'organisation-usage': '/organisation/:organisationId/usage', - 'organisations': '/organisations', - 'password-reset': '/password-reset/confirm/:uid/:token/', - 'permissions': '/project/:projectId/permissions', - 'project-redirect': '/project/:projectId', - 'project-settings': '/project/:projectId/settings', - 'project-settings-in-environment': - '/project/:projectId/environment/:environmentId/project-settings', - 'release-manager': '/organisation/:organisationId/release-manager', - 'release-pipelines': '/project/:projectId/release-pipelines', - 'release-pipelines-detail': '/project/:projectId/release-pipelines/:id', - 'release-pipelines-detail-edit': - '/project/:projectId/release-pipelines/:id/edit', - 'root': '/', - 'saml': '/saml', - 'scheduled-change': - '/project/:projectId/environment/:environmentId/scheduled-changes/:id', - 'scheduled-changes': - '/project/:projectId/environment/:environmentId/scheduled-changes', - 'sdk-keys': '/project/:projectId/environment/:environmentId/sdk-keys', - 'segment': '/project/:projectId/segments/:id', - 'segments': '/project/:projectId/segments', - 'signup': '/signup', -} +export { routes } from './routePaths' export default (