Skip to content

Commit aaa3633

Browse files
pondrejkpondrejk
committed
Fixes #39253 - Implement "Not Found" error handling for invalid Job Invocation IDs
1 parent c3cd96e commit aaa3633

11 files changed

Lines changed: 330 additions & 49 deletions

File tree

‎webpack/JobInvocationDetail/CheckboxesActions.js‎

Lines changed: 4 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import {
1414
} from '@patternfly/react-icons';
1515
import axios from 'axios';
1616
import { foremanUrl } from 'foremanReact/common/helpers';
17+
import { usePermissions } from 'foremanReact/common/hooks/Permissions/permissionHooks';
1718
import { translate as __, sprintf } from 'foremanReact/common/I18n';
1819
import { addToast } from 'foremanReact/components/ToastsList';
1920
import PropTypes from 'prop-types';
@@ -24,10 +25,7 @@ import {
2425
DIRECT_OPEN_HOST_LIMIT,
2526
templateInvocationPageUrl,
2627
} from './JobInvocationConstants';
27-
import {
28-
selectHasPermission,
29-
selectTaskCancelable,
30-
} from './JobInvocationSelectors';
28+
import { selectTaskCancelable } from './JobInvocationSelectors';
3129
import OpenAllInvocationsModal from './OpenAllInvocationsModal';
3230

3331
/* eslint-disable camelcase */
@@ -115,12 +113,8 @@ export const CheckboxesActions = ({
115113
const dispatch = useDispatch();
116114
const [toBeOpened, setToBeOpened] = useState([]);
117115

118-
const hasCreatePermission = useSelector(
119-
selectHasPermission('create_job_invocations')
120-
);
121-
const hasCancelPermission = useSelector(
122-
selectHasPermission('cancel_job_invocations')
123-
);
116+
const hasCreatePermission = usePermissions(['create_job_invocations']);
117+
const hasCancelPermission = usePermissions(['cancel_job_invocations']);
124118
const jobSearchQuery = `job_invocation.id = ${jobID}`;
125119
const filterQuery =
126120
filter && filter !== 'all_statuses'

‎webpack/JobInvocationDetail/JobInvocationConstants.js‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ export const templateInvocationPageUrl = (hostID, jobID) =>
3232
`/job_invocations_detail/${jobID}/host_invocation/${hostID}`;
3333

3434
export const jobInvocationDetailsUrl = id => `/job_invocations/${id}`;
35+
export const jobInvocationsIndexPath = '/job_invocations';
3536

3637
export const STATUS = {
3738
PENDING: 'pending',

‎webpack/JobInvocationDetail/JobInvocationEmptyState.js‎

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
import PropTypes from 'prop-types';
2+
import React from 'react';
3+
import { PageSection, PageSectionVariants } from '@patternfly/react-core';
4+
import { translate as __ } from 'foremanReact/common/I18n';
5+
import { foremanUrl } from 'foremanReact/common/helpers';
6+
import ResourceLoadFailedEmptyState from 'foremanReact/components/common/EmptyState/ResourceLoadFailedEmptyState';
7+
import { jobInvocationsIndexPath } from './JobInvocationConstants';
8+
9+
const JobInvocationEmptyState = ({
10+
jobInvocationId,
11+
httpStatus,
12+
errorMessage,
13+
}) => (
14+
<PageSection variant={PageSectionVariants.light}>
15+
<ResourceLoadFailedEmptyState
16+
resourceLabel={__('job invocation')}
17+
resourceId={jobInvocationId}
18+
httpStatus={httpStatus}
19+
errorMessage={errorMessage}
20+
viewPermissions={['view_job_invocations']}
21+
requiredPermissions={['view_job_invocations']}
22+
primaryAction={{
23+
label: __('Go to job invocations'),
24+
url: foremanUrl(jobInvocationsIndexPath),
25+
ouiaId: 'job-invocation-empty-state-go-to-job-invocations-button',
26+
}}
27+
secondaryActions={[
28+
{
29+
label: __('Create a new job invocation'),
30+
url: foremanUrl('/job_invocations/new'),
31+
ouiaId: 'job-invocation-empty-state-create-new-job-invocation-button',
32+
},
33+
]}
34+
ouiaIdPrefix="job-invocation-empty-state"
35+
/>
36+
</PageSection>
37+
);
38+
39+
JobInvocationEmptyState.propTypes = {
40+
jobInvocationId: PropTypes.string.isRequired,
41+
httpStatus: PropTypes.number,
42+
errorMessage: PropTypes.string,
43+
};
44+
45+
JobInvocationEmptyState.defaultProps = {
46+
httpStatus: null,
47+
errorMessage: null,
48+
};
49+
50+
export default JobInvocationEmptyState;

‎webpack/JobInvocationDetail/JobInvocationToolbarButtons.js‎

Lines changed: 38 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import {
1212
} from '@patternfly/react-core/deprecated';
1313
import { translate as __ } from 'foremanReact/common/I18n';
1414
import { foremanUrl } from 'foremanReact/common/helpers';
15+
import { usePermissions } from 'foremanReact/common/hooks/Permissions/permissionHooks';
1516
import { get } from 'foremanReact/redux/API';
1617
import {
1718
cancelJob,
@@ -23,10 +24,7 @@ import {
2324
GET_REPORT_TEMPLATES,
2425
GET_REPORT_TEMPLATE_INPUTS,
2526
} from './JobInvocationConstants';
26-
import {
27-
selectTaskCancelable,
28-
selectHasPermission,
29-
} from './JobInvocationSelectors';
27+
import { selectTaskCancelable } from './JobInvocationSelectors';
3028

3129
const JobInvocationToolbarButtons = ({ jobId, data }) => {
3230
const { succeeded, failed, task, recurrence, permissions } = data;
@@ -38,8 +36,11 @@ const JobInvocationToolbarButtons = ({ jobId, data }) => {
3836
? permissions.edit_recurring_logics
3937
: false;
4038
const isTaskCancelable = useSelector(selectTaskCancelable);
41-
const useHasPermission = permissionRequired =>
42-
useSelector(selectHasPermission(permissionRequired));
39+
const canCreateJobInvocations = usePermissions(['create_job_invocations']);
40+
const canCancelJobInvocations = usePermissions(['cancel_job_invocations']);
41+
const canGenerateReportTemplates = usePermissions([
42+
'generate_report_templates',
43+
]);
4344
const [isActionOpen, setIsActionOpen] = useState(false);
4445
const [reportTemplateJobId, setReportTemplateJobId] = useState(undefined);
4546
const [templateInputId, setTemplateInputId] = useState(undefined);
@@ -60,39 +61,55 @@ const JobInvocationToolbarButtons = ({ jobId, data }) => {
6061
};
6162

6263
useEffect(() => {
64+
let isMounted = true;
6365
dispatch(
6466
get({
6567
key: GET_REPORT_TEMPLATES,
6668
url: '/api/report_templates',
6769
handleSuccess: ({ data: { results } }) => {
68-
setReportTemplateJobId(
69-
results.find(result => result.name === 'Job - Invocation Report')
70-
?.id
71-
);
70+
if (isMounted) {
71+
setReportTemplateJobId(
72+
results.find(result => result.name === 'Job - Invocation Report')
73+
?.id
74+
);
75+
}
7276
},
7377
handleError: () => {
74-
setReportTemplateJobId(undefined);
78+
if (isMounted) {
79+
setReportTemplateJobId(undefined);
80+
}
7581
},
7682
})
7783
);
84+
return () => {
85+
isMounted = false;
86+
};
7887
}, [dispatch]);
7988
useEffect(() => {
89+
let isMounted = true;
8090
if (reportTemplateJobId !== undefined) {
8191
dispatch(
8292
get({
8393
key: GET_REPORT_TEMPLATE_INPUTS,
8494
url: `/api/templates/${reportTemplateJobId}/template_inputs`,
8595
handleSuccess: ({ data: { results } }) => {
86-
setTemplateInputId(
87-
results.find(result => result.name === 'job_id')?.id
88-
);
96+
if (isMounted) {
97+
setTemplateInputId(
98+
results.find(result => result.name === 'job_id')?.id
99+
);
100+
}
89101
},
90102
handleError: () => {
91-
setTemplateInputId(undefined);
103+
if (isMounted) {
104+
setTemplateInputId(undefined);
105+
}
92106
},
93107
})
94108
);
95109
}
110+
return () => {
111+
isMounted = false;
112+
};
96113
}, [dispatch, reportTemplateJobId]);
97114

98115
const recurrenceDropdownItems = recurrence
@@ -136,9 +153,7 @@ const JobInvocationToolbarButtons = ({ jobId, data }) => {
136153
ouiaId="rerun-succeeded-dropdown-item"
137154
href={foremanUrl(`/job_invocations/${jobId}/rerun?succeeded_only=1`)}
138155
key="rerun-succeeded"
139-
isDisabled={
140-
!useHasPermission('create_job_invocations') || !(succeeded > 0)
141-
}
156+
isDisabled={!canCreateJobInvocations || !(succeeded > 0)}
142157
description="Rerun job on successful hosts"
143158
>
144159
{__('Rerun successful')}
@@ -147,7 +162,7 @@ const JobInvocationToolbarButtons = ({ jobId, data }) => {
147162
ouiaId="rerun-failed-dropdown-item"
148163
href={foremanUrl(`/job_invocations/${jobId}/rerun?failed_only=1`)}
149164
key="rerun-failed"
150-
isDisabled={!useHasPermission('create_job_invocations') || !(failed > 0)}
165+
isDisabled={!canCreateJobInvocations || !(failed > 0)}
151166
description="Rerun job on failed hosts"
152167
>
153168
{__('Rerun failed')}
@@ -167,9 +182,7 @@ const JobInvocationToolbarButtons = ({ jobId, data }) => {
167182
onClick={() => dispatch(cancelJob(jobId, false))}
168183
key="cancel"
169184
component="button"
170-
isDisabled={
171-
!useHasPermission('cancel_job_invocations') || !isTaskCancelable
172-
}
185+
isDisabled={!canCancelJobInvocations || !isTaskCancelable}
173186
description="Cancel job gracefully"
174187
>
175188
{__('Cancel')}
@@ -179,9 +192,7 @@ const JobInvocationToolbarButtons = ({ jobId, data }) => {
179192
onClick={() => dispatch(cancelJob(jobId, true))}
180193
key="abort"
181194
component="button"
182-
isDisabled={
183-
!useHasPermission('cancel_job_invocations') || !isTaskCancelable
184-
}
195+
isDisabled={!canCancelJobInvocations || !isTaskCancelable}
185196
description="Cancel job immediately"
186197
>
187198
{__('Abort')}
@@ -211,7 +222,7 @@ const JobInvocationToolbarButtons = ({ jobId, data }) => {
211222
)}
212223
variant="secondary"
213224
isDisabled={
214-
!useHasPermission('generate_report_templates') ||
225+
!canGenerateReportTemplates ||
215226
task?.state === STATUS.PENDING ||
216227
templateInputId === undefined
217228
}
@@ -235,7 +246,7 @@ const JobInvocationToolbarButtons = ({ jobId, data }) => {
235246
key="rerun"
236247
href={foremanUrl(`/job_invocations/${jobId}/rerun`)}
237248
variant="control"
238-
isDisabled={!useHasPermission('create_job_invocations')}
249+
isDisabled={!canCreateJobInvocations}
239250
>
240251
{__(`Rerun all`)}
241252
</Button>,

‎webpack/JobInvocationDetail/__tests__/JobInvocationDetailEmptyState.test.js‎

Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,82 @@
1+
import React from 'react';
2+
import { render, screen } from '@testing-library/react';
3+
import '@testing-library/jest-dom/extend-expect';
4+
import { createMemoryHistory } from 'history';
5+
import { Router } from 'react-router-dom';
6+
import JobInvocationEmptyState from '../JobInvocationEmptyState';
7+
8+
jest.mock('foremanReact/common/hooks/Permissions/permissionHooks', () => ({
9+
usePermissions: jest.fn(() => true),
10+
}));
11+
12+
jest.mock(
13+
'foremanReact/components/common/EmptyState/ResourceLoadFailedEmptyState',
14+
() => ({
15+
__esModule: true,
16+
default: ({
17+
resourceLabel,
18+
resourceId,
19+
errorMessage,
20+
primaryAction,
21+
secondaryActions,
22+
ouiaIdPrefix,
23+
}) => (
24+
<div data-testid={ouiaIdPrefix}>
25+
<h1>{`Unable to load ${resourceLabel}`}</h1>
26+
<p>{`The ${resourceLabel} with id ${resourceId} could not be found.`}</p>
27+
{errorMessage && <p>{`Server returned: ${errorMessage}`}</p>}
28+
<button type="button" data-testid={primaryAction.ouiaId}>
29+
{primaryAction.label}
30+
</button>
31+
{secondaryActions.map(action => (
32+
<button key={action.ouiaId} type="button" data-testid={action.ouiaId}>
33+
{action.label}
34+
</button>
35+
))}
36+
</div>
37+
),
38+
})
39+
);
40+
41+
describe('JobInvocationEmptyState', () => {
42+
it('renders the failed load empty state for a job invocation', () => {
43+
const jobInvocationId = '99';
44+
const errorMessage = 'Record not found';
45+
const history = createMemoryHistory();
46+
47+
render(
48+
<Router history={history}>
49+
<JobInvocationEmptyState
50+
jobInvocationId={jobInvocationId}
51+
httpStatus={404}
52+
errorMessage={errorMessage}
53+
/>
54+
</Router>
55+
);
56+
57+
expect(
58+
screen.getByTestId('job-invocation-empty-state')
59+
).toBeInTheDocument();
60+
expect(
61+
screen.getByRole('heading', { name: 'Unable to load job invocation' })
62+
).toBeInTheDocument();
63+
expect(
64+
screen.getByText(
65+
`The job invocation with id ${jobInvocationId} could not be found.`
66+
)
67+
).toBeInTheDocument();
68+
expect(
69+
screen.getByText(`Server returned: ${errorMessage}`)
70+
).toBeInTheDocument();
71+
expect(
72+
screen.getByTestId(
73+
'job-invocation-empty-state-go-to-job-invocations-button'
74+
)
75+
).toBeInTheDocument();
76+
expect(
77+
screen.getByTestId(
78+
'job-invocation-empty-state-create-new-job-invocation-button'
79+
)
80+
).toBeInTheDocument();
81+
});
82+
});

0 commit comments

Comments
 (0)