Skip to content

Commit 8f0283f

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

6 files changed

Lines changed: 181 additions & 17 deletions

File tree

‎webpack/JobInvocationDetail/JobInvocationEmptyState.js‎

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: 76 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5,19 +5,26 @@ 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';
1515
import { useAPI } from 'foremanReact/common/hooks/API/APIHooks';
16+
import { STATUS as API_STATUS } from 'foremanReact/constants';
17+
import {
18+
selectAPIErrorMessage,
19+
selectAPIHttpStatus,
20+
selectAPIStatus,
21+
} from 'foremanReact/redux/API/APISelectors';
1622

1723
import { JobAdditionInfo } from './JobAdditionInfo';
1824
import JobInvocationHostTable from './JobInvocationHostTable';
1925
import JobInvocationOverview from './JobInvocationOverview';
2026
import JobInvocationSystemStatusChart from './JobInvocationSystemStatusChart';
27+
import JobInvocationEmptyState from './JobInvocationEmptyState';
2128
import JobInvocationToolbarButtons from './JobInvocationToolbarButtons';
2229
import { getJobInvocation, getTask } from './JobInvocationActions';
2330
import './JobInvocationDetail.scss';
@@ -51,9 +58,28 @@ const JobInvocationDetailPage = ({
5158
statusLabel === STATUS.SUCCEEDED ||
5259
statusLabel === STATUS.CANCELLED;
5360
const autoRefresh = task?.state === STATUS.PENDING || false;
54-
useAPI('get', currentPermissionsUrl, {
55-
key: CURRENT_PERMISSIONS,
56-
});
61+
const { status: permissionsApiStatus } = useAPI(
62+
'get',
63+
currentPermissionsUrl,
64+
{
65+
key: CURRENT_PERMISSIONS,
66+
}
67+
);
68+
const jobInvocationApiStatus = useSelector(state =>
69+
selectAPIStatus(state, JOB_INVOCATION_KEY)
70+
);
71+
const jobInvocationErrorMessage = useSelector(state =>
72+
selectAPIErrorMessage(state, JOB_INVOCATION_KEY)
73+
);
74+
const permissionsErrorMessage = useSelector(state =>
75+
selectAPIErrorMessage(state, CURRENT_PERMISSIONS)
76+
);
77+
const jobInvocationHttpStatus = useSelector(state =>
78+
selectAPIHttpStatus(state, JOB_INVOCATION_KEY)
79+
);
80+
const permissionsHttpStatus = useSelector(state =>
81+
selectAPIHttpStatus(state, CURRENT_PERMISSIONS)
82+
);
5783
const [selectedFilter, setSelectedFilter] = useState('');
5884

5985
const handleFilterChange = newFilter => {
@@ -88,6 +114,47 @@ const JobInvocationDetailPage = ({
88114
}
89115
}, [dispatch, taskId]);
90116

117+
const apiFailed =
118+
permissionsApiStatus === API_STATUS.ERROR ||
119+
jobInvocationApiStatus === API_STATUS.ERROR;
120+
121+
const backendErrorMessage = useMemo(() => {
122+
const parts = [];
123+
if (jobInvocationApiStatus === API_STATUS.ERROR) {
124+
const msg = jobInvocationErrorMessage;
125+
if (msg) {
126+
parts.push(msg);
127+
}
128+
}
129+
if (permissionsApiStatus === API_STATUS.ERROR) {
130+
const msg = permissionsErrorMessage;
131+
if (msg) {
132+
parts.push(msg);
133+
}
134+
}
135+
return parts.length ? parts.join('\n\n') : null;
136+
}, [
137+
jobInvocationApiStatus,
138+
permissionsApiStatus,
139+
jobInvocationErrorMessage,
140+
permissionsErrorMessage,
141+
]);
142+
143+
const httpStatus =
144+
jobInvocationApiStatus === API_STATUS.ERROR
145+
? jobInvocationHttpStatus
146+
: permissionsHttpStatus;
147+
148+
if (apiFailed) {
149+
return (
150+
<JobInvocationEmptyState
151+
jobInvocationId={id}
152+
httpStatus={httpStatus}
153+
errorMessage={backendErrorMessage}
154+
/>
155+
);
156+
}
157+
91158
const pageStatus =
92159
items.id === undefined
93160
? STATUS_UPPERCASE.PENDING
@@ -123,7 +190,11 @@ const JobInvocationDetailPage = ({
123190
<PageLayout
124191
header={description}
125192
breadcrumbOptions={breadcrumbOptions}
126-
toolbarButtons={<JobInvocationToolbarButtons jobId={id} data={items} />}
193+
toolbarButtons={
194+
jobInvocationApiStatus === API_STATUS.SUCCESS && (
195+
<JobInvocationToolbarButtons jobId={id} data={items} />
196+
)
197+
}
127198
searchable={false}
128199
>
129200
<Flex className="job-invocation-detail-flex">

‎webpack/__mocks__/foremanReact/common/helpers.js‎

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)