Skip to content

Convert NotificationTemplate route tree to react-router v6 Routes - #408

Merged
cigamit merged 4 commits into
ctrliq:mainfrom
blaipr:feature/react-router-notification-template
Jun 15, 2026
Merged

Convert NotificationTemplate route tree to react-router v6 Routes#408
cigamit merged 4 commits into
ctrliq:mainfrom
blaipr:feature/react-router-notification-template

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 NotificationTemplate screen's route trees from the v5 <Switch>/<Route>/<Redirect> API to v6 <Routes>/<Route> via react-router-dom-v5-compat, following the same pattern used for the Application and CredentialType route trees.

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

  • NotificationTemplates.js (list router): <Switch><Routes>; child routes use the element prop; the detail route is /notification_templates/:id/* so the nested <NotificationTemplate> tree can match the rest of the path; the unused useRouteMatch match.url is replaced by explicit paths.
  • NotificationTemplate.js (detail router): <Switch><Routes> with relative child paths (edit, details); the exact <Redirect> becomes an index route that <Navigate replace>s to details; the Details tab link is built from the route id instead of match.url.
  • Tests: the list test is converted to RTL and a new detail test is added, both using renderWithContexts to assert real v6 route resolution (which panel renders per URL, index redirect, 404 error).
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 off main.
  • Tests: 7 route-resolution tests across the two suites; full screens/NotificationTemplate directory passes (6 suites / 31 tests).
  • npm --prefix awx/ui run lint clean on the changed source.

blaipr added 2 commits June 14, 2026 05:12
Migrate the NotificationTemplate 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 same pattern used for the
Application and CredentialType route trees.

- NotificationTemplates.js: <Switch> -> <Routes>, child routes use the
  element prop, the detail route uses /notification_templates/:id/* so
  the nested <NotificationTemplate> tree matches the rest of the path,
  and the unused useRouteMatch match.url is dropped for explicit paths.
- NotificationTemplate.js: <Switch> -> <Routes> with relative child
  paths (edit, details); the exact <Redirect> becomes an index route
  that <Navigate>s to details; the Details tab link is built from the
  route id instead of match.url.
- Convert the list test and add a detail test, both with
  renderWithContexts (RTL) to assert real v6 route resolution.
Under the v6 route tree the list's v5 useRouteMatch() returns match.url='/',
so the add/detail links resolved to //add and //:id. Build them from the
literal /notification_templates base instead.

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 NotificationTemplate screen’s routing from react-router v5 (Switch/Redirect) to v6-style route resolution (Routes/Navigate) via react-router-dom-v5-compat, and updates tests to assert v6 route matching behavior.

Changes:

  • Converted the NotificationTemplates (list) and NotificationTemplate (detail) route trees to v6 <Routes> patterns, including nested :id/* matching and an index redirect to details.
  • Updated NotificationTemplate list URL construction to use explicit /notification_templates/... paths instead of match.url.
  • Converted/added RTL route-resolution tests for list, add, detail, edit, index redirect, and 404 behavior.

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/NotificationTemplate/NotificationTemplates.js Converts list-level routing to v6 <Routes> and uses explicit absolute paths.
awx/ui/src/screens/NotificationTemplate/NotificationTemplate.js Converts detail-level routing to nested v6 <Routes> with relative child paths and index redirect.
awx/ui/src/screens/NotificationTemplate/NotificationTemplateList/NotificationTemplateList.js Removes useRouteMatch usage and switches to explicit add/detail URL strings.
awx/ui/src/screens/NotificationTemplate/NotificationTemplates.test.js Converts list routing tests to RTL and validates which route branch resolves per URL.
awx/ui/src/screens/NotificationTemplate/NotificationTemplate.test.js Adds RTL tests for nested detail routing (details/edit/index redirect) and 404 handling.

Comment thread awx/ui/src/screens/NotificationTemplate/NotificationTemplates.js Outdated
Comment thread awx/ui/src/screens/NotificationTemplate/NotificationTemplates.test.js Outdated
Comment thread awx/ui/src/screens/NotificationTemplate/NotificationTemplate.js Outdated
blaipr added 2 commits June 14, 2026 22:24
- Fix the doubled '/* /*' in the nested-route JSX comment.
- Drop the unused NotificationTemplates API mock from the dispatcher test.
Landing on /notification_templates/:id fetched the template, then the
index redirect changed the pathname to /details and the pathname-keyed
effect fetched again. Skip the fetch on the bare /:id (it only redirects),
render the index <Navigate> unconditionally so the redirect still fires,
and give the details/edit routes a loading fallback so /:id/details always
matches while the template loads. Real navigation (edit -> details) still
re-fetches, so saved changes are reflected.
@blaipr

blaipr commented Jun 14, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for the review. Addressed in f417913506 and c20c3e6818: fixed the doubled /* /* comment and dropped the unused API mock; and reworked the load so the bare /:id no longer fetches before redirecting. The index <Navigate> now renders unconditionally and the detail/edit routes show a loading fallback, so the bare URL is skipped and only /details fetches. Real navigation (edit to details) still re-fetches, so saved changes are reflected.

@cigamit
cigamit merged commit 7710757 into ctrliq:main Jun 15, 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