diff --git a/awx/ui/src/screens/Job/Job.js b/awx/ui/src/screens/Job/Job.js index 23ee2a1f7..af92672a4 100644 --- a/awx/ui/src/screens/Job/Job.js +++ b/awx/ui/src/screens/Job/Job.js @@ -1,12 +1,11 @@ import React, { useEffect, useCallback, useRef } from 'react'; +import { Link } from 'react-router-dom'; import { + Routes, Route, - Switch, - Redirect, - Link, + Navigate, useParams, - useRouteMatch, -} from 'react-router-dom'; +} from 'react-router-dom-v5-compat'; import { useLingui } from '@lingui/react/macro'; import { CaretLeftIcon } from '@patternfly/react-icons'; import { Card, PageSection } from '@patternfly/react-core'; @@ -37,7 +36,6 @@ export const JOB_URL_SEGMENT_MAP = { function Job({ setBreadcrumb }) { const { t } = useLingui(); const { id, typeSegment } = useParams(); - const match = useRouteMatch(); const type = JOB_URL_SEGMENT_MAP[typeSegment]; @@ -126,8 +124,12 @@ function Job({ setBreadcrumb }) { persistentFilterKey: 'jobs', id: 99, }, - { name: t`Details`, link: `${match.url}/details`, id: 0 }, - { name: t`Output`, link: `${match.url}/output`, id: 1 }, + { + name: t`Details`, + link: `/jobs/${typeSegment}/${id}/details`, + id: 0, + }, + { name: t`Output`, link: `/jobs/${typeSegment}/${id}/output`, id: 1 }, ]; if (relatedJobs?.length > 0) { tabsArray.push({ @@ -172,48 +174,49 @@ function Job({ setBreadcrumb }) {
- - - - {job && [ + + } /> + {job && ( - - , - - {job.type === 'workflow_job' ? ( - - ) : ( - - )} - , - + } + /> + )} + {job && ( + + ) : ( + + ) + } + /> + )} + {t`View Job Details`} - , - ]} - + } + /> +
diff --git a/awx/ui/src/screens/Job/Job.test.js b/awx/ui/src/screens/Job/Job.test.js index d90839f0b..b75ee9bbf 100644 --- a/awx/ui/src/screens/Job/Job.test.js +++ b/awx/ui/src/screens/Job/Job.test.js @@ -5,8 +5,10 @@ import { mountWithContexts } from '../../../testUtils/enzymeHelpers'; import Job from './Job'; jest.mock('../../api'); -jest.mock('react-router-dom', () => ({ - ...jest.requireActual('react-router-dom'), +// Job reads useParams from react-router-dom-v5-compat (the route tree is v6); +// mock it there, keeping the rest of the module real. +jest.mock('react-router-dom-v5-compat', () => ({ + ...jest.requireActual('react-router-dom-v5-compat'), useParams: () => ({ id: 1, typeSegment: 'project', diff --git a/awx/ui/src/screens/Job/JobTypeRedirect.js b/awx/ui/src/screens/Job/JobTypeRedirect.js index 536e8c0c0..84dbe02a0 100644 --- a/awx/ui/src/screens/Job/JobTypeRedirect.js +++ b/awx/ui/src/screens/Job/JobTypeRedirect.js @@ -1,5 +1,6 @@ import React, { useCallback, useEffect } from 'react'; -import { Redirect, Link } from 'react-router-dom'; +import { Link } from 'react-router-dom'; +import { Navigate } from 'react-router-dom-v5-compat'; import { PageSection, Card } from '@patternfly/react-core'; import { useLingui } from '@lingui/react/macro'; @@ -12,7 +13,7 @@ import { JOB_TYPE_URL_SEGMENTS } from '../../constants'; const NOT_FOUND = 'not found'; -function JobTypeRedirect({ id, path, view }) { +function JobTypeRedirect({ id, view }) { const { t } = useLingui(); const { isLoading, @@ -58,7 +59,9 @@ function JobTypeRedirect({ id, path, view }) { ); } const typeSegment = JOB_TYPE_URL_SEGMENTS[job.type]; - return ; + return ( + + ); } JobTypeRedirect.defaultProps = { diff --git a/awx/ui/src/screens/Job/Jobs.js b/awx/ui/src/screens/Job/Jobs.js index 079391573..6bcc0b485 100644 --- a/awx/ui/src/screens/Job/Jobs.js +++ b/awx/ui/src/screens/Job/Jobs.js @@ -1,5 +1,10 @@ import React, { useState, useCallback } from 'react'; -import { Route, Switch, useParams, useRouteMatch } from 'react-router-dom'; +import { + Routes, + Route, + Navigate, + useParams, +} from 'react-router-dom-v5-compat'; import { useLingui } from '@lingui/react/macro'; import { PageSection } from '@patternfly/react-core'; @@ -13,13 +18,23 @@ import { JOB_TYPE_URL_SEGMENTS } from '../../constants'; function TypeRedirect({ view }) { const { id } = useParams(); - const { path } = useRouteMatch(); - return ; + return ; +} + +// Legacy /jobs/system/:id URLs map to the canonical /jobs/management/:id; +// preserve any trailing sub-path (the splat) on the redirect. +function SystemRedirect() { + const { id, '*': rest } = useParams(); + return ( + + ); } function Jobs() { const { t } = useLingui(); - const match = useRouteMatch(); const [breadcrumbConfig, setBreadcrumbConfig] = useState({ '/jobs': t`Jobs`, }); @@ -44,27 +59,33 @@ function Jobs() { return ( <> - - - - - - - - - - - - - - - - - - - - - + + + + + + + } + /> + } /> + } + /> + } + /> + {/* /* so the nested route tree can match details/output */} + } + /> + } /> + ); } diff --git a/awx/ui/src/screens/Job/Jobs.test.js b/awx/ui/src/screens/Job/Jobs.test.js index 3061e37b8..3c42d4c92 100644 --- a/awx/ui/src/screens/Job/Jobs.test.js +++ b/awx/ui/src/screens/Job/Jobs.test.js @@ -1,35 +1,87 @@ import React from 'react'; -import { act } from 'react-dom/test-utils'; -import { mountWithContexts } from '../../../testUtils/enzymeHelpers'; +import { screen, waitFor } from '@testing-library/react'; +import { createMemoryHistory } from 'history'; +import { renderWithContexts } from '../../../testUtils/rtlContexts'; import Jobs from './Jobs'; jest.mock('../../api'); -jest.mock('react-router-dom', () => ({ - ...jest.requireActual('react-router-dom'), - useRouteMatch: () => ({ - path: '/', - }), -})); +// Replace the routed children with markers so the assertions are purely about +// which branch of the v6 tree resolves for a given URL. +jest.mock('components/JobList', () => { + const ReactLib = require('react'); + return { + __esModule: true, + default: () => ReactLib.createElement('div', null, 'JobList'), + }; +}); +jest.mock('./Job', () => { + const ReactLib = require('react'); + return { + __esModule: true, + default: () => ReactLib.createElement('div', null, 'Job detail'), + }; +}); +jest.mock('./JobTypeRedirect', () => { + const ReactLib = require('react'); + return { + __esModule: true, + // Mirror the real component's default of view='output' so the bare + // /jobs/:id route resolves to the output view in the test too. + default: ({ view = 'output' }) => + ReactLib.createElement('div', null, `JobTypeRedirect:${view}`), + }; +}); + +function renderAt(path) { + const history = createMemoryHistory({ initialEntries: [path] }); + return renderWithContexts(, { + context: { router: { history } }, + }); +} describe('', () => { - test('initially renders successfully', async () => { - let wrapper; - await act(async () => { - wrapper = mountWithContexts(); - }); - expect(wrapper.find('JobList')).toHaveLength(1); + test('renders the list at /jobs', async () => { + renderAt('/jobs'); + expect(await screen.findByText('JobList')).toBeInTheDocument(); + }); + + test('renders the typed detail subtree at /jobs/:typeSegment/:id', async () => { + renderAt('/jobs/playbook/5/output'); + expect(await screen.findByText('Job detail')).toBeInTheDocument(); + expect(screen.queryByText('JobList')).not.toBeInTheDocument(); + }); + + test('routes an untyped /jobs/:id to the type redirect defaulting to output', async () => { + renderAt('/jobs/5'); + // the bare route renders with no explicit view, which + // defaults to 'output' + expect( + await screen.findByText('JobTypeRedirect:output') + ).toBeInTheDocument(); + expect(screen.queryByText('JobList')).not.toBeInTheDocument(); + }); + + test('routes an untyped /jobs/:id/details to the details type redirect', async () => { + renderAt('/jobs/5/details'); + expect( + await screen.findByText('JobTypeRedirect:details') + ).toBeInTheDocument(); + }); + + test('redirects legacy /jobs/system/:id to /jobs/management/:id', async () => { + const { history } = renderAt('/jobs/system/5'); + await waitFor(() => + expect(history.location.pathname).toBe('/jobs/management/5') + ); + expect(await screen.findByText('Job detail')).toBeInTheDocument(); }); - test('should display a breadcrumb heading', async () => { - let wrapper; - await act(async () => { - wrapper = mountWithContexts(); - }); - const screenHeader = wrapper.find('ScreenHeader'); - expect(screenHeader).toHaveLength(1); - expect(screenHeader.prop('breadcrumbConfig')).toEqual({ - '/jobs': 'Jobs', - }); + test('preserves the trailing sub-path when redirecting /jobs/system/:id/*', async () => { + const { history } = renderAt('/jobs/system/5/output'); + await waitFor(() => + expect(history.location.pathname).toBe('/jobs/management/5/output') + ); + expect(await screen.findByText('Job detail')).toBeInTheDocument(); }); });