Skip to content

Convert Template and WorkflowJobTemplate route trees to react-router v6 - #427

Merged
cigamit merged 5 commits into
ctrliq:mainfrom
blaipr:feature/react-router-template
Jun 16, 2026
Merged

Convert Template and WorkflowJobTemplate route trees to react-router v6#427
cigamit merged 5 commits into
ctrliq:mainfrom
blaipr:feature/react-router-template

Conversation

@blaipr

@blaipr blaipr commented Jun 14, 2026

Copy link
Copy Markdown
Contributor
SUMMARY

Converts the Template, WorkflowJobTemplate and TemplateSurvey route trees from react-router v5 to v6, finishing the react-router v6 migration for the Templates screen (react-router-dom-v5-compat bridge). These are the last remaining <Switch> screens.

  • <Switch>/<Route>/<Redirect> -> <Routes>/<Route>/<Navigate> in Template.js, WorkflowJobTemplate.js and TemplateSurvey.js.
  • Template / WorkflowJobTemplate keep the :templateType/:id route patterns so the matched params reach descendants through the compat layer. The exact <Redirect> to the details tab becomes an index <Route> + <Navigate replace>.
  • The schedules and survey tabs delegate to the shared v6 <Schedules> and <TemplateSurvey> components, mounted with a trailing /* so their nested route trees match.
  • TemplateSurvey derives its base path from the location and reads the template type/id from the template prop, so it no longer depends on the parent router. Its own <Switch> becomes <Routes>.
  • The Survey toolbar/add/edit helpers derive their base path from the location instead of useRouteMatch (which returns / under a v6 parent).
  • JobTemplateDetail reads its route params from react-router-dom-v5-compat so the id resolves under the converted parent.
  • The Template not-found route is no longer gated on the loading flag, avoiding a transient render where no route matches the current location.
  • Templates.js (the list/add dispatcher) stays on v5, matching the pattern used for the other dispatchers (Projects.js, InventorySources.js).
DEPENDENCY

Stacked on #422 (the v6 <Schedules>/<Schedule> conversion). The diff includes those two shared files; once #422 merges this will rebase cleanly.

ISSUE TYPE
  • Bug, Docs Fix or other nominal change
COMPONENT NAME
  • UI

blaipr added 2 commits June 14, 2026 17:47
The shared Schedules/Schedule components are mounted at several different
base paths (templates, projects, inventory sources, management jobs), so
they can't hardcode an absolute base. Schedule.js already derived its
base from the location (pathRoot = pathname up to 'schedules'); apply the
same technique to Schedules.js and convert both from v5
<Switch>/<Redirect> to v6 <Routes>/<Navigate>.

- Schedules.js: drop useRouteMatch; compute the base from useLocation and
  use it for the list/add/:scheduleId routes (the schedule detail
  delegates with /*).
- Schedule.js: <Switch> -> <Routes>; the exact <Redirect> becomes a
  <Route> Navigate to details; edit/details kept on their computed
  pathRoot paths; not-found kept as path="*".

Because the routes are computed-absolute, they resolve whether the parent
screen is still on v5 <Switch> or already migrated to v6 - so this
decouples the shared-component conversion from the Template / Project /
ManagementJob / InventorySource / WorkflowJobTemplate screens, which can
now be migrated independently afterward.

Verified: components/Schedule (13 suites) and all five parent screens
that mount <Schedules> pass unchanged.
Migrate the Template, WorkflowJobTemplate and TemplateSurvey screens from
react-router v5 <Switch>/<Route>/<Redirect> to v6
<Routes>/<Route>/<Navigate> via the react-router-dom-v5-compat bridge.

- Template/WorkflowJobTemplate keep the :templateType/:id route patterns so
  the matched params reach descendants through the compat layer; the exact
  <Redirect> to the details tab becomes an index <Route> + <Navigate replace>.
- The schedules and survey tabs delegate to the shared v6 <Schedules> and
  <TemplateSurvey> components with a trailing /* so their nested routes match.
- TemplateSurvey derives its base path from the location and reads the
  template type/id from the template prop, so it no longer depends on the
  parent router. Its own <Switch> becomes <Routes>.
- The Survey toolbar/add/edit helpers derive their base path from the
  location instead of useRouteMatch, which returns '/' under a v6 parent.
- JobTemplateDetail reads its route params from react-router-dom-v5-compat
  so the id resolves under the converted parent.
- The Template not-found route is no longer gated on the loading flag, which
  avoids a transient render where no route matches the current location.

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

Converts the remaining Templates-related route trees from react-router v5 (<Switch>/<Redirect>) to react-router v6 (<Routes>/<Navigate>) using react-router-dom-v5-compat, completing the v6 migration for the Templates detail screens while keeping params flowing to legacy descendants.

Changes:

  • Migrated Template and WorkflowJobTemplate detail screens to v6-style route trees and replaced the “details tab redirect” with an index-style Navigate.
  • Updated TemplateSurvey and Survey add/edit/toolbar helpers to derive their base paths from location rather than useRouteMatch.
  • Included the shared Schedule/Schedules v6 conversion and updated survey-related tests for the new param/id handling.

Reviewed changes

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

Show a summary per file
File Description
awx/ui/src/screens/Template/WorkflowJobTemplate.js Migrates Workflow Job Template routes to v6 <Routes> with Navigate redirects and /* delegation for nested trees.
awx/ui/src/screens/Template/Template.js Migrates Job Template routes to v6 <Routes>, adds an explicit Navigate for the base details redirect, and keeps not-found available during loading.
awx/ui/src/screens/Template/TemplateSurvey.js Converts survey subtree to v6 <Routes> and derives base survey path from location + template prop.
awx/ui/src/screens/Template/Survey/SurveyToolbar.js Replaces useRouteMatch() with location-derived base path for toolbar “Add” navigation.
awx/ui/src/screens/Template/Survey/SurveyQuestionEdit.js Updates edit navigation/redirect to use location-derived survey base path.
awx/ui/src/screens/Template/Survey/SurveyQuestionAdd.js Updates add navigation to compute base survey URL from location instead of useRouteMatch().
awx/ui/src/screens/Template/JobTemplateDetail/JobTemplateDetail.js Ensures route params are read via react-router-dom-v5-compat to resolve under v6-converted parents.
awx/ui/src/screens/Template/TemplateSurvey.test.js Adjusts expectations to match numeric ids and the new “id from template prop” behavior.
awx/ui/src/components/Schedule/Schedules.js Converts shared Schedules subtree to v6 <Routes> and derives base from location for reuse under different parents.
awx/ui/src/components/Schedule/Schedule.js Converts Schedule detail subtree to v6 <Routes> and replaces the v5 redirect with Navigate.

Comment thread awx/ui/src/screens/Template/TemplateSurvey.test.js
…e template

The job-template delete test mounted at .../job_template/15/survey but
used mockJobTemplateData (id 7) and asserted destroySurvey(7). Use 7 in
the URL too so the route id and template.id match and the intent is clear.
@blaipr

blaipr commented Jun 14, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for the review. Addressed in 59c0f23593: aligned the survey-delete test URL id with the template data id (mounted at .../job_template/7/survey to match mockJobTemplateData.id) so the route id and template.id agree and the intent is clear.

cigamit reported that opening a schedule's details fails. <Schedules> is
mounted by the parent screens via a ".../schedules/*" route, so it is a v6
descendant; it (and <Schedule>) built absolute baseUrl/pathRoot-derived
route paths, which do not match in that descendant context, so the
add/detail/edit routes never resolved.

Use relative route paths (add, :scheduleId/*, index in Schedules; index ->
details, details, edit in Schedule). The pathRoot-derived absolute values
are kept only for the tab/breadcrumb Links, not the route paths. Mount the
Schedules test under its parent route.

(cherry picked from commit 4dd8df0)
@cigamit

cigamit commented Jun 16, 2026

Copy link
Copy Markdown
Contributor

Everything is working except for surveys, the page is blank for it, no errors in the logs.
image

cigamit reported the survey tab rendering blank with no errors. TemplateSurvey
is a v6 descendant (Template mounts it at .../:id/survey/*), but its <Routes>
used absolute surveyUrl-based paths, which do not match in a descendant tree
(the CLAUDE.md v6 trap), so nothing rendered.

Use relative child paths (add/edit) and an index route for the SurveyList.
Rewrite the test to mount TemplateSurvey under a real v6 .../survey/* route
(so the index resolves) instead of a v5 Route with mocked params, which also
resolves the Copilot note about the route id vs template id mismatch.
@blaipr

blaipr commented Jun 16, 2026

Copy link
Copy Markdown
Contributor Author

Fixed the blank survey tab (cbc625f). Root cause: TemplateSurvey is a v6 descendant (Template mounts it at .../:id/survey/*), but its <Routes> used absolute surveyUrl paths, which don't match in a descendant tree - so nothing rendered. Switched to relative child paths (add/edit) with an index route for the survey list.

Browser-verified on a live instance (puppeteer): the survey list renders the questions, and /survey/add renders the add form - both previously blank.

Also addressed the Copilot note: the test now mounts TemplateSurvey under a real v6 survey/* route (instead of a v5 <Route> with mocked params), so it actually exercises the descendant routing - the old mocked test passed with the bug. Route ids now match the template ids, asserted with concrete literals (7/15).

@cigamit
cigamit merged commit 0f5f6c5 into ctrliq:main Jun 16, 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