Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
83 changes: 43 additions & 40 deletions awx/ui/src/screens/Job/Job.js
Original file line number Diff line number Diff line change
@@ -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';
Expand Down Expand Up @@ -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];

Expand Down Expand Up @@ -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({
Expand Down Expand Up @@ -172,48 +174,49 @@ function Job({ setBreadcrumb }) {
<div ref={ref}>
<Card>
<RoutedTabs
isWorkflow={match.url.startsWith('/jobs/workflow')}
isWorkflow={typeSegment === 'workflow'}
tabsArray={tabsArray}
/>
<Switch>
<Redirect from="/jobs/system/:id" to="/jobs/management/:id" exact />
<Redirect
from="/jobs/:typeSegment/:id"
to="/jobs/:typeSegment/:id/output"
exact
/>
{job && [
<Routes>
<Route index element={<Navigate to="output" replace />} />
{job && (
<Route
key={
job.type === 'workflow_job' ? 'workflow-details' : 'details'
}
path="/jobs/:typeSegment/:id/details"
>
<JobDetail
job={job}
inventorySourceLabels={inventorySourceChoices}
/>
</Route>,
<Route key="output" path="/jobs/:typeSegment/:id/output">
{job.type === 'workflow_job' ? (
<WorkflowOutput job={job} />
) : (
<JobOutput
path="details"
element={
<JobDetail
job={job}
eventRelatedSearchableKeys={eventRelatedSearchableKeys}
eventSearchableKeys={eventSearchableKeys}
inventorySourceLabels={inventorySourceChoices}
/>
)}
</Route>,
<Route key="not-found" path="*">
}
/>
)}
{job && (
<Route
path="output"
element={
job.type === 'workflow_job' ? (
<WorkflowOutput job={job} />
) : (
<JobOutput
job={job}
eventRelatedSearchableKeys={eventRelatedSearchableKeys}
eventSearchableKeys={eventSearchableKeys}
/>
)
}
/>
)}
<Route
path="*"
element={
<ContentError isNotFound>
<Link to={`/jobs/${typeSegment}/${id}/details`}>
{t`View Job Details`}
</Link>
</ContentError>
</Route>,
]}
</Switch>
}
/>
</Routes>
</Card>
</div>
</PageSection>
Expand Down
6 changes: 4 additions & 2 deletions awx/ui/src/screens/Job/Job.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down
9 changes: 6 additions & 3 deletions awx/ui/src/screens/Job/JobTypeRedirect.js
Original file line number Diff line number Diff line change
@@ -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';
Expand All @@ -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,
Expand Down Expand Up @@ -58,7 +59,9 @@ function JobTypeRedirect({ id, path, view }) {
);
}
const typeSegment = JOB_TYPE_URL_SEGMENTS[job.type];
return <Redirect from={path} to={`/jobs/${typeSegment}/${job.id}/${view}`} />;
return (
<Navigate to={`/jobs/${typeSegment}/${job.id}/${view}`} replace />
);
}

JobTypeRedirect.defaultProps = {
Expand Down
71 changes: 46 additions & 25 deletions awx/ui/src/screens/Job/Jobs.js
Original file line number Diff line number Diff line change
@@ -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';
Expand All @@ -13,13 +18,23 @@ import { JOB_TYPE_URL_SEGMENTS } from '../../constants';

function TypeRedirect({ view }) {
const { id } = useParams();
const { path } = useRouteMatch();
return <JobTypeRedirect id={id} path={path} view={view} />;
return <JobTypeRedirect id={id} view={view} />;
}

// 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 (
<Navigate
to={`/jobs/management/${id}${rest ? `/${rest}` : ''}`}
replace
/>
);
}

function Jobs() {
const { t } = useLingui();
const match = useRouteMatch();
const [breadcrumbConfig, setBreadcrumbConfig] = useState({
'/jobs': t`Jobs`,
});
Expand All @@ -44,27 +59,33 @@ function Jobs() {
return (
<>
<ScreenHeader streamType="job" breadcrumbConfig={breadcrumbConfig} />
<Switch>
<Route exact path={match.path}>
<PageSection>
<PersistentFilters pageKey="jobs">
<JobList showTypeColumn />
</PersistentFilters>
</PageSection>
</Route>
<Route path={`${match.path}/:id/details`}>
<TypeRedirect view="details" />
</Route>
<Route path={`${match.path}/:id/output`}>
<TypeRedirect view="output" />
</Route>
<Route path={`${match.path}/:typeSegment/:id`}>
<Job setBreadcrumb={buildBreadcrumbConfig} />
</Route>
<Route path={`${match.path}/:id`}>
<TypeRedirect />
</Route>
</Switch>
<Routes>
<Route
path="/jobs"
element={
<PageSection>
<PersistentFilters pageKey="jobs">
<JobList showTypeColumn />
</PersistentFilters>
</PageSection>
}
/>
<Route path="/jobs/system/:id/*" element={<SystemRedirect />} />
<Route
path="/jobs/:id/details"
element={<TypeRedirect view="details" />}
/>
<Route
path="/jobs/:id/output"
element={<TypeRedirect view="output" />}
/>
{/* /* so the nested <Job> route tree can match details/output */}
<Route
path="/jobs/:typeSegment/:id/*"
element={<Job setBreadcrumb={buildBreadcrumbConfig} />}
/>
<Route path="/jobs/:id" element={<TypeRedirect />} />
</Routes>
</>
);
}
Expand Down
100 changes: 76 additions & 24 deletions awx/ui/src/screens/Job/Jobs.test.js
Original file line number Diff line number Diff line change
@@ -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 <Routes> 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(<Jobs />, {
context: { router: { history } },
});
}

describe('<Jobs />', () => {
test('initially renders successfully', async () => {
let wrapper;
await act(async () => {
wrapper = mountWithContexts(<Jobs />);
});
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 <JobTypeRedirect /> 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(<Jobs />);
});
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();
});
});