fix: add missing /groups serve.json rewrite causing direct-nav 404 - #385
Merged
Conversation
serve build -l 3001 runs deliberately without -s (see b8a3808), so every client-side route needs an explicit rewrite entry in frontend/public/serve.json. PR #326 added the /groups route and nav link but never added the matching rewrite, so a page refresh, bookmark, or shared link to /groups hard-404s in production while client-side navigation works fine. - Add the missing /groups rewrite. - Add App.routes.test.js: a parity guard asserting every <Route> path in App.js has a matching serve.json rewrite and vice versa, with fail-loud assertions (route-tag count must match extracted-path count, minimum 8 routes) so an unparseable route shape fails instead of silently narrowing the guard. - Wire `npm test` into the Frontend Build CI job — it previously ran only `npm ci && npm run build`, so none of the 6 existing frontend test files (or this new one) ever executed in CI. Fixes #383
11 tasks
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.
Summary
/groupsrewrite tofrontend/public/serve.json.serveruns deliberately without-s(see b8a3808), so every client-side route needs an explicit rewrite entry; PR feat: experimental Groups $evaluate page (#322) #326 added the/groupsroute and nav link but never added the matching rewrite, so a refresh, bookmark, or shared link to/groupshard-404s in production while client-side nav works fine.frontend/src/App.routes.test.js: a parity guard asserting every<Route>path inApp.jshas a matchingserve.jsonrewrite and vice versa. Fails loud (route-tag count must equal extracted-path count, minimum 8 routes) so an unparseable route shape fails the test instead of silently narrowing the guard, rather than a general react-router/serve-handler equivalence claim.npm testinto theFrontend BuildCI job (previouslynpm ci && npm run buildonly — none of the 6 pre-existing frontend test files, or this new one, ever ran in CI). Confirmed all 7 suites (92 tests) pass locally before adding this step.Related issue
Closes #383
Type of change
Checklist
Test plan
cd frontend && npm ci && CI=true npm test -- --watchAll=false— 7 suites, 92 tests, all greenserve.jsonline, reran, confirmedApp.routes.test.jsfails with the exact missing-path diff, restored the fix, confirmed green againdocker compose up -d --build(full stack per.env.example), then:curl http://localhost:3001/groups→200 text/html(was404)/,/measures,/jobs,/groups,/groups/,/results,/results/:jobId,/validation,/settings) plus/groups?x=1→ all200 text/htmlhttp://localhost:3001/nope.pngstill returns a real404(not a silently-swallowed200app-shell fallback),manifest.jsonand the built JS bundle still serve correctly/groupsloads the app shell with no console errors; enabled thegroups_enabledadmin toggle in Settings, clicked "Groups" in the sidebar nav, then hard-refreshed on/groups— the exact user path that was 404ing before this fix — loads correctly, no console errorsFollow-up filed separately (not in this PR): #384 tracks collapsing
App.js's 5-way route-path duplication (<Route>JSX,ALL_NAV_ITEMS,PAGE_TITLE,SEARCH_PLACEHOLDER,serve.json) into one sharedROUTESconstant.