Skip to content

Convert the Application route tree from react-router v5 Switch to v6 Routes - #400

Merged
cigamit merged 4 commits into
ctrliq:mainfrom
blaipr:feature/react-router-route-tree
Jun 14, 2026
Merged

Convert the Application route tree from react-router v5 Switch to v6 Routes#400
cigamit merged 4 commits into
ctrliq:mainfrom
blaipr:feature/react-router-route-tree

Conversation

@blaipr

@blaipr blaipr commented Jun 13, 2026

Copy link
Copy Markdown
Contributor

SUMMARY

First of the route-tree conversions, now that the v6-compat bridge has merged. Converts the Application screen's two route-tree files from v5 <Switch> to v6 <Routes>, establishing the pattern for the remaining route trees.

Applications.js (list / add / detail):

  • <Switch><Routes>, <Route path><Child/></Route><Route path element={<Child/>} />.
  • Uses absolute paths: this screen still mounts under App.js's v5 ProtectedRoute, so there is no v6 parent route to resolve relative paths against. The /applications/:id route gains a /* so the nested Application route tree can match the rest of the path.

Application/Application.js (detail tabs):

  • <Switch><Routes>, <Redirect from=":id" to=":id/details" exact /> → an index route rendering <Navigate replace />.
  • Uses paths relative to the /applications/:id/* parent established above (details, edit, tokens).

Also clears the screen's remaining v5 useRouteMatch (the remaining "prefix-match" usage): ApplicationsList built links from match.url and ApplicationForm checked match.url.endsWith('edit'). The list always renders at /applications so it builds from that literal; the form reads useLocation().pathname. The Application screen now uses no v5 router APIs.

Why this is safe and isolated

The compat package still exports v5 Switch for the remaining unconverted route trees, so v5 and v6 trees coexist under the single mounted compat router. Nothing outside the Application screen changes. The final react-router-dom → v6 flip and removal of react-router-dom-v5-compat stay for the end of the migration, once every <Switch> is gone.

Verification

Both suites are rewritten from Enzyme to RTL and now assert real route resolution through the compat router (renderWithContexts mounts it, the same way the app does):

  • /applications renders the list, /applications/add the add form, /applications/:id/... the nested detail subtree.
  • The nested tabs resolve under a /applications/:id/* parent route (mounted in the test exactly as Applications.js wires it in the app): details, edit, tokens, and the bare /applications/:id index path redirecting to details.
  • A 404 on the detail fetch surfaces the not-found error instead of a tab panel.

One assertion changed meaning for the better: route params are real strings now ('1'), where the old test mocked useParams to return the number 1.

ISSUE TYPE

  • New or Enhanced Feature

COMPONENT NAME

  • UI

ASCENDER VERSION

awx: 25.4.1.dev21+g761ea17.d20260613

ADDITIONAL INFORMATION

npm --prefix awx/ui run lint     # clean
npm --prefix awx/ui run test     # full suite green

blaipr added 2 commits June 13, 2026 14:58
First route-tree conversion of step 2b, on top of the merged v6-compat
bridge. Applications.js and Application/Application.js move from v5
<Switch>/<Route>/<Redirect> to v6 <Routes>/<Route element>/<Navigate>:

- Applications.js uses absolute paths (it mounts under App.js's v5
  ProtectedRoute, so there is no v6 parent route to be relative to); the
  /applications/:id route gains /* so the nested Application route tree
  can match the rest.
- Application/Application.js uses paths relative to that :id/* parent,
  with an index route doing the details redirect that <Redirect exact>
  used to do.

The compat package still exports v5 Switch for the ~84 unconverted route
trees, so this is isolated and reversible; the react-router-dom -> v6
flip and compat removal stay for the end of step 2b.

Both suites are rewritten to RTL and now assert real route resolution
through the compat router (renderWithContexts mounts it): each URL
renders the expected branch, the index path redirects to details, and
the nested tree mounts under a /applications/:id/* parent exactly as the
app wires it. Route params are real strings now, not the numbers the old
mocked useParams faked.
ApplicationsList and ApplicationForm used match.url (the v5 useRouteMatch
prefix) to build links and detect the edit route. With the route tree on
v6 there is no useRouteMatch: ApplicationsList always renders at
/applications so it builds links from that literal, and ApplicationForm
checks useLocation().pathname for the edit suffix. Completes the
Application screen's move off v5 router APIs.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Migrates the Application screen route trees from react-router v5 <Switch> to v6 <Routes> using react-router-dom-v5-compat, removing remaining v5 useRouteMatch usage in this screen and updating the associated UI tests to assert real route resolution via React Testing Library.

Changes:

  • Converted Applications.js and Application/Application.js to v6-style <Routes> / element={...} routing (with /applications/:id/* to support nested matching).
  • Replaced v5 useRouteMatch usages in the Application list/form with useLocation/literal paths for link building and route checks.
  • Rewrote the Application screen tests from Enzyme to RTL to validate actual routing behavior (list/add/detail subtree, nested tabs, and 404 behavior).

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
awx/ui/src/screens/Application/shared/ApplicationForm.js Replaces useRouteMatch with useLocation().pathname to detect edit mode.
awx/ui/src/screens/Application/ApplicationsList/ApplicationsList.js Removes useRouteMatch; builds links from a fixed /applications base.
awx/ui/src/screens/Application/Applications.test.js Converts to RTL and asserts which v6 <Routes> branch resolves for given URLs.
awx/ui/src/screens/Application/Applications.js Converts top-level Application screen routing to v6 <Routes> and adds /* for nested detail routing.
awx/ui/src/screens/Application/Application/Application.test.js Converts to RTL; mounts under /applications/:id/* and asserts nested tab routing + 404 behavior.
awx/ui/src/screens/Application/Application/Application.js Converts nested detail tabs routing to v6 <Routes> with relative child paths and index redirect.

Comment thread awx/ui/src/screens/Application/Application/Application.js
The index route redirected to an absolute /applications/${id}/details;
use a relative "details" target so the nested router stays self-contained
and isn't coupled to the parent mount path (review feedback on ctrliq#400).
@blaipr

blaipr commented Jun 14, 2026

Copy link
Copy Markdown
Contributor Author

Addressed in 638145a — the index route now redirects with a relative to="details" instead of the absolute /applications/${id}/details, keeping the nested router self-contained as suggested. Lint is clean and the Application suite (47 tests) passes.

@cigamit

cigamit commented Jun 14, 2026

Copy link
Copy Markdown
Contributor

Having an issue with this one, after creating an application, clicking on tokens or clicking on a token then edit then cancel results in failures

django.request Not Found: /api/v2/applications/undefined/tokens/
django.request Not Found: /api/v2/applications/undefined/

After the Application route tree moved to v6 <Routes>, descendant
components that still read useParams from plain react-router-dom got an
empty params object (no v5 <Route> ancestor provides :id anymore), so
the application id resolved to undefined — producing requests to
/api/v2/applications/undefined/tokens/ and /api/v2/applications/undefined/
when opening Tokens or editing/cancelling.

- ApplicationTokenList.js and ApplicationEdit.js: read useParams (and
  useLocation) from react-router-dom-v5-compat so the id resolves from
  the v6 route match.
- Application.js: the tokens route becomes tokens/* so deeper token URLs
  still resolve to the list, matching the old non-exact v5 route.
- Tests: ApplicationEdit mocks useParams on v5-compat (keeping useNavigate
  real); ApplicationTokenList gains a regression test that mounts under
  /applications/:id/tokens/* and asserts readTokens is called with the
  route id rather than undefined.
@blaipr

blaipr commented Jun 14, 2026

Copy link
Copy Markdown
Contributor Author

Good catch, thanks. Fixed in 0c1e469.

The problem: once this screen's route tree moved to v6 Routes, a couple of the components underneath it were still importing useParams from plain react-router-dom (v5). There's no v5 Route ancestor left to provide :id, so v5 useParams() just returned {} and the application id came through as undefined. That's where the /api/v2/applications/undefined/tokens/ (token list) and /api/v2/applications/undefined/ (edit, then cancel sends you to /applications/undefined/details, which refetches) were coming from.

What I changed:

  • ApplicationTokenList.js and ApplicationEdit.js now read useParams (and useLocation) from react-router-dom-v5-compat, so the id resolves from the v6 match.
  • Application.js: the tokens route is now tokens/* so deeper token URLs like tokens/:tokenId/details still resolve to the list, same as the old non-exact v5 route did.

On the tests:

  • Added a regression test in ApplicationTokenList.test.js that mounts the list under the real /applications/:id/tokens/* route and checks readTokens gets called with the route id ('5') instead of undefined. It fails without the fix.
  • Updated ApplicationEdit.test.js to mock useParams on react-router-dom-v5-compat instead (left useNavigate real so the cancel/submit navigation checks still run).

Whole screens/Application suite passes (10 suites, 48 tests) and lint is clean. I also walked through both flows you hit (create then Tokens, and token then edit then cancel) and the undefined URLs are gone.

@cigamit
cigamit merged commit 3db8a5b into ctrliq:main Jun 14, 2026
@cigamit cigamit self-assigned this Jun 18, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants