Skip to content

Commit 4f79c5f

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

15 files changed

Lines changed: 356 additions & 78 deletions

‎jest.config.js‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
const tfmConfig = require('@theforeman/test/src/pluginConfig');
2+
const { foremanRelativePath, foremanLocation } = require('@theforeman/find-foreman');
3+
4+
const foremanReactRelative = 'webpack/assets/javascripts/react_app';
5+
const foremanFull = foremanLocation();
6+
const foremanReactFull = foremanRelativePath(foremanReactRelative);
7+
8+
tfmConfig.moduleNameMapper['^foremanReact(.*)$'] = `${foremanReactFull}/$1`;
9+
10+
tfmConfig.resolver = null;
11+
tfmConfig.moduleDirectories = [
12+
`${foremanFull}/node_modules`,
13+
`${foremanFull}/node_modules/@theforeman/vendor-core/node_modules`,
14+
'node_modules',
15+
];
16+
17+
tfmConfig.setupFilesAfterEnv = [
18+
...(tfmConfig.setupFilesAfterEnv || []),
19+
'@testing-library/jest-dom',
20+
];
21+
22+
module.exports = tfmConfig;

‎package.json‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,9 @@
55
"scripts": {
66
"lint": "tfm-lint --plugin -d /webpack",
77
"lint:custom": "eslint ./webpack",
8-
"test": "tfm-test --plugin",
9-
"test:watch": "tfm-test --plugin --watchAll",
10-
"test:current": "tfm-test --plugin --watch",
8+
"test": "tfm-test --plugin --config jest.config.js",
9+
"test:watch": "tfm-test --plugin --config jest.config.js --watchAll",
10+
"test:current": "tfm-test --plugin --config jest.config.js --watch",
1111
"publish-coverage": "tfm-publish-coverage"
1212
},
1313
"repository": {

‎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: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@ import { useForemanHostDetailsPageUrl } from 'foremanReact/Root/Context/ForemanC
66
import JobStatusIcon from '../react_app/components/RecentJobsCard/JobStatusIcon';
77

88
export const JOB_INVOCATION_KEY = 'JOB_INVOCATION_KEY';
9-
export const CURRENT_PERMISSIONS = 'CURRENT_PERMISSIONS';
109
export const UPDATE_JOB = 'UPDATE_JOB';
1110
export const CANCEL_JOB = 'CANCEL_JOB';
1211
export const GET_TASK = 'GET_TASK';
@@ -20,9 +19,6 @@ export const GET_TEMPLATE_INVOCATION = 'GET_TEMPLATE_INVOCATION';
2019
export const DIRECT_OPEN_HOST_LIMIT = 3;
2120
export const ALL_JOB_HOSTS = 'ALL_JOB_HOSTS';
2221
export const AWAITING_STATUS_FILTER = '(job_invocation.result = N/A)';
23-
export const currentPermissionsUrl = foremanUrl(
24-
'/api/v2/permissions/current_permissions'
25-
);
2622

2723
export const showTemplateInvocationUrl = (hostID, jobID) =>
2824
`/show_template_invocation_by_host/${hostID}/job_invocation/${jobID}`;
@@ -32,6 +28,10 @@ export const templateInvocationPageUrl = (hostID, jobID) =>
3228
`/job_invocations_detail/${jobID}/host_invocation/${hostID}`;
3329

3430
export const jobInvocationDetailsUrl = id => `/job_invocations/${id}`;
31+
export const jobInvocationsIndexPath = '/job_invocations';
32+
export const jobInvocationsNewPath = '/job_invocations/new';
33+
export const jobInvocationsIndexUrl = foremanUrl(jobInvocationsIndexPath);
34+
export const jobInvocationsNewUrl = foremanUrl(jobInvocationsNewPath);
3535

3636
export const STATUS = {
3737
PENDING: 'pending',

‎webpack/JobInvocationDetail/JobInvocationEmptyState.js‎

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

‎webpack/JobInvocationDetail/JobInvocationSelectors.js‎

Lines changed: 0 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -3,13 +3,11 @@ import {
33
selectAPIResponse,
44
selectAPIStatus,
55
} from 'foremanReact/redux/API/APISelectors';
6-
import { STATUS as APIStatus } from 'foremanReact/constants';
76
import {
87
JOB_INVOCATION_KEY,
98
GET_TASK,
109
GET_TEMPLATE_INVOCATION,
1110
LIST_TEMPLATE_INVOCATIONS,
12-
CURRENT_PERMISSIONS,
1311
} from './JobInvocationConstants';
1412

1513
export const selectItems = state =>
@@ -29,16 +27,3 @@ export const selectTemplateInvocationStatus = hostID => state =>
2927
export const selectTemplateInvocationList = state =>
3028
selectAPIResponse(state, LIST_TEMPLATE_INVOCATIONS)
3129
?.template_invocations_task_by_hosts;
32-
33-
export const selectCurrentPermisions = state =>
34-
selectAPIResponse(state, CURRENT_PERMISSIONS);
35-
36-
export const selectHasPermission = permissionRequired => state => {
37-
const status = selectAPIStatus(state, CURRENT_PERMISSIONS);
38-
const selectCurrentPermissions = selectCurrentPermisions(state)?.results;
39-
return status === APIStatus.RESOLVED
40-
? selectCurrentPermissions?.some(
41-
permission => permission.name === permissionRequired
42-
)
43-
: false;
44-
};

‎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: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
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+
import { createForemanContextWrapper } from './foremanTestHelpers';
8+
9+
describe('JobInvocationEmptyState', () => {
10+
it('renders the failed load empty state for a job invocation', () => {
11+
const jobInvocationId = '99';
12+
const errorMessage = 'Record not found';
13+
const history = createMemoryHistory();
14+
const ForemanContextWrapper = createForemanContextWrapper();
15+
16+
render(
17+
<Router history={history}>
18+
<ForemanContextWrapper>
19+
<JobInvocationEmptyState
20+
jobInvocationId={jobInvocationId}
21+
httpStatus={404}
22+
errorMessage={errorMessage}
23+
/>
24+
</ForemanContextWrapper>
25+
</Router>
26+
);
27+
28+
expect(
29+
screen.getByRole('heading', {
30+
name: 'Unable to load job invocation',
31+
level: 5,
32+
})
33+
).toBeInTheDocument();
34+
expect(
35+
screen.getByText(
36+
(_, element) =>
37+
element?.tagName === 'P' &&
38+
element.textContent?.includes(
39+
`The job invocation with id ${jobInvocationId} could not be found. It may have been deleted or may not be available in your current organization or location scope.`
40+
)
41+
)
42+
).toBeInTheDocument();
43+
expect(
44+
screen.getByText(`Server returned: ${errorMessage}`)
45+
).toBeInTheDocument();
46+
expect(
47+
screen.getByRole('button', { name: 'Go to job invocations' })
48+
).toBeInTheDocument();
49+
expect(
50+
screen.getByRole('button', { name: 'Create a new job invocation' })
51+
).toBeInTheDocument();
52+
});
53+
});

0 commit comments

Comments
 (0)