Convert the shared Schedule components to react-router v6 Routes - #422
Convert the shared Schedule components to react-router v6 Routes#422blaipr wants to merge 3 commits into
Conversation
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.
There was a problem hiding this comment.
Pull request overview
Migrates the shared Schedule route subtree to the react-router v6 route API (via react-router-dom-v5-compat) so the schedule list/add/detail/edit screens can be mounted from multiple parent base paths while the broader v5 → v6 migration continues.
Changes:
- Converted
Schedules.jsfrom v5Switch/Route+useRouteMatchto v6Routes/Route, deriving a base schedule URL fromlocation.pathname. - Converted
Schedule.jsfrom v5Switch/Redirectto v6Routes/Navigate, keeping the existingpathRoot-based routing. - Updated the schedule detail mount route to delegate nested routes using
/*.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| awx/ui/src/components/Schedule/Schedules.js | Converts schedule list/add/detail routing to v6 Routes and computes a base URL from the current pathname. |
| awx/ui/src/components/Schedule/Schedule.js | Converts schedule detail routing to v6 Routes/Navigate while keeping pathRoot-derived absolute paths. |
- Schedule.js: import useParams/useLocation from react-router-dom-v5-compat (not v5) so scheduleId resolves under the v6 parent route. - Schedule.js: the edit route used :id while the component reads scheduleId; use :scheduleId consistently. - Schedules.js: derive baseUrl with substring and handle the 'schedules'- absent case explicitly so it never collapses to a relative path. - Drop the doubled '/* /*' nested-route comment in both files.
|
Thanks for the review. Addressed in |
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.
* Convert the shared Schedule components to react-router v6 Routes 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. * Convert Project route tree to react-router v6 Routes Migrate the Project list and detail route trees from the react-router v5 <Switch>/<Route>/<Redirect> API to v6 <Routes>/<Route> via react-router-dom-v5-compat. Now unblocked by the shared Schedule component conversion. - Projects.js: <Switch> -> <Routes>; the detail route uses /projects/:id/* so the nested <Project> tree matches. - Project.js: <Switch> -> <Routes> with relative child paths (details, edit, access, notifications, job_templates, schedules/*); the exact <Redirect> becomes an index route that <Navigate>s to details; the schedules tab delegates with schedules/* to the shared <Schedules>; not-found kept as path="*". - Tests: mount Project under the real /projects/:id/* route (drop the react-router-dom useRouteMatch/useParams mocks) and stub the list in Projects.test.js; mock RootAPI.readAssetVariables for the brand-name hook the resolved detail route now mounts. Depends on the shared Schedule component conversion (PR #422), included in this branch until that PR merges. * Address review comments on the shared Schedule components - Schedule.js: import useParams/useLocation from react-router-dom-v5-compat (not v5) so scheduleId resolves under the v6 parent route. - Schedule.js: the edit route used :id while the component reads scheduleId; use :scheduleId consistently. - Schedules.js: derive baseUrl with substring and handle the 'schedules'- absent case explicitly so it never collapses to a relative path. - Drop the doubled '/* /*' nested-route comment in both files. (cherry picked from commit 4b0b609) * Address review comments: fix doubled '/* /*' in Project route comments * Address review comment: cover the Project index redirect Add a test that mounting at the bare /projects/:id resolves to the details tab (the index <Route> -> <Navigate> replacing the old v5 Redirect). * Fix shared Schedule routing: use relative paths 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) * Fix the project Sync button under the v6 router cigamit reported the Sync button looked/behaved wrong. ProjectSyncButton used v5 useRouteMatch(), which resolves to url '/' under the compat router (no v5 route ancestor), so the isDetailsView check was always false. Derive it from useLocation().pathname instead. * Mount the ProjectDetail sync-button test at the details URL The Sync button only shows its 'Sync' label on the details view, which ProjectSyncButton now derives from useLocation; mount the test there.
|
About the 'can not list schedules of a Job Template / Page not found at /api/v2/schedules/templates/' regression. This PR converts only the shared Schedule component to v6 relative routes. The catch is that the relative routes only resolve when the Schedule component is mounted under a v6 consumer screen (Template, Project, ManagementJob, Inventory source). On this branch the consumer screens are still the v5 versions from main, so the relative routes resolve against the wrong base and you get /api/v2/schedules//. In other words the shared Schedule conversion and the consumer screen conversions have to land together. I have cherry-picked this exact v6 Schedule change into each consumer PR (#421, #423, #424, #427), so it is already carried there and verified working in a browser in that context. Suggestion: merge this one first and the consumer PRs on top (their cherry-picks become no-ops), or fold it into the consumers and close this. Either works, it just should not merge or be tested in isolation. Happy to do whichever you prefer. |
|
Closing this as redundant. The shared v6 Schedule conversion already landed on main through #424, which carried the same two-file change as a cherry-pick. Verified against the current main: this branch's only contribution is Schedule.js, Schedules.js and Schedules.test.js, and all three are now byte-identical to main (0 lines of difference). Merging this PR would apply nothing. The four consumer screens that delegate to Schedules each carry the same v6 Schedule change so they are self-contained and verified working in a browser: Inventory #421, ManagementJob #423, Project #424 (merged), Template #427. InventorySource #426 is superseded by #421. Nothing is lost by closing this. |
* Convert the shared Schedule components to react-router v6 Routes 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. * Convert ManagementJob route tree to react-router v6 Routes Migrate the ManagementJob list and detail route trees from the react-router v5 <Switch>/<Route>/<Redirect> API to v6 <Routes>/<Route> via react-router-dom-v5-compat. Now unblocked by the shared Schedule component conversion. - ManagementJobs.js: <Switch> -> <Routes>; the detail route uses /management_jobs/:id/* so the nested <ManagementJob> tree matches. - ManagementJob.js: <Switch> -> <Routes>; useRouteMatch replaced with the id route param + an explicit detailUrl; the exact <Redirect> becomes a <Route> that <Navigate>s to schedules (the default tab); the schedules tab delegates with schedules/* to the shared <Schedules>; notifications uses the element prop. - ManagementJobs.test.js: mount at /management_jobs with a stubbed list so the route resolves without an API call. Depends on the shared Schedule component conversion (PR #422), which is included in this branch until that PR merges. * Address review comments on the shared Schedule components - Schedule.js: import useParams/useLocation from react-router-dom-v5-compat (not v5) so scheduleId resolves under the v6 parent route. - Schedule.js: the edit route used :id while the component reads scheduleId; use :scheduleId consistently. - Schedules.js: derive baseUrl with substring and handle the 'schedules'- absent case explicitly so it never collapses to a relative path. - Drop the doubled '/* /*' nested-route comment in both files. (cherry picked from commit 4b0b609) * Fix shared Schedule routing: use relative paths 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) * Fix ManagementJob schedule routes to use relative paths ManagementJob is rendered as a descendant of the v6 ManagementJobs dispatcher (path management_jobs/:id/*), so its nested <Routes> resolves relative to management_jobs/:id. The child routes used absolute paths (management_jobs/:id/schedules/*), which do not match in a descendant route tree, so clicking a schedule rendered blank (the Schedule detail never mounted and never fetched the schedule). Use relative paths (index, notifications, schedules/*) so the nested Schedule route tree resolves.


SUMMARY
Continues the react-router v5 → v6 route-tree migration. Converts the shared Schedule components (
components/Schedule/Schedules.jsandSchedule.js) from the v5<Switch>/<Redirect>API to v6<Routes>/<Navigate>viareact-router-dom-v5-compat.These components are reused at several different base paths (templates, projects, inventory sources, management jobs), so they can't hardcode an absolute base.
Schedule.jsalready derived its base from the location (pathRoot= the pathname up toschedules); this applies the same technique toSchedules.js.Schedules.js: dropuseRouteMatch; compute the base fromuseLocationand use it for the list / add /:scheduleIdroutes (the schedule detail delegates with/*).Schedule.js:<Switch>→<Routes>; the exact<Redirect>becomes a<Route>that<Navigate>s to details; the edit/details routes keep their computedpathRootpaths; not-found stayspath="*".Because the routes are computed-absolute, they resolve whether the parent screen is still on v5
<Switch>or already migrated to v6. This decouples the shared-component conversion from the screens that mount<Schedules>(Template, Project, ManagementJob, InventorySource, WorkflowJobTemplate), which can now be migrated independently afterward.ISSUE TYPE
COMPONENT NAME
ADDITIONAL INFORMATION
components/Schedulepasses (13 suites / 126 tests), and all five parent screens that mount<Schedules>pass unchanged.npm --prefix awx/ui run lintclean on the changed source.