diff --git a/app/controllers/api/v2/job_invocations_controller.rb b/app/controllers/api/v2/job_invocations_controller.rb index 0bf640111..3dbb7467d 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,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] } + return unless @include_permissions + @task_by_host = template_invocations.to_h do |ti| + task = ti.run_host_job_task + [ti.host_id, task] + end + @permissions_by_host = hosts.to_h do |host| + 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), + :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/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/app/views/api/v2/job_invocations/hosts.json.rabl b/app/views/api/v2/job_invocations/hosts.json.rabl index 7e627dfef..e07a9bbde 100644 --- a/app/views/api/v2/job_invocations/hosts.json.rabl +++ b/app/views/api/v2/job_invocations/hosts.json.rabl @@ -13,3 +13,12 @@ end node :smart_proxy_name do |host| @smart_proxy_name[host.id] end + +node(:task, :if => ->(_) { @task_by_host }) do |host| + task = @task_by_host[host.id] + task && { :id => task.id, :cancellable => task.try(:cancellable?) } +end + +node(:permissions, :if => ->(_) { @permissions_by_host }) do |host| + @permissions_by_host[host.id] +end diff --git a/app/views/api/v2/job_invocations/main.json.rabl b/app/views/api/v2/job_invocations/main.json.rabl index 3e4111a44..18cd9b31a 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.try(:cancellable?) } end if @template_invocations 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' diff --git a/webpack/JobInvocationDetail/JobInvocationActions.js b/webpack/JobInvocationDetail/JobInvocationActions.js index 131696068..fa0ce9ea8 100644 --- a/webpack/JobInvocationDetail/JobInvocationActions.js +++ b/webpack/JobInvocationDetail/JobInvocationActions.js @@ -1,52 +1,59 @@ 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, CHANGE_ENABLED_RECURRING_LOGIC, - GET_TASK, JOB_INVOCATION_KEY, - UPDATE_JOB, + STATUS, } from './JobInvocationConstants'; -export const getJobInvocation = url => dispatch => { - const fetchData = withInterval( +let pollTimeoutId = null; + +const FINISHED_STATUSES = [STATUS.FAILED, STATUS.SUCCEEDED, STATUS.CANCELLED]; + +export const isJobFinished = statusLabel => + 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( get({ key: JOB_INVOCATION_KEY, - params: { include_permissions: true, include_hosts: false }, + params: { include_hosts: false, ...params }, url, + handleSuccess: ({ data }) => { + if (!isJobFinished(data.status_label)) { + pollTimeoutId = setTimeout( + () => fetchJobInvocation(dispatch, url), + 5000 + ); + } else { + pollTimeoutId = null; + } + }, handleError: () => { - dispatch(stopInterval(JOB_INVOCATION_KEY)); + 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', - }), - 1000 + errorToast: ({ response }) => extractErrorMessage(response), + }) ); +}; - dispatch(fetchData); +export const getJobInvocation = url => dispatch => { + stopJobInvocationPolling(); + fetchJobInvocation(dispatch, url, { include_permissions: true }); }; -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 stopJobInvocationPolling = () => { + clearTimeout(pollTimeoutId); + pollTimeoutId = null; }; export const cancelJob = (jobId, force) => dispatch => { @@ -66,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({ @@ -81,26 +82,12 @@ export const cancelJob = (jobId, force) => dispatch => { message: infoToast(), }) ); - dispatch(updateJob(jobId)); }, }) ); }; -export const getTask = taskId => dispatch => { - dispatch( - get({ - key: GET_TASK, - url: `/foreman_tasks/api/tasks/${taskId}`, - }) - ); -}; - -export const enableRecurringLogic = ( - recurrenceId, - enabled, - jobId -) => dispatch => { +export const enableRecurringLogic = (recurrenceId, enabled) => dispatch => { const successToast = () => enabled ? sprintf(__('Recurring logic %s disabled successfully.'), recurrenceId) @@ -122,19 +109,12 @@ export const enableRecurringLogic = ( 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.' - ), - handleSuccess: () => dispatch(updateJob(jobId)), + errorToast: ({ response }) => errorToast(extractErrorMessage(response)), }) ); }; -export const cancelRecurringLogic = (recurrenceId, jobId) => dispatch => { +export const cancelRecurringLogic = recurrenceId => dispatch => { const successToast = () => sprintf(__('Recurring logic %s cancelled successfully.'), recurrenceId); const errorToast = response => @@ -148,14 +128,7 @@ export const cancelRecurringLogic = (recurrenceId, jobId) => 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.' - ), - handleSuccess: () => dispatch(updateJob(jobId)), + errorToast: ({ response }) => errorToast(extractErrorMessage(response)), }) ); }; diff --git a/webpack/JobInvocationDetail/JobInvocationConstants.js b/webpack/JobInvocationDetail/JobInvocationConstants.js index 8eb70ffe0..4ea808c59 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'; @@ -18,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' @@ -26,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 9daaafe58..87b8293a2 100644 --- a/webpack/JobInvocationDetail/JobInvocationHostTable.js +++ b/webpack/JobInvocationDetail/JobInvocationHostTable.js @@ -39,9 +39,7 @@ import { CheckboxesActions } from './CheckboxesActions'; import DropdownFilter from './DropdownFilter'; import Columns, { JOB_INVOCATION_HOSTS, - LIST_TEMPLATE_INVOCATIONS, STATUS_UPPERCASE, - ALL_JOB_HOSTS, AWAITING_STATUS_FILTER, } from './JobInvocationConstants'; import { TemplateInvocation } from './TemplateInvocation'; @@ -51,8 +49,8 @@ import { PopupAlert } from './OpenAllInvocationsModal'; const JobInvocationHostTable = ({ id, initialFilter, + jobFinished, onFilterUpdate, - statusLabel, targeting, }) => { const columns = Columns(); @@ -69,7 +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 jobFinishedRef = useRef(jobFinished); + useEffect(() => { + jobFinishedRef.current = jobFinished; + }, [jobFinished]); const [hostInvocationStates, setHostInvocationStates] = useState({}); @@ -153,33 +158,44 @@ const JobInvocationHostTable = ({ [initialFilter, urlSearchQuery] ); - const handleResponse = useCallback((data, key) => { - if (key === JOB_INVOCATION_HOSTS) { - const ids = data.data.results.map(i => i.id); - - setApiResponse(data.data); - setAllHostsIds(ids); - } - + const updateHostsState = useCallback(data => { + 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`, - params: requestParams, - handleSuccess: data => handleResponse(data, callParams.key), - handleError: () => setStatus(STATUS_UPPERCASE.ERROR), + key: JOB_INVOCATION_HOSTS, + url: `/api/job_invocations/${id}/hosts`, + params: { include_permissions: true, ...requestParams }, + handleSuccess: data => { + updateHostsState(data); + if (!jobFinishedRef.current) { + pollTimeoutId.current = setTimeout( + () => makeApiCall(currentPollParams.current), + 5000 + ); + } else { + pollTimeoutId.current = null; + } + }, + 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', }) ); }, - [dispatch, id, handleResponse] + [dispatch, id, updateHostsState] ); const filterApiCall = useCallback( @@ -202,7 +218,11 @@ const JobInvocationHostTable = ({ finalParams.search = filterSearch; } - makeApiCall(finalParams, { key: JOB_INVOCATION_HOSTS }); + currentPollParams.current = finalParams; + clearTimeout(pollTimeoutId.current); + pollTimeoutId.current = null; + + makeApiCall(finalParams); const urlSearchParams = new URLSearchParams(window.location.search); @@ -222,43 +242,35 @@ const JobInvocationHostTable = ({ ] ); - // Filter change - const handleFilterChange = useCallback( - newFilter => { - onFilterUpdate(newFilter); - }, - [onFilterUpdate] - ); - // Effects // run after mount 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(() => { - if (initialFilter !== '') filterApiCall(); + const filterChanged = initialFilter !== prevFilter.current; + const statusChanged = jobFinished !== prevJobFinished.current; - if (statusLabel !== prevStatusLabel.current) { - prevStatusLabel.current = statusLabel; + if ((filterChanged || statusChanged) && initialFilter !== '') { + prevFilter.current = initialFilter; + prevJobFinished.current = jobFinished; filterApiCall(); } - }, [initialFilter, statusLabel, id, filterApiCall]); + }, [initialFilter, jobFinished, id, filterApiCall]); + + useEffect( + () => () => { + clearTimeout(pollTimeoutId.current); + }, + [] + ); const { updateSearchQuery: updateSearchQueryBulk, @@ -403,7 +415,7 @@ const JobInvocationHostTable = ({ , 0 ? fetchBulkParams() : null} @@ -541,13 +553,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/JobInvocationSelectors.js b/webpack/JobInvocationDetail/JobInvocationSelectors.js index eca70541a..db2b8f259 100644 --- a/webpack/JobInvocationDetail/JobInvocationSelectors.js +++ b/webpack/JobInvocationDetail/JobInvocationSelectors.js @@ -6,19 +6,16 @@ import { import { STATUS as APIStatus } from 'foremanReact/constants'; import { JOB_INVOCATION_KEY, - GET_TASK, + JOB_INVOCATION_HOSTS, GET_TEMPLATE_INVOCATION, - LIST_TEMPLATE_INVOCATIONS, CURRENT_PERMISSIONS, } from './JobInvocationConstants'; 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}`); @@ -26,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/JobInvocationToolbarButtons.js b/webpack/JobInvocationDetail/JobInvocationToolbarButtons.js index 8c2d621fb..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, jobId) - ) + dispatch(enableRecurringLogic(recurrence?.id, recurringEnabled)) } key="change-enabled-recurring" component="button" @@ -117,7 +115,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 9045be218..8f6ba51c5 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,49 +74,44 @@ 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(() => { + let cancelled = false; const dispatchFetch = () => { dispatch( APIActions.get({ url: templateURL, key: `${GET_TEMPLATE_INVOCATION}_${hostID}`, + handleSuccess: ({ data }) => { + if (cancelled) return; + const finished = data?.finished ?? false; + // eslint-disable-next-line camelcase + const autoRefresh = data?.auto_refresh || false; + if (!finished && autoRefresh) { + timeoutRef.current = setTimeout(dispatchFetch, 5000); + } + }, + handleError: () => { + if (cancelled) return; + 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); - } + cancelled = true; + clearTimeout(timeoutRef.current); }; }, [isExpanded, dispatch, templateURL, hostID]); 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 || {}; diff --git a/webpack/JobInvocationDetail/__tests__/JobInvocationPolling.test.js b/webpack/JobInvocationDetail/__tests__/JobInvocationPolling.test.js new file mode 100644 index 000000000..32dc7ae5d --- /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(5000); + 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(5000); + + 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(5000); + + // 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(15000); + + expect(api.get).toHaveBeenCalledTimes(4); + }); +}); 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(() => { diff --git a/webpack/JobInvocationDetail/index.js b/webpack/JobInvocationDetail/index.js index 34fc19443..7222e90fb 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,13 +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, + isJobFinished, + stopJobInvocationPolling, +} from './JobInvocationActions'; import './JobInvocationDetail.scss'; import { CURRENT_PERMISSIONS, DATE_OPTIONS, - JOB_INVOCATION_KEY, - STATUS, STATUS_UPPERCASE, currentPermissionsUrl, } from './JobInvocationConstants'; @@ -46,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, }); @@ -73,20 +69,10 @@ const JobInvocationDetailPage = ({ useEffect(() => { dispatch(getJobInvocation(`/api/job_invocations/${id}`)); - if (finished && !autoRefresh) { - dispatch(stopInterval(JOB_INVOCATION_KEY)); - } return () => { - dispatch(stopInterval(JOB_INVOCATION_KEY)); + stopJobInvocationPolling(); }; - }, [dispatch, id, finished, autoRefresh]); - - const taskId = task?.id; - useEffect(() => { - if (taskId !== undefined) { - dispatch(getTask(`${taskId}`)); - } - }, [dispatch, taskId]); + }, [dispatch, id]); const pageStatus = items.id === undefined @@ -183,10 +169,8 @@ const JobInvocationDetailPage = ({