From 9ad68554b9d11bcf2985384fae44f0d063ba836c Mon Sep 17 00:00:00 2001 From: Adam Ruzicka Date: Tue, 26 May 2026 10:32:46 +0200 Subject: [PATCH 01/27] Fixes #39363 - Do not ask for host statuses when reading job invocation details The extra data it gave doesn't seem to be used anywhere in the job invocation details page. --- .../JobInvocationDetail/__tests__/MainInformation.test.js | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/webpack/JobInvocationDetail/__tests__/MainInformation.test.js b/webpack/JobInvocationDetail/__tests__/MainInformation.test.js index d30172220..90f04640a 100644 --- a/webpack/JobInvocationDetail/__tests__/MainInformation.test.js +++ b/webpack/JobInvocationDetail/__tests__/MainInformation.test.js @@ -35,8 +35,11 @@ jest.spyOn(api, 'get'); const originalToLocaleString = Date.prototype.toLocaleString; beforeAll(() => { // eslint-disable-next-line no-extend-native - Date.prototype.toLocaleString = function (locale, options) { - return originalToLocaleString.call(this, locale, { ...options, timeZone: 'UTC' }); + Date.prototype.toLocaleString = function(locale, options) { + return originalToLocaleString.call(this, locale, { + ...options, + timeZone: 'UTC', + }); }; }); afterAll(() => { From ece91808b07749ec1ed158b68b3703fd5fa052c4 Mon Sep 17 00:00:00 2001 From: Adam Ruzicka Date: Tue, 26 May 2026 12:48:05 +0200 Subject: [PATCH 02/27] Wait for previous poll response before scheduling the next one Replace setInterval-based polling (via withInterval) with a setTimeout chain that only schedules the next request after the previous one completes or fails, preventing request pileup when the server is slow. Co-Authored-By: Claude Opus 4.6 --- .../JobInvocationActions.js | 32 +++++++++++++------ webpack/JobInvocationDetail/index.js | 12 ++++--- 2 files changed, 29 insertions(+), 15 deletions(-) diff --git a/webpack/JobInvocationDetail/JobInvocationActions.js b/webpack/JobInvocationDetail/JobInvocationActions.js index 131696068..ab8d9dbca 100644 --- a/webpack/JobInvocationDetail/JobInvocationActions.js +++ b/webpack/JobInvocationDetail/JobInvocationActions.js @@ -2,10 +2,6 @@ import { translate as __, sprintf } from 'foremanReact/common/I18n'; import { foremanUrl } from 'foremanReact/common/helpers'; import { addToast } from 'foremanReact/components/ToastsList'; import { APIActions, get } from 'foremanReact/redux/API'; -import { - stopInterval, - withInterval, -} from 'foremanReact/redux/middlewares/IntervalMiddleware'; import { CANCEL_JOB, CANCEL_RECURRING_LOGIC, @@ -15,14 +11,21 @@ import { UPDATE_JOB, } from './JobInvocationConstants'; -export const getJobInvocation = url => dispatch => { - const fetchData = withInterval( +let pollTimeoutId = null; + +const scheduleNextPoll = (dispatch, url) => { + pollTimeoutId = setTimeout(() => fetchJobInvocation(dispatch, url), 1000); +}; + +const fetchJobInvocation = (dispatch, url) => { + dispatch( get({ key: JOB_INVOCATION_KEY, params: { include_permissions: true, include_hosts: false }, url, + handleSuccess: () => scheduleNextPoll(dispatch, url), handleError: () => { - dispatch(stopInterval(JOB_INVOCATION_KEY)); + pollTimeoutId = null; }, errorToast: ({ response }) => // eslint-disable-next-line camelcase @@ -31,11 +34,20 @@ export const getJobInvocation = url => dispatch => { response?.data?.error?.full_messages || response?.data?.error?.message || 'Error', - }), - 1000 + }) ); +}; + +export const getJobInvocation = url => dispatch => { + stopJobInvocationPolling(); + fetchJobInvocation(dispatch, url); +}; - dispatch(fetchData); +export const stopJobInvocationPolling = () => { + if (pollTimeoutId !== null) { + clearTimeout(pollTimeoutId); + pollTimeoutId = null; + } }; export const updateJob = jobId => dispatch => { diff --git a/webpack/JobInvocationDetail/index.js b/webpack/JobInvocationDetail/index.js index 34fc19443..dfa0b75b3 100644 --- a/webpack/JobInvocationDetail/index.js +++ b/webpack/JobInvocationDetail/index.js @@ -11,7 +11,6 @@ import { useDispatch, useSelector } from 'react-redux'; import PageLayout from 'foremanReact/routes/common/PageLayout/PageLayout'; import PropTypes from 'prop-types'; import SkeletonLoader from 'foremanReact/components/common/SkeletonLoader'; -import { stopInterval } from 'foremanReact/redux/middlewares/IntervalMiddleware'; import { useAPI } from 'foremanReact/common/hooks/API/APIHooks'; import { JobAdditionInfo } from './JobAdditionInfo'; @@ -19,12 +18,15 @@ import JobInvocationHostTable from './JobInvocationHostTable'; import JobInvocationOverview from './JobInvocationOverview'; import JobInvocationSystemStatusChart from './JobInvocationSystemStatusChart'; import JobInvocationToolbarButtons from './JobInvocationToolbarButtons'; -import { getJobInvocation, getTask } from './JobInvocationActions'; +import { + getJobInvocation, + getTask, + stopJobInvocationPolling, +} from './JobInvocationActions'; import './JobInvocationDetail.scss'; import { CURRENT_PERMISSIONS, DATE_OPTIONS, - JOB_INVOCATION_KEY, STATUS, STATUS_UPPERCASE, currentPermissionsUrl, @@ -74,10 +76,10 @@ const JobInvocationDetailPage = ({ useEffect(() => { dispatch(getJobInvocation(`/api/job_invocations/${id}`)); if (finished && !autoRefresh) { - dispatch(stopInterval(JOB_INVOCATION_KEY)); + stopJobInvocationPolling(); } return () => { - dispatch(stopInterval(JOB_INVOCATION_KEY)); + stopJobInvocationPolling(); }; }, [dispatch, id, finished, autoRefresh]); From 9f735332fdbb4550eb953a9565d2aef52f3f450d Mon Sep 17 00:00:00 2001 From: Adam Ruzicka Date: Tue, 26 May 2026 12:59:07 +0200 Subject: [PATCH 03/27] Fetch permissions only on initial load, not on every poll tick The include_permissions param causes the backend to instantiate an Authorizer and check 3 permissions on every request. These are user permissions that don't change during a page session, so fetch them once on the initial request and skip them on subsequent polls. Co-Authored-By: Claude Opus 4.6 --- webpack/JobInvocationDetail/JobInvocationActions.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/webpack/JobInvocationDetail/JobInvocationActions.js b/webpack/JobInvocationDetail/JobInvocationActions.js index ab8d9dbca..f783866a5 100644 --- a/webpack/JobInvocationDetail/JobInvocationActions.js +++ b/webpack/JobInvocationDetail/JobInvocationActions.js @@ -17,11 +17,11 @@ const scheduleNextPoll = (dispatch, url) => { pollTimeoutId = setTimeout(() => fetchJobInvocation(dispatch, url), 1000); }; -const fetchJobInvocation = (dispatch, url) => { +const fetchJobInvocation = (dispatch, url, params = {}) => { dispatch( get({ key: JOB_INVOCATION_KEY, - params: { include_permissions: true, include_hosts: false }, + params: { include_hosts: false }, url, handleSuccess: () => scheduleNextPoll(dispatch, url), handleError: () => { @@ -40,7 +40,7 @@ const fetchJobInvocation = (dispatch, url) => { export const getJobInvocation = url => dispatch => { stopJobInvocationPolling(); - fetchJobInvocation(dispatch, url); + fetchJobInvocation(dispatch, url, { include_permissions: true }); }; export const stopJobInvocationPolling = () => { From 6733116a70b11ea2270fb8b2b6fb16acf9d01b37 Mon Sep 17 00:00:00 2001 From: Adam Ruzicka Date: Tue, 26 May 2026 13:03:08 +0200 Subject: [PATCH 04/27] =?UTF-8?q?Remove=20updateJob=20=E2=80=94=20its=20re?= =?UTF-8?q?sponse=20was=20never=20read?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit updateJob fetched /api/job_invocations/:id into the UPDATE_JOB Redux key, but no selector ever read from that key. The polling loop already refreshes the job state via JOB_INVOCATION_KEY every second, so these extra requests after cancel/abort/recurring actions were wasted. Co-Authored-By: Claude Opus 4.6 --- .../JobInvocationDetail/JobInvocationActions.js | 16 ---------------- 1 file changed, 16 deletions(-) diff --git a/webpack/JobInvocationDetail/JobInvocationActions.js b/webpack/JobInvocationDetail/JobInvocationActions.js index f783866a5..ebb9839e0 100644 --- a/webpack/JobInvocationDetail/JobInvocationActions.js +++ b/webpack/JobInvocationDetail/JobInvocationActions.js @@ -1,5 +1,4 @@ import { translate as __, sprintf } from 'foremanReact/common/I18n'; -import { foremanUrl } from 'foremanReact/common/helpers'; import { addToast } from 'foremanReact/components/ToastsList'; import { APIActions, get } from 'foremanReact/redux/API'; import { @@ -8,7 +7,6 @@ import { CHANGE_ENABLED_RECURRING_LOGIC, GET_TASK, JOB_INVOCATION_KEY, - UPDATE_JOB, } from './JobInvocationConstants'; let pollTimeoutId = null; @@ -50,17 +48,6 @@ export const stopJobInvocationPolling = () => { } }; -export const updateJob = jobId => dispatch => { - const url = foremanUrl(`/api/job_invocations/${jobId}`); - dispatch( - APIActions.get({ - url, - key: UPDATE_JOB, - params: { include_hosts: false }, - }) - ); -}; - export const cancelJob = (jobId, force) => dispatch => { const infoToast = () => force @@ -93,7 +80,6 @@ export const cancelJob = (jobId, force) => dispatch => { message: infoToast(), }) ); - dispatch(updateJob(jobId)); }, }) ); @@ -141,7 +127,6 @@ export const enableRecurringLogic = ( response?.data?.error?.message || 'Unknown error.' ), - handleSuccess: () => dispatch(updateJob(jobId)), }) ); }; @@ -167,7 +152,6 @@ export const cancelRecurringLogic = (recurrenceId, jobId) => dispatch => { response?.data?.error?.message || 'Unknown error.' ), - handleSuccess: () => dispatch(updateJob(jobId)), }) ); }; From bbcbbbe86a26a6707d5710444f22ef17a395047d Mon Sep 17 00:00:00 2001 From: Adam Ruzicka Date: Tue, 26 May 2026 13:07:09 +0200 Subject: [PATCH 05/27] Fix host table firing duplicate API calls on mount MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The second useEffect called filterApiCall() unconditionally whenever any dependency changed, and again when statusLabel changed — causing 2-3 identical requests on initial load. Track previous values for both initialFilter and statusLabel so filterApiCall fires only once per actual change. Also move setStatus(RESOLVED) inside the JOB_INVOCATION_HOSTS branch of handleResponse so the LIST_TEMPLATE_INVOCATIONS fetch (which doesn't populate table data) doesn't prematurely flip the table out of its loading state. Co-Authored-By: Claude Opus 4.6 --- .../JobInvocationDetail/JobInvocationHostTable.js | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/webpack/JobInvocationDetail/JobInvocationHostTable.js b/webpack/JobInvocationDetail/JobInvocationHostTable.js index 9daaafe58..bac06d4b8 100644 --- a/webpack/JobInvocationDetail/JobInvocationHostTable.js +++ b/webpack/JobInvocationDetail/JobInvocationHostTable.js @@ -70,6 +70,7 @@ const JobInvocationHostTable = ({ // Expansive items const [expandedHost, setExpandedHost] = useState(new Set()); const prevStatusLabel = useRef(statusLabel); + const prevFilter = useRef(initialFilter); const [hostInvocationStates, setHostInvocationStates] = useState({}); @@ -159,9 +160,8 @@ const JobInvocationHostTable = ({ setApiResponse(data.data); setAllHostsIds(ids); + setStatus(STATUS_UPPERCASE.RESOLVED); } - - setStatus(STATUS_UPPERCASE.RESOLVED); }, []); // Call hosts data with params @@ -252,10 +252,13 @@ const JobInvocationHostTable = ({ }, [makeApiCall, id, initialFilter, onFilterUpdate]); useEffect(() => { - if (initialFilter !== '') filterApiCall(); + const filterChanged = initialFilter !== prevFilter.current; + const statusChanged = statusLabel !== prevStatusLabel.current; + + prevFilter.current = initialFilter; + prevStatusLabel.current = statusLabel; - if (statusLabel !== prevStatusLabel.current) { - prevStatusLabel.current = statusLabel; + if ((filterChanged || statusChanged) && initialFilter !== '') { filterApiCall(); } }, [initialFilter, statusLabel, id, filterApiCall]); From 861ceb2f71072ac61d7e007435d325fb8993a588 Mon Sep 17 00:00:00 2001 From: Adam Ruzicka Date: Tue, 26 May 2026 13:17:33 +0200 Subject: [PATCH 06/27] Include cancellable in job invocation task, drop separate task fetch The detail page fetched /foreman_tasks/api/tasks/:id solely to read available_actions.cancellable. Add cancellable to the task child in the job invocation response instead, eliminating a separate API call. Co-Authored-By: Claude Opus 4.6 --- app/views/api/v2/job_invocations/main.json.rabl | 1 + webpack/JobInvocationDetail/JobInvocationActions.js | 10 ---------- webpack/JobInvocationDetail/JobInvocationSelectors.js | 5 +---- webpack/JobInvocationDetail/index.js | 8 -------- 4 files changed, 2 insertions(+), 22 deletions(-) diff --git a/app/views/api/v2/job_invocations/main.json.rabl b/app/views/api/v2/job_invocations/main.json.rabl index 3e4111a44..56d8de551 100644 --- a/app/views/api/v2/job_invocations/main.json.rabl +++ b/app/views/api/v2/job_invocations/main.json.rabl @@ -34,6 +34,7 @@ end child :task do attributes :id, :state, :started_at + node(:cancellable) { |task| task.cancellable? } end if @template_invocations diff --git a/webpack/JobInvocationDetail/JobInvocationActions.js b/webpack/JobInvocationDetail/JobInvocationActions.js index ebb9839e0..8d0c2fb9b 100644 --- a/webpack/JobInvocationDetail/JobInvocationActions.js +++ b/webpack/JobInvocationDetail/JobInvocationActions.js @@ -5,7 +5,6 @@ import { CANCEL_JOB, CANCEL_RECURRING_LOGIC, CHANGE_ENABLED_RECURRING_LOGIC, - GET_TASK, JOB_INVOCATION_KEY, } from './JobInvocationConstants'; @@ -85,15 +84,6 @@ export const cancelJob = (jobId, force) => dispatch => { ); }; -export const getTask = taskId => dispatch => { - dispatch( - get({ - key: GET_TASK, - url: `/foreman_tasks/api/tasks/${taskId}`, - }) - ); -}; - export const enableRecurringLogic = ( recurrenceId, enabled, diff --git a/webpack/JobInvocationDetail/JobInvocationSelectors.js b/webpack/JobInvocationDetail/JobInvocationSelectors.js index eca70541a..93adfa9ab 100644 --- a/webpack/JobInvocationDetail/JobInvocationSelectors.js +++ b/webpack/JobInvocationDetail/JobInvocationSelectors.js @@ -6,7 +6,6 @@ import { import { STATUS as APIStatus } from 'foremanReact/constants'; import { JOB_INVOCATION_KEY, - GET_TASK, GET_TEMPLATE_INVOCATION, LIST_TEMPLATE_INVOCATIONS, CURRENT_PERMISSIONS, @@ -15,10 +14,8 @@ import { export const selectItems = state => selectAPIResponse(state, JOB_INVOCATION_KEY); -export const selectTask = state => selectAPIResponse(state, GET_TASK); - export const selectTaskCancelable = state => - selectTask(state).available_actions?.cancellable || false; + selectItems(state).task?.cancellable || false; export const selectTemplateInvocation = hostID => state => selectAPIResponse(state, `${GET_TEMPLATE_INVOCATION}_${hostID}`); diff --git a/webpack/JobInvocationDetail/index.js b/webpack/JobInvocationDetail/index.js index dfa0b75b3..42aa14432 100644 --- a/webpack/JobInvocationDetail/index.js +++ b/webpack/JobInvocationDetail/index.js @@ -20,7 +20,6 @@ import JobInvocationSystemStatusChart from './JobInvocationSystemStatusChart'; import JobInvocationToolbarButtons from './JobInvocationToolbarButtons'; import { getJobInvocation, - getTask, stopJobInvocationPolling, } from './JobInvocationActions'; import './JobInvocationDetail.scss'; @@ -83,13 +82,6 @@ const JobInvocationDetailPage = ({ }; }, [dispatch, id, finished, autoRefresh]); - const taskId = task?.id; - useEffect(() => { - if (taskId !== undefined) { - dispatch(getTask(`${taskId}`)); - } - }, [dispatch, taskId]); - const pageStatus = items.id === undefined ? STATUS_UPPERCASE.PENDING From aace775b26f92ba1f4500ac3fc6c5775fdd5d6a0 Mon Sep 17 00:00:00 2001 From: Adam Ruzicka Date: Tue, 26 May 2026 13:21:29 +0200 Subject: [PATCH 07/27] Replace setInterval with setTimeout chain in TemplateInvocation polling Co-Authored-By: Claude Opus 4.6 --- .../JobInvocationDetail/TemplateInvocation.js | 48 +++++++++---------- 1 file changed, 22 insertions(+), 26 deletions(-) diff --git a/webpack/JobInvocationDetail/TemplateInvocation.js b/webpack/JobInvocationDetail/TemplateInvocation.js index 9045be218..03575ccd6 100644 --- a/webpack/JobInvocationDetail/TemplateInvocation.js +++ b/webpack/JobInvocationDetail/TemplateInvocation.js @@ -67,7 +67,6 @@ export const TemplateInvocation = ({ showCommand, setShowCommand, }) => { - const intervalRef = useRef(null); const templateURL = showTemplateInvocationUrl(hostID, jobID); const hostDetailsPageUrl = useForemanHostDetailsPageUrl(); @@ -75,48 +74,45 @@ export const TemplateInvocation = ({ const response = useSelector(selectTemplateInvocation(hostID)); const dispatch = useDispatch(); - const responseRef = useRef(response); - useEffect(() => { - responseRef.current = response; - }, [response]); + const timeoutRef = useRef(null); useEffect(() => { + const scheduleNextPoll = () => { + timeoutRef.current = setTimeout(() => dispatchFetch(), 5000); + }; + const dispatchFetch = () => { dispatch( APIActions.get({ url: templateURL, key: `${GET_TEMPLATE_INVOCATION}_${hostID}`, + handleSuccess: ({ data }) => { + const finished = data?.finished ?? true; + // eslint-disable-next-line camelcase + const autoRefresh = data?.auto_refresh || false; + if (!finished && autoRefresh) { + scheduleNextPoll(); + } + }, + handleError: () => { + timeoutRef.current = null; + }, }) ); }; - if (intervalRef.current) { - clearInterval(intervalRef.current); - intervalRef.current = null; + if (timeoutRef.current) { + clearTimeout(timeoutRef.current); + timeoutRef.current = null; } if (isExpanded) { - if (isEmpty(responseRef.current)) { - dispatchFetch(); - } - - intervalRef.current = setInterval(() => { - const latestResponse = responseRef.current; - const finished = latestResponse?.finished ?? true; - // eslint-disable-next-line camelcase - const autoRefresh = latestResponse?.auto_refresh || false; - - if (!finished && autoRefresh) { - dispatchFetch(); - } else if (intervalRef.current) { - clearInterval(intervalRef.current); - } - }, 5000); + dispatchFetch(); } return () => { - if (intervalRef.current) { - clearInterval(intervalRef.current); + if (timeoutRef.current) { + clearTimeout(timeoutRef.current); } }; }, [isExpanded, dispatch, templateURL, hostID]); From 8061dce1d7fbeeb6573903ba638997d92928c929 Mon Sep 17 00:00:00 2001 From: Adam Ruzicka Date: Tue, 26 May 2026 13:32:29 +0200 Subject: [PATCH 08/27] Stop job invocation polling when the job finishes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit handleSuccess unconditionally rescheduled the next poll, so the useEffect's stopJobInvocationPolling call raced against in-flight requests — the cleared timeout was immediately replaced by the completing fetch. Check the response data inside handleSuccess instead, and simplify the effect to only handle unmount cleanup. Co-Authored-By: Claude Opus 4.6 --- .../JobInvocationActions.js | 18 +++++++++++++++++- webpack/JobInvocationDetail/index.js | 5 +---- 2 files changed, 18 insertions(+), 5 deletions(-) diff --git a/webpack/JobInvocationDetail/JobInvocationActions.js b/webpack/JobInvocationDetail/JobInvocationActions.js index 8d0c2fb9b..cfa24137e 100644 --- a/webpack/JobInvocationDetail/JobInvocationActions.js +++ b/webpack/JobInvocationDetail/JobInvocationActions.js @@ -6,10 +6,20 @@ import { CANCEL_RECURRING_LOGIC, CHANGE_ENABLED_RECURRING_LOGIC, JOB_INVOCATION_KEY, + STATUS, } from './JobInvocationConstants'; let pollTimeoutId = null; +const isJobFinished = ({ status_label: statusLabel, task }) => { + const finished = + statusLabel === STATUS.FAILED || + statusLabel === STATUS.SUCCEEDED || + statusLabel === STATUS.CANCELLED; + const autoRefresh = task?.state === STATUS.PENDING || false; + return finished && !autoRefresh; +}; + const scheduleNextPoll = (dispatch, url) => { pollTimeoutId = setTimeout(() => fetchJobInvocation(dispatch, url), 1000); }; @@ -20,7 +30,13 @@ const fetchJobInvocation = (dispatch, url, params = {}) => { key: JOB_INVOCATION_KEY, params: { include_hosts: false }, url, - handleSuccess: () => scheduleNextPoll(dispatch, url), + handleSuccess: ({ data }) => { + if (!isJobFinished(data)) { + scheduleNextPoll(dispatch, url); + } else { + pollTimeoutId = null; + } + }, handleError: () => { pollTimeoutId = null; }, diff --git a/webpack/JobInvocationDetail/index.js b/webpack/JobInvocationDetail/index.js index 42aa14432..83df98eb5 100644 --- a/webpack/JobInvocationDetail/index.js +++ b/webpack/JobInvocationDetail/index.js @@ -74,13 +74,10 @@ const JobInvocationDetailPage = ({ useEffect(() => { dispatch(getJobInvocation(`/api/job_invocations/${id}`)); - if (finished && !autoRefresh) { - stopJobInvocationPolling(); - } return () => { stopJobInvocationPolling(); }; - }, [dispatch, id, finished, autoRefresh]); + }, [dispatch, id]); const pageStatus = items.id === undefined From 6c431762d21bff28da407d1031fbfb0d08bfa150 Mon Sep 17 00:00:00 2001 From: Adam Ruzicka Date: Thu, 2 Jul 2026 15:35:20 +0200 Subject: [PATCH 09/27] =?UTF-8?q?Drop=20dead=20autoRefresh=20logic=20?= =?UTF-8?q?=E2=80=94=20status=5Flabel=20already=20implies=20task=20state?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit status_label is derived from task.state: it reaches a terminal value (failed/succeeded/cancelled) only when task.state == 'stopped', so checking task.state separately adds nothing. Remove the autoRefresh variable and the unused finished/autoRefresh props passed to JobInvocationHostTable. Co-Authored-By: Claude Sonnet 4.6 --- webpack/JobInvocationDetail/JobInvocationActions.js | 12 ++++-------- webpack/JobInvocationDetail/index.js | 7 ------- 2 files changed, 4 insertions(+), 15 deletions(-) diff --git a/webpack/JobInvocationDetail/JobInvocationActions.js b/webpack/JobInvocationDetail/JobInvocationActions.js index cfa24137e..5b6ebb197 100644 --- a/webpack/JobInvocationDetail/JobInvocationActions.js +++ b/webpack/JobInvocationDetail/JobInvocationActions.js @@ -11,14 +11,10 @@ import { let pollTimeoutId = null; -const isJobFinished = ({ status_label: statusLabel, task }) => { - const finished = - statusLabel === STATUS.FAILED || - statusLabel === STATUS.SUCCEEDED || - statusLabel === STATUS.CANCELLED; - const autoRefresh = task?.state === STATUS.PENDING || false; - return finished && !autoRefresh; -}; +const isJobFinished = ({ status_label: statusLabel }) => + statusLabel === STATUS.FAILED || + statusLabel === STATUS.SUCCEEDED || + statusLabel === STATUS.CANCELLED; const scheduleNextPoll = (dispatch, url) => { pollTimeoutId = setTimeout(() => fetchJobInvocation(dispatch, url), 1000); diff --git a/webpack/JobInvocationDetail/index.js b/webpack/JobInvocationDetail/index.js index 83df98eb5..60a22181a 100644 --- a/webpack/JobInvocationDetail/index.js +++ b/webpack/JobInvocationDetail/index.js @@ -47,11 +47,6 @@ const JobInvocationDetailPage = ({ start_at: startAt, targeting = {}, } = items; - const finished = - statusLabel === STATUS.FAILED || - statusLabel === STATUS.SUCCEEDED || - statusLabel === STATUS.CANCELLED; - const autoRefresh = task?.state === STATUS.PENDING || false; useAPI('get', currentPermissionsUrl, { key: CURRENT_PERMISSIONS, }); @@ -174,8 +169,6 @@ const JobInvocationDetailPage = ({ Date: Thu, 2 Jul 2026 15:41:38 +0200 Subject: [PATCH 10/27] Fix include_permissions being dropped from initial job invocation fetch fetchJobInvocation hardcoded params: { include_hosts: false } and ignored its params argument. Spread the argument so the initial load can pass include_permissions: true while subsequent polls omit it. Co-Authored-By: Claude Sonnet 4.6 --- webpack/JobInvocationDetail/JobInvocationActions.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/webpack/JobInvocationDetail/JobInvocationActions.js b/webpack/JobInvocationDetail/JobInvocationActions.js index 5b6ebb197..b70ee57ea 100644 --- a/webpack/JobInvocationDetail/JobInvocationActions.js +++ b/webpack/JobInvocationDetail/JobInvocationActions.js @@ -24,7 +24,7 @@ const fetchJobInvocation = (dispatch, url, params = {}) => { dispatch( get({ key: JOB_INVOCATION_KEY, - params: { include_hosts: false }, + params: { include_hosts: false, ...params }, url, handleSuccess: ({ data }) => { if (!isJobFinished(data)) { From b3201918d73c215ffbfc97abc1c9dece92a144ab Mon Sep 17 00:00:00 2001 From: Adam Ruzicka Date: Thu, 2 Jul 2026 15:54:33 +0200 Subject: [PATCH 11/27] Add tests for job invocation polling behaviour Covers: initial fetch params (include_permissions + include_hosts), poll scheduling while running, stopping on all terminal statuses (succeeded/failed/cancelled) and on error, stopJobInvocationPolling cancelling a pending timeout, and re-entrant getJobInvocation cancelling the previous poll. Co-Authored-By: Claude Sonnet 4.6 --- .../__tests__/JobInvocationPolling.test.js | 158 ++++++++++++++++++ 1 file changed, 158 insertions(+) create mode 100644 webpack/JobInvocationDetail/__tests__/JobInvocationPolling.test.js diff --git a/webpack/JobInvocationDetail/__tests__/JobInvocationPolling.test.js b/webpack/JobInvocationDetail/__tests__/JobInvocationPolling.test.js new file mode 100644 index 000000000..4a6e14ce2 --- /dev/null +++ b/webpack/JobInvocationDetail/__tests__/JobInvocationPolling.test.js @@ -0,0 +1,158 @@ +import configureMockStore from 'redux-mock-store'; +import thunk from 'redux-thunk'; +import * as api from 'foremanReact/redux/API'; +import { + getJobInvocation, + stopJobInvocationPolling, +} from '../JobInvocationActions'; +import { JOB_INVOCATION_KEY } from '../JobInvocationConstants'; + +jest.spyOn(api, 'get'); +jest.useFakeTimers(); + +const mockStore = configureMockStore([thunk]); + +const runningData = { status_label: 'running' }; +const succeededData = { status_label: 'succeeded' }; +const failedData = { status_label: 'failed' }; +const cancelledData = { status_label: 'cancelled' }; + +const setupGetMock = responseData => { + api.get.mockImplementation(({ handleSuccess, ...action }) => { + handleSuccess && handleSuccess({ data: responseData }); + return { type: 'get', ...action }; + }); +}; + +const url = '/api/job_invocations/1'; + +describe('job invocation polling', () => { + beforeEach(() => { + api.get.mockReset(); + stopJobInvocationPolling(); + jest.clearAllTimers(); + }); + + it('sends include_permissions and include_hosts on the initial fetch', () => { + setupGetMock(succeededData); + const store = mockStore({}); + + store.dispatch(getJobInvocation(url)); + + expect(api.get).toHaveBeenCalledTimes(1); + expect(api.get.mock.calls[0][0]).toMatchObject({ + key: JOB_INVOCATION_KEY, + url, + params: { include_hosts: false, include_permissions: true }, + }); + }); + + it('schedules the next poll when job is still running', () => { + setupGetMock(runningData); + const store = mockStore({}); + + store.dispatch(getJobInvocation(url)); + expect(api.get).toHaveBeenCalledTimes(1); + + jest.advanceTimersByTime(1000); + expect(api.get).toHaveBeenCalledTimes(2); + }); + + it('does not send include_permissions on subsequent poll requests', () => { + setupGetMock(runningData); + const store = mockStore({}); + + store.dispatch(getJobInvocation(url)); + jest.advanceTimersByTime(1000); + + expect(api.get).toHaveBeenCalledTimes(2); + expect(api.get.mock.calls[1][0].params).not.toHaveProperty( + 'include_permissions' + ); + expect(api.get.mock.calls[1][0].params).toMatchObject({ + include_hosts: false, + }); + }); + + it('stops polling when job succeeds', () => { + setupGetMock(succeededData); + const store = mockStore({}); + + store.dispatch(getJobInvocation(url)); + expect(api.get).toHaveBeenCalledTimes(1); + + jest.advanceTimersByTime(5000); + expect(api.get).toHaveBeenCalledTimes(1); + }); + + it('stops polling when job fails', () => { + setupGetMock(failedData); + const store = mockStore({}); + + store.dispatch(getJobInvocation(url)); + jest.advanceTimersByTime(5000); + + expect(api.get).toHaveBeenCalledTimes(1); + }); + + it('stops polling when job is cancelled', () => { + setupGetMock(cancelledData); + const store = mockStore({}); + + store.dispatch(getJobInvocation(url)); + jest.advanceTimersByTime(5000); + + expect(api.get).toHaveBeenCalledTimes(1); + }); + + it('stops polling on fetch error', () => { + api.get.mockImplementation(({ handleError, ...action }) => { + handleError && handleError(); + return { type: 'get', ...action }; + }); + const store = mockStore({}); + + store.dispatch(getJobInvocation(url)); + jest.advanceTimersByTime(5000); + + expect(api.get).toHaveBeenCalledTimes(1); + }); + + it('stopJobInvocationPolling cancels a pending timeout', () => { + setupGetMock(runningData); + const store = mockStore({}); + + store.dispatch(getJobInvocation(url)); + expect(api.get).toHaveBeenCalledTimes(1); + + stopJobInvocationPolling(); + jest.advanceTimersByTime(5000); + + expect(api.get).toHaveBeenCalledTimes(1); + }); + + it('getJobInvocation cancels any existing poll before starting a new one', () => { + setupGetMock(runningData); + const store = mockStore({}); + + store.dispatch(getJobInvocation(url)); + expect(api.get).toHaveBeenCalledTimes(1); + + // Second call before the timeout fires should cancel the pending poll + store.dispatch(getJobInvocation(url)); + jest.advanceTimersByTime(1000); + + // Only one more poll should fire (from the second invocation), not two + expect(api.get).toHaveBeenCalledTimes(3); + }); + + it('keeps polling as long as the job is running', () => { + setupGetMock(runningData); + const store = mockStore({}); + + store.dispatch(getJobInvocation(url)); + jest.advanceTimersByTime(3000); + + expect(api.get).toHaveBeenCalledTimes(4); + }); +}); From 7bdf7a02372727a625840b3587346b51d3ca9ecb Mon Sep 17 00:00:00 2001 From: Adam Ruzicka Date: Thu, 2 Jul 2026 16:03:58 +0200 Subject: [PATCH 12/27] Apply simplifications from code review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Inline scheduleNextPoll into handleSuccess in TemplateInvocation — removes a one-use helper and makes the self-scheduling pattern direct - Update prevFilter/prevStatusLabel refs only when the guard passes, not unconditionally before it - Remove dead UPDATE_JOB and GET_TASK constants Co-Authored-By: Claude Sonnet 4.6 --- webpack/JobInvocationDetail/JobInvocationConstants.js | 2 -- webpack/JobInvocationDetail/JobInvocationHostTable.js | 5 ++--- webpack/JobInvocationDetail/TemplateInvocation.js | 6 +----- 3 files changed, 3 insertions(+), 10 deletions(-) diff --git a/webpack/JobInvocationDetail/JobInvocationConstants.js b/webpack/JobInvocationDetail/JobInvocationConstants.js index 8eb70ffe0..afc0019fb 100644 --- a/webpack/JobInvocationDetail/JobInvocationConstants.js +++ b/webpack/JobInvocationDetail/JobInvocationConstants.js @@ -7,9 +7,7 @@ import JobStatusIcon from '../react_app/components/RecentJobsCard/JobStatusIcon' export const JOB_INVOCATION_KEY = 'JOB_INVOCATION_KEY'; export const CURRENT_PERMISSIONS = 'CURRENT_PERMISSIONS'; -export const UPDATE_JOB = 'UPDATE_JOB'; export const CANCEL_JOB = 'CANCEL_JOB'; -export const GET_TASK = 'GET_TASK'; export const GET_TEMPLATE_INVOCATIONS = 'GET_TEMPLATE_INVOCATIONS'; export const CHANGE_ENABLED_RECURRING_LOGIC = 'CHANGE_ENABLED_RECURRING_LOGIC'; export const CANCEL_RECURRING_LOGIC = 'CANCEL_RECURRING_LOGIC'; diff --git a/webpack/JobInvocationDetail/JobInvocationHostTable.js b/webpack/JobInvocationDetail/JobInvocationHostTable.js index bac06d4b8..f3a53d88d 100644 --- a/webpack/JobInvocationDetail/JobInvocationHostTable.js +++ b/webpack/JobInvocationDetail/JobInvocationHostTable.js @@ -255,10 +255,9 @@ const JobInvocationHostTable = ({ const filterChanged = initialFilter !== prevFilter.current; const statusChanged = statusLabel !== prevStatusLabel.current; - prevFilter.current = initialFilter; - prevStatusLabel.current = statusLabel; - if ((filterChanged || statusChanged) && initialFilter !== '') { + prevFilter.current = initialFilter; + prevStatusLabel.current = statusLabel; filterApiCall(); } }, [initialFilter, statusLabel, id, filterApiCall]); diff --git a/webpack/JobInvocationDetail/TemplateInvocation.js b/webpack/JobInvocationDetail/TemplateInvocation.js index 03575ccd6..9e428b729 100644 --- a/webpack/JobInvocationDetail/TemplateInvocation.js +++ b/webpack/JobInvocationDetail/TemplateInvocation.js @@ -77,10 +77,6 @@ export const TemplateInvocation = ({ const timeoutRef = useRef(null); useEffect(() => { - const scheduleNextPoll = () => { - timeoutRef.current = setTimeout(() => dispatchFetch(), 5000); - }; - const dispatchFetch = () => { dispatch( APIActions.get({ @@ -91,7 +87,7 @@ export const TemplateInvocation = ({ // eslint-disable-next-line camelcase const autoRefresh = data?.auto_refresh || false; if (!finished && autoRefresh) { - scheduleNextPoll(); + timeoutRef.current = setTimeout(dispatchFetch, 5000); } }, handleError: () => { From 26ca0320e55e61ae014f80049ae889846c7cb838 Mon Sep 17 00:00:00 2001 From: Adam Ruzicka Date: Thu, 2 Jul 2026 16:53:22 +0200 Subject: [PATCH 13/27] Fix TemplateInvocation poll race and drop dead jobId params MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add a cancelled flag inside the TemplateInvocation useEffect closure so that handleSuccess/handleError callbacks from in-flight fetches are ignored after the effect tears down. Without this, a handleError arriving after a fast isExpanded toggle could null out the new live timeoutRef, silently breaking polling. Also remove the now-unused jobId parameter from enableRecurringLogic and cancelRecurringLogic — it was only ever passed to the removed updateJob dispatch. Co-Authored-By: Claude Sonnet 4.6 --- webpack/JobInvocationDetail/JobInvocationActions.js | 8 ++------ .../JobInvocationDetail/JobInvocationToolbarButtons.js | 4 ++-- webpack/JobInvocationDetail/TemplateInvocation.js | 8 +++++--- 3 files changed, 9 insertions(+), 11 deletions(-) diff --git a/webpack/JobInvocationDetail/JobInvocationActions.js b/webpack/JobInvocationDetail/JobInvocationActions.js index b70ee57ea..2a921b597 100644 --- a/webpack/JobInvocationDetail/JobInvocationActions.js +++ b/webpack/JobInvocationDetail/JobInvocationActions.js @@ -96,11 +96,7 @@ export const cancelJob = (jobId, force) => dispatch => { ); }; -export const enableRecurringLogic = ( - recurrenceId, - enabled, - jobId -) => dispatch => { +export const enableRecurringLogic = (recurrenceId, enabled) => dispatch => { const successToast = () => enabled ? sprintf(__('Recurring logic %s disabled successfully.'), recurrenceId) @@ -133,7 +129,7 @@ export const enableRecurringLogic = ( ); }; -export const cancelRecurringLogic = (recurrenceId, jobId) => dispatch => { +export const cancelRecurringLogic = recurrenceId => dispatch => { const successToast = () => sprintf(__('Recurring logic %s cancelled successfully.'), recurrenceId); const errorToast = response => diff --git a/webpack/JobInvocationDetail/JobInvocationToolbarButtons.js b/webpack/JobInvocationDetail/JobInvocationToolbarButtons.js index 8c2d621fb..371c1e385 100644 --- a/webpack/JobInvocationDetail/JobInvocationToolbarButtons.js +++ b/webpack/JobInvocationDetail/JobInvocationToolbarButtons.js @@ -102,7 +102,7 @@ const JobInvocationToolbarButtons = ({ jobId, data }) => { ouiaId="change-enabled-recurring-dropdown-item" onClick={() => dispatch( - enableRecurringLogic(recurrence?.id, recurringEnabled, jobId) + enableRecurringLogic(recurrence?.id, recurringEnabled) ) } key="change-enabled-recurring" @@ -117,7 +117,7 @@ const JobInvocationToolbarButtons = ({ jobId, data }) => { , dispatch(cancelRecurringLogic(recurrence?.id, jobId))} + onClick={() => dispatch(cancelRecurringLogic(recurrence?.id))} key="cancel-recurring" component="button" isDisabled={ diff --git a/webpack/JobInvocationDetail/TemplateInvocation.js b/webpack/JobInvocationDetail/TemplateInvocation.js index 9e428b729..a3c500098 100644 --- a/webpack/JobInvocationDetail/TemplateInvocation.js +++ b/webpack/JobInvocationDetail/TemplateInvocation.js @@ -77,12 +77,14 @@ export const TemplateInvocation = ({ const timeoutRef = useRef(null); useEffect(() => { + let cancelled = false; const dispatchFetch = () => { dispatch( APIActions.get({ url: templateURL, key: `${GET_TEMPLATE_INVOCATION}_${hostID}`, handleSuccess: ({ data }) => { + if (cancelled) return; const finished = data?.finished ?? true; // eslint-disable-next-line camelcase const autoRefresh = data?.auto_refresh || false; @@ -91,6 +93,7 @@ export const TemplateInvocation = ({ } }, handleError: () => { + if (cancelled) return; timeoutRef.current = null; }, }) @@ -107,9 +110,8 @@ export const TemplateInvocation = ({ } return () => { - if (timeoutRef.current) { - clearTimeout(timeoutRef.current); - } + cancelled = true; + clearTimeout(timeoutRef.current); }; }, [isExpanded, dispatch, templateURL, hostID]); From 8d13947531241ce7586def023c003b8cd0e26b02 Mon Sep 17 00:00:00 2001 From: Adam Ruzicka Date: Thu, 2 Jul 2026 17:22:49 +0200 Subject: [PATCH 14/27] Make lints happy --- app/views/api/v2/job_invocations/main.json.rabl | 2 +- webpack/JobInvocationDetail/JobInvocationToolbarButtons.js | 4 +--- webpack/JobInvocationDetail/index.js | 1 - 3 files changed, 2 insertions(+), 5 deletions(-) diff --git a/app/views/api/v2/job_invocations/main.json.rabl b/app/views/api/v2/job_invocations/main.json.rabl index 56d8de551..10e965aca 100644 --- a/app/views/api/v2/job_invocations/main.json.rabl +++ b/app/views/api/v2/job_invocations/main.json.rabl @@ -34,7 +34,7 @@ end child :task do attributes :id, :state, :started_at - node(:cancellable) { |task| task.cancellable? } + node(:cancellable, &:cancellable?) end if @template_invocations diff --git a/webpack/JobInvocationDetail/JobInvocationToolbarButtons.js b/webpack/JobInvocationDetail/JobInvocationToolbarButtons.js index 371c1e385..b4ca22979 100644 --- a/webpack/JobInvocationDetail/JobInvocationToolbarButtons.js +++ b/webpack/JobInvocationDetail/JobInvocationToolbarButtons.js @@ -101,9 +101,7 @@ const JobInvocationToolbarButtons = ({ jobId, data }) => { - dispatch( - enableRecurringLogic(recurrence?.id, recurringEnabled) - ) + dispatch(enableRecurringLogic(recurrence?.id, recurringEnabled)) } key="change-enabled-recurring" component="button" diff --git a/webpack/JobInvocationDetail/index.js b/webpack/JobInvocationDetail/index.js index 60a22181a..a7529590a 100644 --- a/webpack/JobInvocationDetail/index.js +++ b/webpack/JobInvocationDetail/index.js @@ -26,7 +26,6 @@ import './JobInvocationDetail.scss'; import { CURRENT_PERMISSIONS, DATE_OPTIONS, - STATUS, STATUS_UPPERCASE, currentPermissionsUrl, } from './JobInvocationConstants'; From 7779ceba4ad3da1fe3862de341742141025c4f4b Mon Sep 17 00:00:00 2001 From: Adam Ruzicka Date: Fri, 3 Jul 2026 13:07:59 +0200 Subject: [PATCH 15/27] Poll host table while job is running Adds a self-scheduling setTimeout chain in JobInvocationHostTable that refreshes the host list every 5s while the job is not finished. Uses a ref for statusLabel so the poll callback doesn't need to be recreated on every job status update. filterApiCall cancels any pending poll before issuing a manual fetch to avoid races. Co-Authored-By: Claude Sonnet 4.6 --- .../JobInvocationHostTable.js | 62 +++++++++++++++++++ 1 file changed, 62 insertions(+) diff --git a/webpack/JobInvocationDetail/JobInvocationHostTable.js b/webpack/JobInvocationDetail/JobInvocationHostTable.js index f3a53d88d..71451143c 100644 --- a/webpack/JobInvocationDetail/JobInvocationHostTable.js +++ b/webpack/JobInvocationDetail/JobInvocationHostTable.js @@ -40,6 +40,7 @@ import DropdownFilter from './DropdownFilter'; import Columns, { JOB_INVOCATION_HOSTS, LIST_TEMPLATE_INVOCATIONS, + STATUS, STATUS_UPPERCASE, ALL_JOB_HOSTS, AWAITING_STATUS_FILTER, @@ -71,6 +72,12 @@ const JobInvocationHostTable = ({ const [expandedHost, setExpandedHost] = useState(new Set()); const prevStatusLabel = useRef(statusLabel); const prevFilter = useRef(initialFilter); + const pollTimeoutId = useRef(null); + const currentPollParams = useRef({}); + const statusLabelRef = useRef(statusLabel); + useEffect(() => { + statusLabelRef.current = statusLabel; + }, [statusLabel]); const [hostInvocationStates, setHostInvocationStates] = useState({}); @@ -154,6 +161,42 @@ const JobInvocationHostTable = ({ [initialFilter, urlSearchQuery] ); + const isJobFinished = () => { + const label = statusLabelRef.current; + return ( + label === STATUS.FAILED || + label === STATUS.SUCCEEDED || + label === STATUS.CANCELLED + ); + }; + + const pollHostTable = useCallback(() => { + dispatch( + APIActions.get({ + key: JOB_INVOCATION_HOSTS, + url: `/api/job_invocations/${id}/hosts`, + params: currentPollParams.current, + handleSuccess: data => { + const ids = data.data.results.map(i => i.id); + setApiResponse(data.data); + setAllHostsIds(ids); + setStatus(STATUS_UPPERCASE.RESOLVED); + if (!isJobFinished()) { + pollTimeoutId.current = setTimeout(pollHostTable, 5000); + } else { + pollTimeoutId.current = null; + } + }, + handleError: () => { + pollTimeoutId.current = null; + setStatus(STATUS_UPPERCASE.ERROR); + }, + errorToast: ({ response }) => + response?.data?.error?.full_messages?.[0] || response, + }) + ); + }, [dispatch, id]); + const handleResponse = useCallback((data, key) => { if (key === JOB_INVOCATION_HOSTS) { const ids = data.data.results.map(i => i.id); @@ -202,6 +245,12 @@ const JobInvocationHostTable = ({ finalParams.search = filterSearch; } + currentPollParams.current = finalParams; + if (pollTimeoutId.current !== null) { + clearTimeout(pollTimeoutId.current); + pollTimeoutId.current = null; + } + makeApiCall(finalParams, { key: JOB_INVOCATION_HOSTS }); const urlSearchParams = new URLSearchParams(window.location.search); @@ -262,6 +311,19 @@ const JobInvocationHostTable = ({ } }, [initialFilter, statusLabel, id, filterApiCall]); + useEffect(() => { + if (initialFilter !== '' && !isJobFinished()) { + pollTimeoutId.current = setTimeout(pollHostTable, 5000); + } + return () => { + if (pollTimeoutId.current !== null) { + clearTimeout(pollTimeoutId.current); + pollTimeoutId.current = null; + } + }; + // eslint-disable-next-line react-hooks/exhaustive-deps + }, [initialFilter, pollHostTable]); + const { updateSearchQuery: updateSearchQueryBulk, fetchBulkParams, From 1258caa890e73a2b04d388ef1d967af173aebcdb Mon Sep 17 00:00:00 2001 From: Adam Ruzicka Date: Fri, 3 Jul 2026 13:13:06 +0200 Subject: [PATCH 16/27] Refresh LIST_TEMPLATE_INVOCATIONS on page change and poll MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit RowActions reads permissions from selectTemplateInvocationList which is keyed by LIST_TEMPLATE_INVOCATIONS. It was only fetched once on mount (page 1 hosts), so switching pages left it stale — RowActions returned null for any host not in the initial result, hiding the actions column. Now filterApiCall and pollHostTable both refresh LIST_TEMPLATE_INVOCATIONS alongside the main hosts fetch, keeping it in sync with whatever page is currently visible. Co-Authored-By: Claude Sonnet 4.6 --- .../JobInvocationDetail/JobInvocationHostTable.js | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/webpack/JobInvocationDetail/JobInvocationHostTable.js b/webpack/JobInvocationDetail/JobInvocationHostTable.js index 71451143c..59b36e6d1 100644 --- a/webpack/JobInvocationDetail/JobInvocationHostTable.js +++ b/webpack/JobInvocationDetail/JobInvocationHostTable.js @@ -195,6 +195,13 @@ const JobInvocationHostTable = ({ response?.data?.error?.full_messages?.[0] || response, }) ); + dispatch( + APIActions.get({ + key: LIST_TEMPLATE_INVOCATIONS, + url: `/job_invocations/${id}/hosts`, + params: currentPollParams.current, + }) + ); }, [dispatch, id]); const handleResponse = useCallback((data, key) => { @@ -252,6 +259,10 @@ const JobInvocationHostTable = ({ } makeApiCall(finalParams, { key: JOB_INVOCATION_HOSTS }); + makeApiCall(finalParams, { + url: `/job_invocations/${id}/hosts`, + key: LIST_TEMPLATE_INVOCATIONS, + }); const urlSearchParams = new URLSearchParams(window.location.search); @@ -262,6 +273,7 @@ const JobInvocationHostTable = ({ history.push({ search: urlSearchParams.toString() }); }, [ + id, initialFilter, urlSearchQuery, defaultParams, From 41ed2d32113d7f33e83d4d931c0d39ae0a7577c0 Mon Sep 17 00:00:00 2001 From: Adam Ruzicka Date: Fri, 3 Jul 2026 13:29:59 +0200 Subject: [PATCH 17/27] Include task and permissions in API hosts response The RowActions component needs per-host task and permissions data to render the actions column. Previously this came from a separate non-API endpoint (list_jobs_hosts) fetched once on mount, causing the actions column to disappear on page switches since the stale data had no entries for hosts on pages 2+. Now set_statuses_and_smart_proxies builds @task_by_host and @permissions_by_host from the already-loaded template invocations and exposes them in the hosts.json.rabl response. RowActions reads directly from the paginated JOB_INVOCATION_HOSTS Redux key instead of a separate LIST_TEMPLATE_INVOCATIONS key. Drops ALL_JOB_HOSTS, LIST_TEMPLATE_INVOCATIONS, handleResponse, selectTemplateInvocationList, and the non-API /job_invocations/:id/hosts fetch entirely. Co-Authored-By: Claude Sonnet 4.6 --- .../api/v2/job_invocations_controller.rb | 13 +++++ .../api/v2/job_invocations/hosts.json.rabl | 8 +++ .../JobInvocationConstants.js | 3 -- .../JobInvocationHostTable.js | 52 +++++-------------- .../JobInvocationSelectors.js | 9 ++-- .../TemplateActionButtons.js | 4 +- 6 files changed, 40 insertions(+), 49 deletions(-) diff --git a/app/controllers/api/v2/job_invocations_controller.rb b/app/controllers/api/v2/job_invocations_controller.rb index 0bf640111..0113ab81e 100644 --- a/app/controllers/api/v2/job_invocations_controller.rb +++ b/app/controllers/api/v2/job_invocations_controller.rb @@ -329,6 +329,19 @@ def set_statuses_and_smart_proxies end @smart_proxy_id = template_invocations.to_h { |ti| [ti.host_id, ti.smart_proxy_id] } @smart_proxy_name = template_invocations.to_h { |ti| [ti.host_id, ti.smart_proxy_name] } + @task_by_host = template_invocations.to_h do |ti| + task = ti.run_host_job_task + [ti.host_id, task&.attributes&.merge(:cancellable => task.cancellable?)] + end + @permissions_by_host = hosts.to_h do |host| + template_invocation = template_invocations_by_host_id[host.id] + task = template_invocation.try(:run_host_job_task) + [host.id, { + :view_foreman_tasks => authorized_for(:permission => :view_foreman_tasks, :auth_object => task), + :cancel_job_invocations => authorized_for(:permission => :cancel_job_invocations, :auth_object => @job_invocation), + :execute_jobs => authorized_for(controller: :job_invocations, action: :create) && (!host.infrastructure_host? || User.current.can?(:execute_jobs_on_infrastructure_hosts)), + }] + end end end end diff --git a/app/views/api/v2/job_invocations/hosts.json.rabl b/app/views/api/v2/job_invocations/hosts.json.rabl index 7e627dfef..32214a51e 100644 --- a/app/views/api/v2/job_invocations/hosts.json.rabl +++ b/app/views/api/v2/job_invocations/hosts.json.rabl @@ -13,3 +13,11 @@ end node :smart_proxy_name do |host| @smart_proxy_name[host.id] end + +node :task do |host| + @task_by_host[host.id] +end + +node :permissions do |host| + @permissions_by_host[host.id] +end diff --git a/webpack/JobInvocationDetail/JobInvocationConstants.js b/webpack/JobInvocationDetail/JobInvocationConstants.js index afc0019fb..4ea808c59 100644 --- a/webpack/JobInvocationDetail/JobInvocationConstants.js +++ b/webpack/JobInvocationDetail/JobInvocationConstants.js @@ -16,7 +16,6 @@ export const GET_REPORT_TEMPLATE_INPUTS = 'GET_REPORT_TEMPLATE_INPUTS'; export const JOB_INVOCATION_HOSTS = 'JOB_INVOCATION_HOSTS'; export const GET_TEMPLATE_INVOCATION = 'GET_TEMPLATE_INVOCATION'; export const DIRECT_OPEN_HOST_LIMIT = 3; -export const ALL_JOB_HOSTS = 'ALL_JOB_HOSTS'; export const AWAITING_STATUS_FILTER = '(job_invocation.result = N/A)'; export const currentPermissionsUrl = foremanUrl( '/api/v2/permissions/current_permissions' @@ -24,8 +23,6 @@ export const currentPermissionsUrl = foremanUrl( export const showTemplateInvocationUrl = (hostID, jobID) => `/show_template_invocation_by_host/${hostID}/job_invocation/${jobID}`; -export const LIST_TEMPLATE_INVOCATIONS = 'LIST_TEMPLATE_INVOCATIONS'; - export const templateInvocationPageUrl = (hostID, jobID) => `/job_invocations_detail/${jobID}/host_invocation/${hostID}`; diff --git a/webpack/JobInvocationDetail/JobInvocationHostTable.js b/webpack/JobInvocationDetail/JobInvocationHostTable.js index 59b36e6d1..e3cf31d43 100644 --- a/webpack/JobInvocationDetail/JobInvocationHostTable.js +++ b/webpack/JobInvocationDetail/JobInvocationHostTable.js @@ -39,10 +39,8 @@ import { CheckboxesActions } from './CheckboxesActions'; import DropdownFilter from './DropdownFilter'; import Columns, { JOB_INVOCATION_HOSTS, - LIST_TEMPLATE_INVOCATIONS, STATUS, STATUS_UPPERCASE, - ALL_JOB_HOSTS, AWAITING_STATUS_FILTER, } from './JobInvocationConstants'; import { TemplateInvocation } from './TemplateInvocation'; @@ -195,41 +193,29 @@ const JobInvocationHostTable = ({ response?.data?.error?.full_messages?.[0] || response, }) ); - dispatch( - APIActions.get({ - key: LIST_TEMPLATE_INVOCATIONS, - url: `/job_invocations/${id}/hosts`, - params: currentPollParams.current, - }) - ); }, [dispatch, id]); - const handleResponse = useCallback((data, key) => { - if (key === JOB_INVOCATION_HOSTS) { - const ids = data.data.results.map(i => i.id); - - setApiResponse(data.data); - setAllHostsIds(ids); - setStatus(STATUS_UPPERCASE.RESOLVED); - } - }, []); - // Call hosts data with params const makeApiCall = useCallback( - (requestParams, callParams = {}) => { + requestParams => { dispatch( APIActions.get({ - key: callParams.key ?? ALL_JOB_HOSTS, - url: callParams.url ?? `/api/job_invocations/${id}/hosts`, + key: JOB_INVOCATION_HOSTS, + url: `/api/job_invocations/${id}/hosts`, params: requestParams, - handleSuccess: data => handleResponse(data, callParams.key), + handleSuccess: data => { + const ids = data.data.results.map(i => i.id); + setApiResponse(data.data); + setAllHostsIds(ids); + setStatus(STATUS_UPPERCASE.RESOLVED); + }, handleError: () => setStatus(STATUS_UPPERCASE.ERROR), errorToast: ({ response }) => response?.data?.error?.full_messages?.[0] || response, }) ); }, - [dispatch, id, handleResponse] + [dispatch, id] ); const filterApiCall = useCallback( @@ -258,11 +244,7 @@ const JobInvocationHostTable = ({ pollTimeoutId.current = null; } - makeApiCall(finalParams, { key: JOB_INVOCATION_HOSTS }); - makeApiCall(finalParams, { - url: `/job_invocations/${id}/hosts`, - key: LIST_TEMPLATE_INVOCATIONS, - }); + makeApiCall(finalParams); const urlSearchParams = new URLSearchParams(window.location.search); @@ -273,7 +255,6 @@ const JobInvocationHostTable = ({ history.push({ search: urlSearchParams.toString() }); }, [ - id, initialFilter, urlSearchQuery, defaultParams, @@ -296,21 +277,12 @@ const JobInvocationHostTable = ({ const initializedRef = useRef(false); useEffect(() => { if (!initializedRef.current) { - // Job Invo template load - makeApiCall( - {}, - { - url: `/job_invocations/${id}/hosts`, - key: LIST_TEMPLATE_INVOCATIONS, - } - ); - if (initialFilter === '') { onFilterUpdate('all_statuses'); } initializedRef.current = true; } - }, [makeApiCall, id, initialFilter, onFilterUpdate]); + }, [initialFilter, onFilterUpdate]); useEffect(() => { const filterChanged = initialFilter !== prevFilter.current; diff --git a/webpack/JobInvocationDetail/JobInvocationSelectors.js b/webpack/JobInvocationDetail/JobInvocationSelectors.js index 93adfa9ab..db2b8f259 100644 --- a/webpack/JobInvocationDetail/JobInvocationSelectors.js +++ b/webpack/JobInvocationDetail/JobInvocationSelectors.js @@ -6,8 +6,8 @@ import { import { STATUS as APIStatus } from 'foremanReact/constants'; import { JOB_INVOCATION_KEY, + JOB_INVOCATION_HOSTS, GET_TEMPLATE_INVOCATION, - LIST_TEMPLATE_INVOCATIONS, CURRENT_PERMISSIONS, } from './JobInvocationConstants'; @@ -23,9 +23,10 @@ export const selectTemplateInvocation = hostID => state => export const selectTemplateInvocationStatus = hostID => state => selectAPIStatus(state, `${GET_TEMPLATE_INVOCATION}_${hostID}`); -export const selectTemplateInvocationList = state => - selectAPIResponse(state, LIST_TEMPLATE_INVOCATIONS) - ?.template_invocations_task_by_hosts; +export const selectHostFromJobInvocationHosts = hostID => state => + selectAPIResponse(state, JOB_INVOCATION_HOSTS)?.results?.find( + h => h.id === hostID + ); export const selectCurrentPermisions = state => selectAPIResponse(state, CURRENT_PERMISSIONS); diff --git a/webpack/JobInvocationDetail/TemplateInvocationComponents/TemplateActionButtons.js b/webpack/JobInvocationDetail/TemplateInvocationComponents/TemplateActionButtons.js index f9242f646..2151a7d37 100644 --- a/webpack/JobInvocationDetail/TemplateInvocationComponents/TemplateActionButtons.js +++ b/webpack/JobInvocationDetail/TemplateInvocationComponents/TemplateActionButtons.js @@ -6,7 +6,7 @@ import { ActionsColumn } from '@patternfly/react-table'; import { APIActions } from 'foremanReact/redux/API'; import { addToast } from 'foremanReact/components/ToastsList'; import { translate as __ } from 'foremanReact/common/I18n'; -import { selectTemplateInvocationList } from '../JobInvocationSelectors'; +import { selectHostFromJobInvocationHosts } from '../JobInvocationSelectors'; import './index.scss'; const actions = ({ @@ -83,7 +83,7 @@ const actions = ({ export const RowActions = ({ hostID, jobID }) => { const dispatch = useDispatch(); - const response = useSelector(selectTemplateInvocationList)?.[hostID]; + const response = useSelector(selectHostFromJobInvocationHosts(hostID)); if (!response?.permissions) return null; const { task, permissions } = response; const { id: taskID, cancellable: taskCancellable } = task || {}; From cd207a6b8b60186a00b9540b93ed7894a77c0e78 Mon Sep 17 00:00:00 2001 From: Adam Ruzicka Date: Fri, 3 Jul 2026 13:32:25 +0200 Subject: [PATCH 18/27] =?UTF-8?q?Drop=20list=5Fjobs=5Fhosts=20=E2=80=94=20?= =?UTF-8?q?replaced=20by=20API=20hosts=20endpoint?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Task and permissions data is now included in /api/job_invocations/:id/hosts, so the separate non-API list_jobs_hosts action is no longer needed. Co-Authored-By: Claude Sonnet 4.6 --- app/controllers/job_invocations_controller.rb | 29 +------------------ config/routes.rb | 1 - lib/foreman_remote_execution/plugin.rb | 4 +-- 3 files changed, 3 insertions(+), 31 deletions(-) diff --git a/app/controllers/job_invocations_controller.rb b/app/controllers/job_invocations_controller.rb index 5ef6abe06..7f66d40f4 100644 --- a/app/controllers/job_invocations_controller.rb +++ b/app/controllers/job_invocations_controller.rb @@ -149,33 +149,6 @@ def preview_job_invocations_per_host render :json => {:job_invocations => job_invocations} end - def list_jobs_hosts - @job_invocation = resource_base.find(params[:id]) - hosts = @job_invocation.targeting.hosts.authorized(:view_hosts, Host) - hosts = hosts.search_for(params[:search]) - template_invocations_task_by_hosts = {} - hosts.each do |host| - template_invocation = @job_invocation.template_invocations.find { |template_inv| template_inv.host_id == host.id } - next unless template_invocation - template_invocation_task = template_invocation.run_host_job_task - template_invocations_task_by_hosts[host.id] = - { - :host_name => host.name, - :id => host.id, - :task => template_invocation_task.attributes.merge({cancellable: template_invocation_task.cancellable? }), - :permissions => { - :view_foreman_tasks => authorized_for(:permission => :view_foreman_tasks, :auth_object => template_invocation_task), - :cancel_job_invocations => authorized_for(:permission => :cancel_job_invocations, :auth_object => @job_invocation), - :execute_jobs => authorized_for(controller: :job_invocations, action: :create) && (!host.infrastructure_host? || User.current.can?(:execute_jobs_on_infrastructure_hosts)), - }, - } - end - - render json: { - :template_invocations_task_by_hosts => template_invocations_task_by_hosts, - } - end - private def action_permission @@ -186,7 +159,7 @@ def action_permission 'create' when 'cancel' 'cancel' - when 'chart', 'preview_job_invocations_per_host', 'list_jobs_hosts' + when 'chart', 'preview_job_invocations_per_host' 'view' else super diff --git a/config/routes.rb b/config/routes.rb index 5227bde52..558cf5882 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -35,7 +35,6 @@ get 'auto_complete_search' end member do - get 'hosts', to: 'job_invocations#list_jobs_hosts' post 'cancel' end end diff --git a/lib/foreman_remote_execution/plugin.rb b/lib/foreman_remote_execution/plugin.rb index 3dace7a7f..7ef0c3364 100644 --- a/lib/foreman_remote_execution/plugin.rb +++ b/lib/foreman_remote_execution/plugin.rb @@ -134,9 +134,9 @@ permission :lock_job_templates, { :job_templates => [:lock, :unlock] }, :resource_type => 'JobTemplate' permission :create_job_invocations, { :job_invocations => [:new, :create, :legacy_create, :refresh, :rerun, :preview_hosts], 'api/v2/job_invocations' => [:create, :rerun] }, :resource_type => 'JobInvocation' - permission :view_job_invocations, { :job_invocations => [:index, :chart, :show, :auto_complete_search, :preview_job_invocations_per_host, :list_jobs_hosts], :template_invocations => [:show, :show_template_invocation_by_host], + permission :view_job_invocations, { :job_invocations => [:index, :chart, :show, :auto_complete_search, :preview_job_invocations_per_host], :template_invocations => [:show, :show_template_invocation_by_host], 'api/v2/job_invocations' => [:index, :show, :output, :raw_output, :outputs, :hosts] }, :resource_type => 'JobInvocation' - permission :view_template_invocations, { :template_invocations => [:show, :template_invocation_preview, :show_template_invocation_by_host], :job_invocations => [:list_jobs_hosts], + permission :view_template_invocations, { :template_invocations => [:show, :template_invocation_preview, :show_template_invocation_by_host], 'api/v2/template_invocations' => [:template_invocations], :ui_job_wizard => [:job_invocation] }, :resource_type => 'TemplateInvocation' permission :create_template_invocations, {}, :resource_type => 'TemplateInvocation' permission :execute_jobs_on_infrastructure_hosts, {}, :resource_type => 'JobInvocation' From 9bf9f05937169a5f60ae9855d485ca1b3519e0f8 Mon Sep 17 00:00:00 2001 From: Adam Ruzicka Date: Fri, 3 Jul 2026 14:05:26 +0200 Subject: [PATCH 19/27] Gate task and permissions in hosts response behind include_permissions param MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The per-host authorization checks are expensive — skip them unless the caller explicitly requests them with include_permissions=true. Co-Authored-By: Claude Sonnet 4.6 --- .../api/v2/job_invocations_controller.rb | 2 ++ .../api/v2/job_invocations/hosts.json.rabl | 4 ++-- .../JobInvocationActions.js | 2 +- .../JobInvocationHostTable.js | 22 +++++++++---------- .../__tests__/JobInvocationPolling.test.js | 8 +++---- 5 files changed, 20 insertions(+), 18 deletions(-) diff --git a/app/controllers/api/v2/job_invocations_controller.rb b/app/controllers/api/v2/job_invocations_controller.rb index 0113ab81e..86f9c758e 100644 --- a/app/controllers/api/v2/job_invocations_controller.rb +++ b/app/controllers/api/v2/job_invocations_controller.rb @@ -118,6 +118,7 @@ def output add_scoped_search_description_for(JobInvocation) param :id, :identifier, :required => true def hosts + @include_permissions = Foreman::Cast.to_bool(params[:include_permissions]) set_hosts_and_template_invocations @total = @hosts.size @hosts = @hosts.search_for(params[:search], :order => params[:order]).paginate(:page => params[:page], :per_page => params[:per_page]) @@ -329,6 +330,7 @@ def set_statuses_and_smart_proxies end @smart_proxy_id = template_invocations.to_h { |ti| [ti.host_id, ti.smart_proxy_id] } @smart_proxy_name = template_invocations.to_h { |ti| [ti.host_id, ti.smart_proxy_name] } + return unless @include_permissions @task_by_host = template_invocations.to_h do |ti| task = ti.run_host_job_task [ti.host_id, task&.attributes&.merge(:cancellable => task.cancellable?)] diff --git a/app/views/api/v2/job_invocations/hosts.json.rabl b/app/views/api/v2/job_invocations/hosts.json.rabl index 32214a51e..94a77c30f 100644 --- a/app/views/api/v2/job_invocations/hosts.json.rabl +++ b/app/views/api/v2/job_invocations/hosts.json.rabl @@ -14,10 +14,10 @@ node :smart_proxy_name do |host| @smart_proxy_name[host.id] end -node :task do |host| +node(:task, :if => ->(_) { @task_by_host }) do |host| @task_by_host[host.id] end -node :permissions do |host| +node(:permissions, :if => ->(_) { @permissions_by_host }) do |host| @permissions_by_host[host.id] end diff --git a/webpack/JobInvocationDetail/JobInvocationActions.js b/webpack/JobInvocationDetail/JobInvocationActions.js index 2a921b597..1a540ddb8 100644 --- a/webpack/JobInvocationDetail/JobInvocationActions.js +++ b/webpack/JobInvocationDetail/JobInvocationActions.js @@ -17,7 +17,7 @@ const isJobFinished = ({ status_label: statusLabel }) => statusLabel === STATUS.CANCELLED; const scheduleNextPoll = (dispatch, url) => { - pollTimeoutId = setTimeout(() => fetchJobInvocation(dispatch, url), 1000); + pollTimeoutId = setTimeout(() => fetchJobInvocation(dispatch, url), 5000); }; const fetchJobInvocation = (dispatch, url, params = {}) => { diff --git a/webpack/JobInvocationDetail/JobInvocationHostTable.js b/webpack/JobInvocationDetail/JobInvocationHostTable.js index e3cf31d43..db18a1877 100644 --- a/webpack/JobInvocationDetail/JobInvocationHostTable.js +++ b/webpack/JobInvocationDetail/JobInvocationHostTable.js @@ -173,7 +173,7 @@ const JobInvocationHostTable = ({ APIActions.get({ key: JOB_INVOCATION_HOSTS, url: `/api/job_invocations/${id}/hosts`, - params: currentPollParams.current, + params: { include_permissions: true, ...currentPollParams.current }, handleSuccess: data => { const ids = data.data.results.map(i => i.id); setApiResponse(data.data); @@ -202,12 +202,15 @@ const JobInvocationHostTable = ({ APIActions.get({ key: JOB_INVOCATION_HOSTS, url: `/api/job_invocations/${id}/hosts`, - params: requestParams, + params: { include_permissions: true, ...requestParams }, handleSuccess: data => { const ids = data.data.results.map(i => i.id); setApiResponse(data.data); setAllHostsIds(ids); setStatus(STATUS_UPPERCASE.RESOLVED); + if (!isJobFinished()) { + pollTimeoutId.current = setTimeout(pollHostTable, 5000); + } }, handleError: () => setStatus(STATUS_UPPERCASE.ERROR), errorToast: ({ response }) => @@ -215,7 +218,7 @@ const JobInvocationHostTable = ({ }) ); }, - [dispatch, id] + [dispatch, id, pollHostTable] ); const filterApiCall = useCallback( @@ -295,18 +298,15 @@ const JobInvocationHostTable = ({ } }, [initialFilter, statusLabel, id, filterApiCall]); - useEffect(() => { - if (initialFilter !== '' && !isJobFinished()) { - pollTimeoutId.current = setTimeout(pollHostTable, 5000); - } - return () => { + useEffect( + () => () => { if (pollTimeoutId.current !== null) { clearTimeout(pollTimeoutId.current); pollTimeoutId.current = null; } - }; - // eslint-disable-next-line react-hooks/exhaustive-deps - }, [initialFilter, pollHostTable]); + }, + [] + ); const { updateSearchQuery: updateSearchQueryBulk, diff --git a/webpack/JobInvocationDetail/__tests__/JobInvocationPolling.test.js b/webpack/JobInvocationDetail/__tests__/JobInvocationPolling.test.js index 4a6e14ce2..32dc7ae5d 100644 --- a/webpack/JobInvocationDetail/__tests__/JobInvocationPolling.test.js +++ b/webpack/JobInvocationDetail/__tests__/JobInvocationPolling.test.js @@ -54,7 +54,7 @@ describe('job invocation polling', () => { store.dispatch(getJobInvocation(url)); expect(api.get).toHaveBeenCalledTimes(1); - jest.advanceTimersByTime(1000); + jest.advanceTimersByTime(5000); expect(api.get).toHaveBeenCalledTimes(2); }); @@ -63,7 +63,7 @@ describe('job invocation polling', () => { const store = mockStore({}); store.dispatch(getJobInvocation(url)); - jest.advanceTimersByTime(1000); + jest.advanceTimersByTime(5000); expect(api.get).toHaveBeenCalledTimes(2); expect(api.get.mock.calls[1][0].params).not.toHaveProperty( @@ -140,7 +140,7 @@ describe('job invocation polling', () => { // Second call before the timeout fires should cancel the pending poll store.dispatch(getJobInvocation(url)); - jest.advanceTimersByTime(1000); + jest.advanceTimersByTime(5000); // Only one more poll should fire (from the second invocation), not two expect(api.get).toHaveBeenCalledTimes(3); @@ -151,7 +151,7 @@ describe('job invocation polling', () => { const store = mockStore({}); store.dispatch(getJobInvocation(url)); - jest.advanceTimersByTime(3000); + jest.advanceTimersByTime(15000); expect(api.get).toHaveBeenCalledTimes(4); }); From c5963e434109e4a457e3d334b0495ab6f2ff2a38 Mon Sep 17 00:00:00 2001 From: Adam Ruzicka Date: Fri, 3 Jul 2026 14:59:27 +0200 Subject: [PATCH 20/27] Pass jobFinished as prop to JobInvocationHostTable MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The parent already knows whether the job is done — move isJobFinished out of the table and into JobInvocationActions where the STATUS constants live, export it, and let index.js pass a plain boolean prop down. The table no longer needs to know what constitutes "finished"; it just checks the prop via a ref (same pattern, cleaner responsibility split). Co-Authored-By: Claude Sonnet 4.6 --- .../JobInvocationActions.js | 4 +-- .../JobInvocationHostTable.js | 33 +++++++------------ webpack/JobInvocationDetail/index.js | 3 +- 3 files changed, 16 insertions(+), 24 deletions(-) diff --git a/webpack/JobInvocationDetail/JobInvocationActions.js b/webpack/JobInvocationDetail/JobInvocationActions.js index 1a540ddb8..1bb88e21f 100644 --- a/webpack/JobInvocationDetail/JobInvocationActions.js +++ b/webpack/JobInvocationDetail/JobInvocationActions.js @@ -11,7 +11,7 @@ import { let pollTimeoutId = null; -const isJobFinished = ({ status_label: statusLabel }) => +export const isJobFinished = statusLabel => statusLabel === STATUS.FAILED || statusLabel === STATUS.SUCCEEDED || statusLabel === STATUS.CANCELLED; @@ -27,7 +27,7 @@ const fetchJobInvocation = (dispatch, url, params = {}) => { params: { include_hosts: false, ...params }, url, handleSuccess: ({ data }) => { - if (!isJobFinished(data)) { + if (!isJobFinished(data.status_label)) { scheduleNextPoll(dispatch, url); } else { pollTimeoutId = null; diff --git a/webpack/JobInvocationDetail/JobInvocationHostTable.js b/webpack/JobInvocationDetail/JobInvocationHostTable.js index db18a1877..542396f5c 100644 --- a/webpack/JobInvocationDetail/JobInvocationHostTable.js +++ b/webpack/JobInvocationDetail/JobInvocationHostTable.js @@ -39,7 +39,6 @@ import { CheckboxesActions } from './CheckboxesActions'; import DropdownFilter from './DropdownFilter'; import Columns, { JOB_INVOCATION_HOSTS, - STATUS, STATUS_UPPERCASE, AWAITING_STATUS_FILTER, } from './JobInvocationConstants'; @@ -50,8 +49,8 @@ import { PopupAlert } from './OpenAllInvocationsModal'; const JobInvocationHostTable = ({ id, initialFilter, + jobFinished, onFilterUpdate, - statusLabel, targeting, }) => { const columns = Columns(); @@ -68,14 +67,14 @@ const JobInvocationHostTable = ({ // Expansive items const [expandedHost, setExpandedHost] = useState(new Set()); - const prevStatusLabel = useRef(statusLabel); + const prevJobFinished = useRef(jobFinished); const prevFilter = useRef(initialFilter); const pollTimeoutId = useRef(null); const currentPollParams = useRef({}); - const statusLabelRef = useRef(statusLabel); + const jobFinishedRef = useRef(jobFinished); useEffect(() => { - statusLabelRef.current = statusLabel; - }, [statusLabel]); + jobFinishedRef.current = jobFinished; + }, [jobFinished]); const [hostInvocationStates, setHostInvocationStates] = useState({}); @@ -159,14 +158,6 @@ const JobInvocationHostTable = ({ [initialFilter, urlSearchQuery] ); - const isJobFinished = () => { - const label = statusLabelRef.current; - return ( - label === STATUS.FAILED || - label === STATUS.SUCCEEDED || - label === STATUS.CANCELLED - ); - }; const pollHostTable = useCallback(() => { dispatch( @@ -179,7 +170,7 @@ const JobInvocationHostTable = ({ setApiResponse(data.data); setAllHostsIds(ids); setStatus(STATUS_UPPERCASE.RESOLVED); - if (!isJobFinished()) { + if (!jobFinishedRef.current) { pollTimeoutId.current = setTimeout(pollHostTable, 5000); } else { pollTimeoutId.current = null; @@ -208,7 +199,7 @@ const JobInvocationHostTable = ({ setApiResponse(data.data); setAllHostsIds(ids); setStatus(STATUS_UPPERCASE.RESOLVED); - if (!isJobFinished()) { + if (!jobFinishedRef.current) { pollTimeoutId.current = setTimeout(pollHostTable, 5000); } }, @@ -289,14 +280,14 @@ const JobInvocationHostTable = ({ useEffect(() => { const filterChanged = initialFilter !== prevFilter.current; - const statusChanged = statusLabel !== prevStatusLabel.current; + const statusChanged = jobFinished !== prevJobFinished.current; if ((filterChanged || statusChanged) && initialFilter !== '') { prevFilter.current = initialFilter; - prevStatusLabel.current = statusLabel; + prevJobFinished.current = jobFinished; filterApiCall(); } - }, [initialFilter, statusLabel, id, filterApiCall]); + }, [initialFilter, jobFinished, id, filterApiCall]); useEffect( () => () => { @@ -589,13 +580,13 @@ JobInvocationHostTable.propTypes = { id: PropTypes.string.isRequired, targeting: PropTypes.object.isRequired, initialFilter: PropTypes.string.isRequired, - statusLabel: PropTypes.string, + jobFinished: PropTypes.bool, onFilterUpdate: PropTypes.func, }; JobInvocationHostTable.defaultProps = { onFilterUpdate: () => {}, - statusLabel: undefined, + jobFinished: false, }; export default JobInvocationHostTable; diff --git a/webpack/JobInvocationDetail/index.js b/webpack/JobInvocationDetail/index.js index a7529590a..7222e90fb 100644 --- a/webpack/JobInvocationDetail/index.js +++ b/webpack/JobInvocationDetail/index.js @@ -20,6 +20,7 @@ import JobInvocationSystemStatusChart from './JobInvocationSystemStatusChart'; import JobInvocationToolbarButtons from './JobInvocationToolbarButtons'; import { getJobInvocation, + isJobFinished, stopJobInvocationPolling, } from './JobInvocationActions'; import './JobInvocationDetail.scss'; @@ -169,7 +170,7 @@ const JobInvocationDetailPage = ({ id={id} targeting={targeting} initialFilter={selectedFilter} - statusLabel={statusLabel} + jobFinished={isJobFinished(statusLabel)} onFilterUpdate={handleFilterChange} /> From f52f7bb93ff8afe2d9a1edf7bf6e3571f22156c1 Mon Sep 17 00:00:00 2001 From: Adam Ruzicka Date: Fri, 3 Jul 2026 15:04:01 +0200 Subject: [PATCH 21/27] Simplify polling helpers in actions and host table Inline scheduleNextPoll (single-line wrapper with no benefit) and extract updateHostsState to deduplicate the repeated state-update block shared between pollHostTable and makeApiCall. Co-Authored-By: Claude Sonnet 4.6 --- .../JobInvocationActions.js | 6 +----- .../JobInvocationHostTable.js | 21 ++++++++++--------- 2 files changed, 12 insertions(+), 15 deletions(-) diff --git a/webpack/JobInvocationDetail/JobInvocationActions.js b/webpack/JobInvocationDetail/JobInvocationActions.js index 1bb88e21f..ed7b5230a 100644 --- a/webpack/JobInvocationDetail/JobInvocationActions.js +++ b/webpack/JobInvocationDetail/JobInvocationActions.js @@ -16,10 +16,6 @@ export const isJobFinished = statusLabel => statusLabel === STATUS.SUCCEEDED || statusLabel === STATUS.CANCELLED; -const scheduleNextPoll = (dispatch, url) => { - pollTimeoutId = setTimeout(() => fetchJobInvocation(dispatch, url), 5000); -}; - const fetchJobInvocation = (dispatch, url, params = {}) => { dispatch( get({ @@ -28,7 +24,7 @@ const fetchJobInvocation = (dispatch, url, params = {}) => { url, handleSuccess: ({ data }) => { if (!isJobFinished(data.status_label)) { - scheduleNextPoll(dispatch, url); + pollTimeoutId = setTimeout(() => fetchJobInvocation(dispatch, url), 5000); } else { pollTimeoutId = null; } diff --git a/webpack/JobInvocationDetail/JobInvocationHostTable.js b/webpack/JobInvocationDetail/JobInvocationHostTable.js index 542396f5c..5aa04df14 100644 --- a/webpack/JobInvocationDetail/JobInvocationHostTable.js +++ b/webpack/JobInvocationDetail/JobInvocationHostTable.js @@ -159,6 +159,13 @@ const JobInvocationHostTable = ({ ); + const updateHostsState = useCallback(data => { + const ids = data.data.results.map(i => i.id); + setApiResponse(data.data); + setAllHostsIds(ids); + setStatus(STATUS_UPPERCASE.RESOLVED); + }, []); + const pollHostTable = useCallback(() => { dispatch( APIActions.get({ @@ -166,10 +173,7 @@ const JobInvocationHostTable = ({ url: `/api/job_invocations/${id}/hosts`, params: { include_permissions: true, ...currentPollParams.current }, handleSuccess: data => { - const ids = data.data.results.map(i => i.id); - setApiResponse(data.data); - setAllHostsIds(ids); - setStatus(STATUS_UPPERCASE.RESOLVED); + updateHostsState(data); if (!jobFinishedRef.current) { pollTimeoutId.current = setTimeout(pollHostTable, 5000); } else { @@ -184,7 +188,7 @@ const JobInvocationHostTable = ({ response?.data?.error?.full_messages?.[0] || response, }) ); - }, [dispatch, id]); + }, [dispatch, id, updateHostsState]); // Call hosts data with params const makeApiCall = useCallback( @@ -195,10 +199,7 @@ const JobInvocationHostTable = ({ url: `/api/job_invocations/${id}/hosts`, params: { include_permissions: true, ...requestParams }, handleSuccess: data => { - const ids = data.data.results.map(i => i.id); - setApiResponse(data.data); - setAllHostsIds(ids); - setStatus(STATUS_UPPERCASE.RESOLVED); + updateHostsState(data); if (!jobFinishedRef.current) { pollTimeoutId.current = setTimeout(pollHostTable, 5000); } @@ -209,7 +210,7 @@ const JobInvocationHostTable = ({ }) ); }, - [dispatch, id, pollHostTable] + [dispatch, id, pollHostTable, updateHostsState] ); const filterApiCall = useCallback( From cb43e25afc51a82b3bcc3d9fac41a2b18af0916f Mon Sep 17 00:00:00 2001 From: Adam Ruzicka Date: Fri, 3 Jul 2026 15:21:17 +0200 Subject: [PATCH 22/27] Fix polling cleanup inconsistencies and error toast fallbacks Co-Authored-By: Claude Opus 4.6 --- .../JobInvocationActions.js | 6 ++-- .../JobInvocationHostTable.js | 35 +++++++++---------- .../JobInvocationDetail/TemplateInvocation.js | 2 +- 3 files changed, 19 insertions(+), 24 deletions(-) diff --git a/webpack/JobInvocationDetail/JobInvocationActions.js b/webpack/JobInvocationDetail/JobInvocationActions.js index ed7b5230a..c69aef1da 100644 --- a/webpack/JobInvocationDetail/JobInvocationActions.js +++ b/webpack/JobInvocationDetail/JobInvocationActions.js @@ -49,10 +49,8 @@ export const getJobInvocation = url => dispatch => { }; export const stopJobInvocationPolling = () => { - if (pollTimeoutId !== null) { - clearTimeout(pollTimeoutId); - pollTimeoutId = null; - } + clearTimeout(pollTimeoutId); + pollTimeoutId = null; }; export const cancelJob = (jobId, force) => dispatch => { diff --git a/webpack/JobInvocationDetail/JobInvocationHostTable.js b/webpack/JobInvocationDetail/JobInvocationHostTable.js index 5aa04df14..f1f19c85f 100644 --- a/webpack/JobInvocationDetail/JobInvocationHostTable.js +++ b/webpack/JobInvocationDetail/JobInvocationHostTable.js @@ -185,7 +185,9 @@ const JobInvocationHostTable = ({ setStatus(STATUS_UPPERCASE.ERROR); }, errorToast: ({ response }) => - response?.data?.error?.full_messages?.[0] || response, + response?.data?.error?.full_messages?.[0] || + response?.data?.error?.message || + 'Error', }) ); }, [dispatch, id, updateHostsState]); @@ -202,11 +204,18 @@ const JobInvocationHostTable = ({ updateHostsState(data); if (!jobFinishedRef.current) { pollTimeoutId.current = setTimeout(pollHostTable, 5000); + } else { + pollTimeoutId.current = null; } }, - handleError: () => setStatus(STATUS_UPPERCASE.ERROR), + handleError: () => { + pollTimeoutId.current = null; + setStatus(STATUS_UPPERCASE.ERROR); + }, errorToast: ({ response }) => - response?.data?.error?.full_messages?.[0] || response, + response?.data?.error?.full_messages?.[0] || + response?.data?.error?.message || + 'Error', }) ); }, @@ -234,10 +243,8 @@ const JobInvocationHostTable = ({ } currentPollParams.current = finalParams; - if (pollTimeoutId.current !== null) { - clearTimeout(pollTimeoutId.current); - pollTimeoutId.current = null; - } + clearTimeout(pollTimeoutId.current); + pollTimeoutId.current = null; makeApiCall(finalParams); @@ -259,13 +266,6 @@ const JobInvocationHostTable = ({ ] ); - // Filter change - const handleFilterChange = useCallback( - newFilter => { - onFilterUpdate(newFilter); - }, - [onFilterUpdate] - ); // Effects // run after mount @@ -292,10 +292,7 @@ const JobInvocationHostTable = ({ useEffect( () => () => { - if (pollTimeoutId.current !== null) { - clearTimeout(pollTimeoutId.current); - pollTimeoutId.current = null; - } + clearTimeout(pollTimeoutId.current); }, [] ); @@ -443,7 +440,7 @@ const JobInvocationHostTable = ({ , 0 ? fetchBulkParams() : null} diff --git a/webpack/JobInvocationDetail/TemplateInvocation.js b/webpack/JobInvocationDetail/TemplateInvocation.js index a3c500098..8f6ba51c5 100644 --- a/webpack/JobInvocationDetail/TemplateInvocation.js +++ b/webpack/JobInvocationDetail/TemplateInvocation.js @@ -85,7 +85,7 @@ export const TemplateInvocation = ({ key: `${GET_TEMPLATE_INVOCATION}_${hostID}`, handleSuccess: ({ data }) => { if (cancelled) return; - const finished = data?.finished ?? true; + const finished = data?.finished ?? false; // eslint-disable-next-line camelcase const autoRefresh = data?.auto_refresh || false; if (!finished && autoRefresh) { From f46dde28ccb3af95747d5b705ca7cb66e5b1f73a Mon Sep 17 00:00:00 2001 From: Adam Ruzicka Date: Fri, 3 Jul 2026 15:47:46 +0200 Subject: [PATCH 23/27] Make lints happy --- webpack/JobInvocationDetail/JobInvocationActions.js | 5 ++++- webpack/JobInvocationDetail/JobInvocationHostTable.js | 2 -- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/webpack/JobInvocationDetail/JobInvocationActions.js b/webpack/JobInvocationDetail/JobInvocationActions.js index c69aef1da..665eea1f0 100644 --- a/webpack/JobInvocationDetail/JobInvocationActions.js +++ b/webpack/JobInvocationDetail/JobInvocationActions.js @@ -24,7 +24,10 @@ const fetchJobInvocation = (dispatch, url, params = {}) => { url, handleSuccess: ({ data }) => { if (!isJobFinished(data.status_label)) { - pollTimeoutId = setTimeout(() => fetchJobInvocation(dispatch, url), 5000); + pollTimeoutId = setTimeout( + () => fetchJobInvocation(dispatch, url), + 5000 + ); } else { pollTimeoutId = null; } diff --git a/webpack/JobInvocationDetail/JobInvocationHostTable.js b/webpack/JobInvocationDetail/JobInvocationHostTable.js index f1f19c85f..7d5157532 100644 --- a/webpack/JobInvocationDetail/JobInvocationHostTable.js +++ b/webpack/JobInvocationDetail/JobInvocationHostTable.js @@ -158,7 +158,6 @@ const JobInvocationHostTable = ({ [initialFilter, urlSearchQuery] ); - const updateHostsState = useCallback(data => { const ids = data.data.results.map(i => i.id); setApiResponse(data.data); @@ -266,7 +265,6 @@ const JobInvocationHostTable = ({ ] ); - // Effects // run after mount const initializedRef = useRef(false); From 9677991dc770f4f8c664d012973c04b9ad913607 Mon Sep 17 00:00:00 2001 From: Adam Ruzicka Date: Fri, 3 Jul 2026 16:24:42 +0200 Subject: [PATCH 24/27] Use try(:cancellable?) to handle base ForemanTasks::Task in tests cancellable? is only defined on DynflowTask; the :some_task factory uses the base class, causing test failures. Co-Authored-By: Claude Sonnet 4.6 --- app/controllers/api/v2/job_invocations_controller.rb | 2 +- app/views/api/v2/job_invocations/main.json.rabl | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/controllers/api/v2/job_invocations_controller.rb b/app/controllers/api/v2/job_invocations_controller.rb index 86f9c758e..5b7f7d91e 100644 --- a/app/controllers/api/v2/job_invocations_controller.rb +++ b/app/controllers/api/v2/job_invocations_controller.rb @@ -333,7 +333,7 @@ def set_statuses_and_smart_proxies return unless @include_permissions @task_by_host = template_invocations.to_h do |ti| task = ti.run_host_job_task - [ti.host_id, task&.attributes&.merge(:cancellable => task.cancellable?)] + [ti.host_id, task&.attributes&.merge(:cancellable => task.try(:cancellable?))] end @permissions_by_host = hosts.to_h do |host| template_invocation = template_invocations_by_host_id[host.id] diff --git a/app/views/api/v2/job_invocations/main.json.rabl b/app/views/api/v2/job_invocations/main.json.rabl index 10e965aca..18cd9b31a 100644 --- a/app/views/api/v2/job_invocations/main.json.rabl +++ b/app/views/api/v2/job_invocations/main.json.rabl @@ -34,7 +34,7 @@ end child :task do attributes :id, :state, :started_at - node(:cancellable, &:cancellable?) + node(:cancellable) { |task| task.try(:cancellable?) } end if @template_invocations From 2421ab32d6cf8d1cf84c3467d8831156eb9848cd Mon Sep 17 00:00:00 2001 From: Adam Ruzicka Date: Fri, 3 Jul 2026 17:17:03 +0200 Subject: [PATCH 25/27] Trim task data in hosts response to id and cancellable The frontend only needs task id (for links) and cancellable (to gate cancel actions). Shape the response in the rabl template rather than serializing all task attributes. Co-Authored-By: Claude Sonnet 4.6 --- app/controllers/api/v2/job_invocations_controller.rb | 2 +- app/views/api/v2/job_invocations/hosts.json.rabl | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/app/controllers/api/v2/job_invocations_controller.rb b/app/controllers/api/v2/job_invocations_controller.rb index 5b7f7d91e..6c090ee65 100644 --- a/app/controllers/api/v2/job_invocations_controller.rb +++ b/app/controllers/api/v2/job_invocations_controller.rb @@ -333,7 +333,7 @@ def set_statuses_and_smart_proxies return unless @include_permissions @task_by_host = template_invocations.to_h do |ti| task = ti.run_host_job_task - [ti.host_id, task&.attributes&.merge(:cancellable => task.try(:cancellable?))] + [ti.host_id, task] end @permissions_by_host = hosts.to_h do |host| template_invocation = template_invocations_by_host_id[host.id] diff --git a/app/views/api/v2/job_invocations/hosts.json.rabl b/app/views/api/v2/job_invocations/hosts.json.rabl index 94a77c30f..e07a9bbde 100644 --- a/app/views/api/v2/job_invocations/hosts.json.rabl +++ b/app/views/api/v2/job_invocations/hosts.json.rabl @@ -15,7 +15,8 @@ node :smart_proxy_name do |host| end node(:task, :if => ->(_) { @task_by_host }) do |host| - @task_by_host[host.id] + task = @task_by_host[host.id] + task && { :id => task.id, :cancellable => task.try(:cancellable?) } end node(:permissions, :if => ->(_) { @permissions_by_host }) do |host| From 96e0aae4f80155cc390518cdec5763bae5844947 Mon Sep 17 00:00:00 2001 From: Adam Ruzicka Date: Tue, 7 Jul 2026 13:36:12 +0200 Subject: [PATCH 26/27] Unify pollHostTable into makeApiCall pollHostTable was a no-args wrapper that always called makeApiCall with currentPollParams.current, so it can be replaced by an inline lambda. Co-Authored-By: Claude Sonnet 4.6 --- .../JobInvocationHostTable.js | 33 +++---------------- 1 file changed, 5 insertions(+), 28 deletions(-) diff --git a/webpack/JobInvocationDetail/JobInvocationHostTable.js b/webpack/JobInvocationDetail/JobInvocationHostTable.js index 7d5157532..87b8293a2 100644 --- a/webpack/JobInvocationDetail/JobInvocationHostTable.js +++ b/webpack/JobInvocationDetail/JobInvocationHostTable.js @@ -165,32 +165,6 @@ const JobInvocationHostTable = ({ setStatus(STATUS_UPPERCASE.RESOLVED); }, []); - const pollHostTable = useCallback(() => { - dispatch( - APIActions.get({ - key: JOB_INVOCATION_HOSTS, - url: `/api/job_invocations/${id}/hosts`, - params: { include_permissions: true, ...currentPollParams.current }, - handleSuccess: data => { - updateHostsState(data); - if (!jobFinishedRef.current) { - pollTimeoutId.current = setTimeout(pollHostTable, 5000); - } else { - pollTimeoutId.current = null; - } - }, - handleError: () => { - pollTimeoutId.current = null; - setStatus(STATUS_UPPERCASE.ERROR); - }, - errorToast: ({ response }) => - response?.data?.error?.full_messages?.[0] || - response?.data?.error?.message || - 'Error', - }) - ); - }, [dispatch, id, updateHostsState]); - // Call hosts data with params const makeApiCall = useCallback( requestParams => { @@ -202,7 +176,10 @@ const JobInvocationHostTable = ({ handleSuccess: data => { updateHostsState(data); if (!jobFinishedRef.current) { - pollTimeoutId.current = setTimeout(pollHostTable, 5000); + pollTimeoutId.current = setTimeout( + () => makeApiCall(currentPollParams.current), + 5000 + ); } else { pollTimeoutId.current = null; } @@ -218,7 +195,7 @@ const JobInvocationHostTable = ({ }) ); }, - [dispatch, id, pollHostTable, updateHostsState] + [dispatch, id, updateHostsState] ); const filterApiCall = useCallback( From 9f45cd3654290894179a39093aaabc460887ca4f Mon Sep 17 00:00:00 2001 From: Adam Ruzicka Date: Tue, 7 Jul 2026 15:07:51 +0200 Subject: [PATCH 27/27] Simplify error toast extraction and reuse task lookup in permissions Extract a shared extractErrorMessage helper to deduplicate the error toast fallback chain across fetchJobInvocation, cancelJob, enableRecurringLogic, and cancelRecurringLogic. Also normalize the inconsistent final fallback ('Error' vs 'Unknown error.') to always use 'Unknown error.'. Replace the redundant run_host_job_task re-lookup in set_permissions_by_host with a direct lookup from @task_by_host, which was already built from the same association. Co-Authored-By: Claude Sonnet 4.6 --- .../api/v2/job_invocations_controller.rb | 3 +- .../JobInvocationActions.js | 44 ++++++------------- 2 files changed, 14 insertions(+), 33 deletions(-) diff --git a/app/controllers/api/v2/job_invocations_controller.rb b/app/controllers/api/v2/job_invocations_controller.rb index 6c090ee65..3dbb7467d 100644 --- a/app/controllers/api/v2/job_invocations_controller.rb +++ b/app/controllers/api/v2/job_invocations_controller.rb @@ -336,8 +336,7 @@ def set_statuses_and_smart_proxies [ti.host_id, task] end @permissions_by_host = hosts.to_h do |host| - template_invocation = template_invocations_by_host_id[host.id] - task = template_invocation.try(:run_host_job_task) + task = @task_by_host[host.id] [host.id, { :view_foreman_tasks => authorized_for(:permission => :view_foreman_tasks, :auth_object => task), :cancel_job_invocations => authorized_for(:permission => :cancel_job_invocations, :auth_object => @job_invocation), diff --git a/webpack/JobInvocationDetail/JobInvocationActions.js b/webpack/JobInvocationDetail/JobInvocationActions.js index 665eea1f0..fa0ce9ea8 100644 --- a/webpack/JobInvocationDetail/JobInvocationActions.js +++ b/webpack/JobInvocationDetail/JobInvocationActions.js @@ -11,10 +11,16 @@ import { let pollTimeoutId = null; +const FINISHED_STATUSES = [STATUS.FAILED, STATUS.SUCCEEDED, STATUS.CANCELLED]; + export const isJobFinished = statusLabel => - statusLabel === STATUS.FAILED || - statusLabel === STATUS.SUCCEEDED || - statusLabel === STATUS.CANCELLED; + FINISHED_STATUSES.includes(statusLabel); + +const extractErrorMessage = response => + // eslint-disable-next-line camelcase + response?.data?.error?.full_messages?.[0] || + response?.data?.error?.message || + 'Unknown error.'; const fetchJobInvocation = (dispatch, url, params = {}) => { dispatch( @@ -35,13 +41,7 @@ const fetchJobInvocation = (dispatch, url, params = {}) => { handleError: () => { pollTimeoutId = null; }, - errorToast: ({ response }) => - // eslint-disable-next-line camelcase - response?.data?.error?.full_messages?.[0] || - // eslint-disable-next-line camelcase - response?.data?.error?.full_messages || - response?.data?.error?.message || - 'Error', + errorToast: ({ response }) => extractErrorMessage(response), }) ); }; @@ -73,13 +73,7 @@ export const cancelJob = (jobId, force) => dispatch => { APIActions.post({ url, key: CANCEL_JOB, - errorToast: ({ response }) => - errorToast( - // eslint-disable-next-line camelcase - response?.data?.error?.full_messages || - response?.data?.error?.message || - 'Unknown error.' - ), + errorToast: ({ response }) => errorToast(extractErrorMessage(response)), handleSuccess: () => { dispatch( addToast({ @@ -115,13 +109,7 @@ export const enableRecurringLogic = (recurrenceId, enabled) => dispatch => { key: CHANGE_ENABLED_RECURRING_LOGIC, params: { recurring_logic: { enabled: !enabled } }, successToast, - errorToast: ({ response }) => - errorToast( - // eslint-disable-next-line camelcase - response?.data?.error?.full_messages || - response?.data?.error?.message || - 'Unknown error.' - ), + errorToast: ({ response }) => errorToast(extractErrorMessage(response)), }) ); }; @@ -140,13 +128,7 @@ export const cancelRecurringLogic = recurrenceId => dispatch => { url, key: CANCEL_RECURRING_LOGIC, successToast, - errorToast: ({ response }) => - errorToast( - // eslint-disable-next-line camelcase - response?.data?.error?.full_messages || - response?.data?.error?.message || - 'Unknown error.' - ), + errorToast: ({ response }) => errorToast(extractErrorMessage(response)), }) ); };