Skip to content

Commit 5ce73bc

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

6 files changed

Lines changed: 148 additions & 20 deletions

File tree

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

webpack/JobInvocationDetail/JobInvocationToolbarButtons.js

Lines changed: 25 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -60,39 +60,55 @@ const JobInvocationToolbarButtons = ({ jobId, data }) => {
6060
};
6161

6262
useEffect(() => {
63+
let isMounted = true;
6364
dispatch(
6465
get({
6566
key: GET_REPORT_TEMPLATES,
6667
url: '/api/report_templates',
6768
handleSuccess: ({ data: { results } }) => {
68-
setReportTemplateJobId(
69-
results.find(result => result.name === 'Job - Invocation Report')
70-
?.id
71-
);
69+
if (isMounted) {
70+
setReportTemplateJobId(
71+
results.find(result => result.name === 'Job - Invocation Report')
72+
?.id
73+
);
74+
}
7275
},
7376
handleError: () => {
74-
setReportTemplateJobId(undefined);
77+
if (isMounted) {
78+
setReportTemplateJobId(undefined);
79+
}
7580
},
7681
})
7782
);
83+
return () => {
84+
isMounted = false;
85+
};
7886
}, [dispatch]);
7987
useEffect(() => {
88+
let isMounted = true;
8089
if (reportTemplateJobId !== undefined) {
8190
dispatch(
8291
get({
8392
key: GET_REPORT_TEMPLATE_INPUTS,
8493
url: `/api/templates/${reportTemplateJobId}/template_inputs`,
8594
handleSuccess: ({ data: { results } }) => {
86-
setTemplateInputId(
87-
results.find(result => result.name === 'job_id')?.id
88-
);
95+
if (isMounted) {
96+
setTemplateInputId(
97+
results.find(result => result.name === 'job_id')?.id
98+
);
99+
}
89100
},
90101
handleError: () => {
91-
setTemplateInputId(undefined);
102+
if (isMounted) {
103+
setTemplateInputId(undefined);
104+
}
92105
},
93106
})
94107
);
95108
}
109+
return () => {
110+
isMounted = false;
111+
};
96112
}, [dispatch, reportTemplateJobId]);
97113

98114
const recurrenceDropdownItems = recurrence

webpack/JobInvocationDetail/__tests__/MainInformation.test.js

Lines changed: 20 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ import {
2626
CHANGE_ENABLED_RECURRING_LOGIC,
2727
GET_REPORT_TEMPLATES,
2828
GET_REPORT_TEMPLATE_INPUTS,
29+
GET_TASK,
2930
JOB_INVOCATION_KEY,
3031
} from '../JobInvocationConstants';
3132

@@ -35,8 +36,11 @@ jest.spyOn(api, 'get');
3536
const originalToLocaleString = Date.prototype.toLocaleString;
3637
beforeAll(() => {
3738
// eslint-disable-next-line no-extend-native
38-
Date.prototype.toLocaleString = function (locale, options) {
39-
return originalToLocaleString.call(this, locale, { ...options, timeZone: 'UTC' });
39+
Date.prototype.toLocaleString = function(locale, options) {
40+
return originalToLocaleString.call(this, locale, {
41+
...options,
42+
timeZone: 'UTC',
43+
});
4044
};
4145
});
4246
afterAll(() => {
@@ -91,6 +95,14 @@ api.get.mockImplementation(({ handleSuccess, ...action }) => {
9195
return { type: 'get', ...action };
9296
});
9397

98+
const initialStateRecurring = {
99+
JOB_INVOCATION_KEY: {
100+
response: jobInvocationDataRecurring,
101+
},
102+
GET_REPORT_TEMPLATES: mockReportTemplatesResponse,
103+
extendable: {},
104+
};
105+
94106
jest.mock('../JobInvocationHostTable.js', () => () => (
95107
<div data-testid="mock-table">Mock Table</div>
96108
));
@@ -217,8 +229,9 @@ describe('JobInvocationDetailPage', () => {
217229
it('should dispatch global actions', async () => {
218230
// recurring in the future
219231
const jobId = jobInvocationDataRecurring.id;
232+
const taskId = jobInvocationDataRecurring.task.id;
220233
const recurrenceId = jobInvocationDataRecurring.recurrence.id;
221-
const store = mockStore(jobInvocationDataRecurring);
234+
const store = mockStore(initialStateRecurring);
222235
render(
223236
<Provider store={store}>
224237
<JobInvocationDetailPage
@@ -234,6 +247,10 @@ describe('JobInvocationDetailPage', () => {
234247
key: JOB_INVOCATION_KEY,
235248
url: `/api/job_invocations/${jobId}?host_status=true`,
236249
},
250+
{
251+
key: GET_TASK,
252+
url: `/foreman_tasks/api/tasks/${taskId}`,
253+
},
237254
{
238255
key: GET_REPORT_TEMPLATE_INPUTS,
239256
url: `/api/templates/${reportTemplateJobId}/template_inputs`,

webpack/JobInvocationDetail/index.js

Lines changed: 43 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -5,29 +5,33 @@ import {
55
PageSectionVariants,
66
Skeleton,
77
} from '@patternfly/react-core';
8-
import React, { useEffect, useState } from 'react';
8+
import React, { useEffect, useMemo, useState } from 'react';
99
import { translate as __, documentLocale } from 'foremanReact/common/I18n';
1010
import { useDispatch, useSelector } from 'react-redux';
1111
import PageLayout from 'foremanReact/routes/common/PageLayout/PageLayout';
1212
import PropTypes from 'prop-types';
1313
import SkeletonLoader from 'foremanReact/components/common/SkeletonLoader';
1414
import { stopInterval } from 'foremanReact/redux/middlewares/IntervalMiddleware';
15-
import { useAPI } from 'foremanReact/common/hooks/API/APIHooks';
15+
import { STATUS as API_STATUS } from 'foremanReact/constants';
16+
import {
17+
selectAPIErrorMessage,
18+
selectAPIHttpStatus,
19+
selectAPIStatus,
20+
} from 'foremanReact/redux/API/APISelectors';
1621

1722
import { JobAdditionInfo } from './JobAdditionInfo';
1823
import JobInvocationHostTable from './JobInvocationHostTable';
1924
import JobInvocationOverview from './JobInvocationOverview';
2025
import JobInvocationSystemStatusChart from './JobInvocationSystemStatusChart';
26+
import JobInvocationEmptyState from './JobInvocationEmptyState';
2127
import JobInvocationToolbarButtons from './JobInvocationToolbarButtons';
2228
import { getJobInvocation, getTask } from './JobInvocationActions';
2329
import './JobInvocationDetail.scss';
2430
import {
25-
CURRENT_PERMISSIONS,
2631
DATE_OPTIONS,
2732
JOB_INVOCATION_KEY,
2833
STATUS,
2934
STATUS_UPPERCASE,
30-
currentPermissionsUrl,
3135
} from './JobInvocationConstants';
3236
import { selectItems } from './JobInvocationSelectors';
3337

@@ -51,9 +55,15 @@ const JobInvocationDetailPage = ({
5155
statusLabel === STATUS.SUCCEEDED ||
5256
statusLabel === STATUS.CANCELLED;
5357
const autoRefresh = task?.state === STATUS.PENDING || false;
54-
useAPI('get', currentPermissionsUrl, {
55-
key: CURRENT_PERMISSIONS,
56-
});
58+
const jobInvocationApiStatus = useSelector(state =>
59+
selectAPIStatus(state, JOB_INVOCATION_KEY)
60+
);
61+
const jobInvocationErrorMessage = useSelector(state =>
62+
selectAPIErrorMessage(state, JOB_INVOCATION_KEY)
63+
);
64+
const jobInvocationHttpStatus = useSelector(state =>
65+
selectAPIHttpStatus(state, JOB_INVOCATION_KEY)
66+
);
5767
const [selectedFilter, setSelectedFilter] = useState('');
5868

5969
const handleFilterChange = newFilter => {
@@ -88,6 +98,27 @@ const JobInvocationDetailPage = ({
8898
}
8999
}, [dispatch, taskId]);
90100

101+
const apiFailed = jobInvocationApiStatus === API_STATUS.ERROR;
102+
103+
const backendErrorMessage = useMemo(() => {
104+
if (jobInvocationApiStatus === API_STATUS.ERROR) {
105+
return jobInvocationErrorMessage || null;
106+
}
107+
return null;
108+
}, [jobInvocationApiStatus, jobInvocationErrorMessage]);
109+
110+
const httpStatus = jobInvocationHttpStatus;
111+
112+
if (apiFailed) {
113+
return (
114+
<JobInvocationEmptyState
115+
jobInvocationId={id}
116+
httpStatus={httpStatus}
117+
errorMessage={backendErrorMessage}
118+
/>
119+
);
120+
}
121+
91122
const pageStatus =
92123
items.id === undefined
93124
? STATUS_UPPERCASE.PENDING
@@ -123,7 +154,11 @@ const JobInvocationDetailPage = ({
123154
<PageLayout
124155
header={description}
125156
breadcrumbOptions={breadcrumbOptions}
126-
toolbarButtons={<JobInvocationToolbarButtons jobId={id} data={items} />}
157+
toolbarButtons={
158+
jobInvocationApiStatus === API_STATUS.SUCCESS && (
159+
<JobInvocationToolbarButtons jobId={id} data={items} />
160+
)
161+
}
127162
searchable={false}
128163
>
129164
<Flex className="job-invocation-detail-flex">
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1 +1,5 @@
11
export const foremanUrl = path => `foreman${path}`;
2+
3+
export const visit = jest.fn(url => {
4+
global.window.location.href = url;
5+
});

webpack/__mocks__/foremanReact/redux/API/APISelectors.js

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,3 +19,8 @@ export const selectAPIErrorMessage = (state, key) => {
1919
const error = selectAPIError(state, key);
2020
return error && error.message;
2121
};
22+
23+
export const selectAPIHttpStatus = (state, key) => {
24+
const error = selectAPIError(state, key);
25+
return error?.response?.status;
26+
};

0 commit comments

Comments
 (0)