Skip to content

Convert CredentialType route tree to react-router v6 Routes - #407

Merged
cigamit merged 3 commits into
ctrliq:mainfrom
blaipr:feature/react-router-credential-type
Jun 15, 2026
Merged

Convert CredentialType route tree to react-router v6 Routes#407
cigamit merged 3 commits into
ctrliq:mainfrom
blaipr:feature/react-router-credential-type

Conversation

@blaipr

@blaipr blaipr commented Jun 14, 2026

Copy link
Copy Markdown
Contributor
SUMMARY

Continues the react-router v5 → v6 route-tree migration. Converts the CredentialType screen's route trees from the v5 <Switch>/<Route>/<Redirect> API to v6 <Routes>/<Route> via react-router-dom-v5-compat, following the exact pattern established for the Application route tree.

UI (no behavior change — same URLs, same panels):

  • CredentialTypes.js (list router): <Switch><Routes>; child routes use the element prop; the detail route is /credential_types/:id/* so the nested <CredentialType> tree can match the rest of the path.
  • CredentialType.js (detail router): <Switch><Routes> with relative child paths (edit, details); the exact <Redirect> is replaced by an index route that <Navigate replace>s to details.
  • Both test suites rewritten with renderWithContexts (RTL) to assert real v6 route resolution (which panel renders per URL, index redirect, 404 error) instead of enzyme structural checks.
ISSUE TYPE
  • Bug, Docs Fix or other nominal change
COMPONENT NAME
  • UI
ADDITIONAL INFORMATION
  • Part of the incremental react-router v6 migration; each screen's route tree is independent and converted on its own branch.
  • Tests: 8 route-resolution tests across the two converted suites; full screens/CredentialType directory passes (8 suites / 37 tests).
  • npm --prefix awx/ui run lint clean on the changed source.

blaipr added 2 commits June 14, 2026 05:08
Migrate the CredentialType list and detail route trees from the
react-router v5 <Switch>/<Route>/<Redirect> API to v6 <Routes>/<Route>
via react-router-dom-v5-compat, following the pattern established for
the Application route tree.

- CredentialTypes.js: <Switch> -> <Routes>, child routes use the
  element prop, and the detail route uses /credential_types/:id/* so
  the nested <CredentialType> tree matches the rest of the path.
- CredentialType.js: <Switch> -> <Routes> with relative child paths
  (edit, details), and the exact <Redirect> is replaced by an index
  route that <Navigate>s to details.
- Rewrite both test suites with renderWithContexts (RTL) to assert
  real v6 route resolution instead of enzyme structural checks.
With the route tree on v6 Routes, the list's v5 useRouteMatch() returns
match.url='/' (no v5 Route ancestor), so the add/detail links built from
${match.url} resolved to //add and //:id/details. Build them from the
literal /credential_types base instead, matching the ApplicationsList
pattern.

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 CredentialType screen route trees from react-router v5’s <Switch>/<Redirect> to v6-style <Routes>/<Route> via react-router-dom-v5-compat, and updates the associated tests to validate real v6 route resolution.

Changes:

  • Convert CredentialTypes (list) routing to <Routes> with element, and mount the detail subtree at /credential_types/:id/*.
  • Convert CredentialType (detail) routing to relative child paths with an index route redirecting to details.
  • Rewrite the CredentialTypes and CredentialType test suites to RTL route-resolution assertions instead of enzyme structure checks.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
awx/ui/src/screens/CredentialType/CredentialTypes.test.js Replaces enzyme structural tests with RTL route-resolution tests using mocked routed children.
awx/ui/src/screens/CredentialType/CredentialTypes.js Converts the list screen router to v6 <Routes> and mounts the nested detail tree with a /* splat.
awx/ui/src/screens/CredentialType/CredentialTypeList/CredentialTypeList.js Removes useRouteMatch usage and hardcodes add/detail link targets to match the new absolute route paths.
awx/ui/src/screens/CredentialType/CredentialType.test.js Rewrites detail routing tests to validate nested v6 route behavior (tabs, index redirect, 404 handling).
awx/ui/src/screens/CredentialType/CredentialType.js Converts the detail screen router to v6 <Routes> with relative child routes and an index redirect to details.

Comment thread awx/ui/src/screens/CredentialType/CredentialTypes.test.js
Comment thread awx/ui/src/screens/CredentialType/CredentialTypes.js
Comment thread awx/ui/src/screens/CredentialType/CredentialType.js
- Fix the doubled '/* /*' in the nested-route JSX comment.
- Drop the unused CredentialTypes API mock from the dispatcher test.
- Import Link from react-router-dom-v5-compat so the component uses a
  single router source.
@blaipr

blaipr commented Jun 14, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for the review. Addressed in a3e40cfe56: fixed the doubled /* /* in the nested-route comment, removed the unused CredentialTypes API mock from the dispatcher test, and moved Link onto react-router-dom-v5-compat so the component uses a single router source.

@cigamit
cigamit merged commit 808a0f8 into ctrliq:main Jun 15, 2026
blaipr added a commit to blaipr/ascender that referenced this pull request Jun 16, 2026
…t routes)

This PR converts App.js to v6 <Routes>, mounting each routeConfig screen as a
descendant at path="/<x>/*". The merged route-tree PRs (ctrliq#407-ctrliq#427) wrote each
list-entry screen's own <Routes> with ABSOLUTE child paths, which only matched
while App.js was still v5 (the screens were effectively top-level). As v6
descendants those absolute paths no longer match the relative remainder, so
most pages rendered blank / not-found after rebasing onto current main
(reported by cigamit).

Convert the list-entry screens to RELATIVE paths (add, :id/*, <Route index>):
Credentials, Jobs, Projects, Inventories, Hosts, Organizations, Users, Teams,
CredentialTypes, WorkflowApprovals, NotificationTemplates, Applications,
ExecutionEnvironments, AllSchedules, InstanceGroups, Instances, Settings,
ManagementJobs (the last via a basePath template-literal path). Also convert
the Template / WorkflowJobTemplate detail <Routes> from absolute
/templates/:templateType/:id/... to relative so their tabs resolve under the
v6 root.

Update each screen's tests to mount the screen as a descendant under a real
<Routes><Route path="/<x>/*" element={<Screen/>}/></Routes>, matching
production (the old top-level mounts only resolved with absolute paths).

Browser-smoke-tested every top-level route plus representative detail tabs
against the dev server: all render with no /api/.../undefined/ calls.
host_metrics and subscription_usage are license-gated out of routeConfig and
are unchanged.
cigamit pushed a commit that referenced this pull request Jun 17, 2026
* Convert the App.js root route tree to react-router v6 Routes

Migrate the application root from the react-router v5
<Switch>/<Route>/<Redirect> API to v6 <Routes>/<Route>/<Navigate> via
react-router-dom-v5-compat. The HashRouter + CompatRouter bridge stays
in place (the package is not flipped to v6-proper yet, since a few
screens still use <Switch>).

- App root <Switch> -> <Routes>: /login, / -> Navigate /home, and a
  catch-all path="*" that renders the ProtectedRoute > ConfigProvider >
  app container. The legacy /*/ trailing-slash redirect is dropped (v6
  matches trailing slashes leniently).
- ProtectedRoute no longer renders a <Route>; it is a v6 element guard
  that returns its children (when authenticated) or <Navigate to="/login">
  (the loginRedirectOverride locationReplace path is unchanged).
- AuthorizedRoutes: <Switch> -> <Routes>; each routeConfig screen mounts
  at path/* (so the screen's own nested <Routes> resolve); metrics and
  the not-found fallback likewise; the unauthorized branch redirects to
  /subscription_management via <Navigate>. The unused match prop passed
  to screens is dropped (no screen reads it).
- App uses useNavigate instead of useHistory.

Screens that are still on v5 <Switch> keep working under the v6 root
because their <Switch> matches the absolute location; already-migrated
screens use absolute v6 paths. Tests: App.test.js (incl. a new
authenticated-children case) and index.test.js pass; lint clean.

* Address review comment: fix always-true redirect condition

`redirectURL !== '/' || redirectURL !== '/home'` is always true, so the
post-login redirect ran even for '/' and '/home'. Use && so it only
navigates when the stored URL is neither of those defaults.

* Make every screen render under the v6 App.js root (relative descendant routes)

This PR converts App.js to v6 <Routes>, mounting each routeConfig screen as a
descendant at path="/<x>/*". The merged route-tree PRs (#407-#427) wrote each
list-entry screen's own <Routes> with ABSOLUTE child paths, which only matched
while App.js was still v5 (the screens were effectively top-level). As v6
descendants those absolute paths no longer match the relative remainder, so
most pages rendered blank / not-found after rebasing onto current main
(reported by cigamit).

Convert the list-entry screens to RELATIVE paths (add, :id/*, <Route index>):
Credentials, Jobs, Projects, Inventories, Hosts, Organizations, Users, Teams,
CredentialTypes, WorkflowApprovals, NotificationTemplates, Applications,
ExecutionEnvironments, AllSchedules, InstanceGroups, Instances, Settings,
ManagementJobs (the last via a basePath template-literal path). Also convert
the Template / WorkflowJobTemplate detail <Routes> from absolute
/templates/:templateType/:id/... to relative so their tabs resolve under the
v6 root.

Update each screen's tests to mount the screen as a descendant under a real
<Routes><Route path="/<x>/*" element={<Screen/>}/></Routes>, matching
production (the old top-level mounts only resolved with absolute paths).

Browser-smoke-tested every top-level route plus representative detail tabs
against the dev server: all render with no /api/.../undefined/ calls.
host_metrics and subscription_usage are license-gated out of routeConfig and
are unchanged.
@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