diff --git a/app/controllers/api/v2/job_invocations_controller.rb b/app/controllers/api/v2/job_invocations_controller.rb index 0bf640111..4965507e4 100644 --- a/app/controllers/api/v2/job_invocations_controller.rb +++ b/app/controllers/api/v2/job_invocations_controller.rb @@ -117,12 +117,14 @@ def output param_group :search_and_pagination, ::Api::V2::BaseController add_scoped_search_description_for(JobInvocation) param :id, :identifier, :required => true + param :include_permissions, :bool, :required => false, :desc => N_('Include per-host task and permission data in the response') def hosts 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]) @subtotal = @hosts.total_entries set_statuses_and_smart_proxies + set_tasks_and_permissions if Foreman::Cast.to_bool(params[:include_permissions]) if params[:awaiting] @hosts = @hosts.select { |host| @host_statuses[host.id] == 'N/A' } end @@ -321,15 +323,35 @@ def set_statuses_and_smart_proxies template_invocations = @template_invocations.where(host_id: @hosts.select(:id)) .includes(:run_host_job_task).to_a hosts = @hosts.to_a - template_invocations_by_host_id = template_invocations.index_by(&:host_id) + @template_invocations_by_host_id = template_invocations.index_by(&:host_id) @host_statuses = hosts.to_h do |host| - template_invocation = template_invocations_by_host_id[host.id] + template_invocation = @template_invocations_by_host_id[host.id] task = template_invocation.try(:run_host_job_task) [host.id, template_invocation_status(task, @job_invocation.task)] 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] } end + + def set_tasks_and_permissions + can_view_tasks = User.current.can?(:view_foreman_tasks) + can_cancel = authorized_for(:permission => :cancel_job_invocations, :auth_object => @job_invocation) + can_create = authorized_for(controller: :job_invocations, action: :create) + can_execute_on_infra = User.current.can?(:execute_jobs_on_infrastructure_hosts) + + hosts = @hosts.to_a + @task_by_host = hosts.to_h do |host| + task = @template_invocations_by_host_id[host.id].try(:run_host_job_task) + [host.id, task ? { :id => task.id, :cancellable => task.try(:cancellable?) || false } : nil] + end + @permissions_by_host = hosts.to_h do |host| + [host.id, { + :view_foreman_tasks => can_view_tasks, + :cancel_job_invocations => can_cancel, + :execute_jobs => can_create && (!host.infrastructure_host? || can_execute_on_infra), + }] + end + 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..bcd6454f0 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, :if => ->(_host) { @task_by_host }) do |host| + @task_by_host[host.id] +end + +node(:permissions, :if => ->(_host) { @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 6dded4bb8..a53190859 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/test/functional/api/v2/job_invocations_controller_test.rb b/test/functional/api/v2/job_invocations_controller_test.rb index 2b9d84e2b..e5d7702d5 100644 --- a/test/functional/api/v2/job_invocations_controller_test.rb +++ b/test/functional/api/v2/job_invocations_controller_test.rb @@ -284,20 +284,59 @@ class JobInvocationsControllerTest < ActionController::TestCase end describe '#hosts' do - test 'should compute job_status for the paginated subset' do - invocation = FactoryBot.create(:job_invocation, :with_template, :with_task) - 2.times { invocation.template_invocations << FactoryBot.create(:template_invocation, :with_task, :with_host, :job_invocation => invocation) } - invocation.job_category = invocation.pattern_template_invocations.first.template.job_category - invocation.targeting.hosts = invocation.template_invocations.map(&:host) - invocation.save! + setup do + @hosts_invocation = FactoryBot.create(:job_invocation, :with_template, :with_task) + 2.times { @hosts_invocation.template_invocations << FactoryBot.create(:template_invocation, :with_task, :with_host, :job_invocation => @hosts_invocation) } + @hosts_invocation.job_category = @hosts_invocation.pattern_template_invocations.first.template.job_category + @hosts_invocation.targeting.hosts = @hosts_invocation.template_invocations.map(&:host) + @hosts_invocation.save! + end - get :hosts, params: { :id => invocation.id, :page => 2, :per_page => 1 } + test 'should compute job_status for the paginated subset' do + get :hosts, params: { :id => @hosts_invocation.id, :page => 2, :per_page => 1 } assert_response :success result = ActiveSupport::JSON.decode(@response.body) assert_equal 3, result['total'] assert_equal 1, result['results'].size assert_equal(['success'], result['results'].map { |r| r['job_status'] }) end + + test 'should not include task and permissions without include_permissions' do + get :hosts, params: { :id => @hosts_invocation.id } + assert_response :success + result = ActiveSupport::JSON.decode(@response.body) + result['results'].each do |host| + assert_not host.key?('task'), 'Expected task to be absent' + assert_not host.key?('permissions'), 'Expected permissions to be absent' + end + end + + test 'should include task and permissions with include_permissions=true' do + get :hosts, params: { :id => @hosts_invocation.id, :include_permissions => true } + assert_response :success + result = ActiveSupport::JSON.decode(@response.body) + result['results'].each do |host| + assert host.key?('task'), 'Expected task to be present' + assert host.key?('permissions'), 'Expected permissions to be present' + permissions = host['permissions'] + assert permissions.key?('view_foreman_tasks') + assert permissions.key?('cancel_job_invocations') + assert permissions.key?('execute_jobs') + end + end + + test 'should handle hosts with nil task gracefully when include_permissions=true' do + invocation = FactoryBot.create(:job_invocation, :with_template, :with_task, :with_unplanned_host) + invocation.job_category = invocation.pattern_template_invocations.first.template.job_category + invocation.save! + + get :hosts, params: { :id => invocation.id, :include_permissions => true } + assert_response :success + result = ActiveSupport::JSON.decode(@response.body) + result['results'].each do |host| + assert host.key?('permissions'), 'Expected permissions to be present' + end + end end describe 'raw output' do diff --git a/webpack/JobInvocationDetail/JobInvocationActions.js b/webpack/JobInvocationDetail/JobInvocationActions.js index 131696068..978a6acc6 100644 --- a/webpack/JobInvocationDetail/JobInvocationActions.js +++ b/webpack/JobInvocationDetail/JobInvocationActions.js @@ -1,52 +1,71 @@ 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 { APIActions } from 'foremanReact/redux/API'; 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( - get({ - key: JOB_INVOCATION_KEY, - params: { include_permissions: true, include_hosts: false }, - url, - handleError: () => { - dispatch(stopInterval(JOB_INVOCATION_KEY)); - }, - 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 - ); +const POLL_INTERVAL = 5000; + +export const isJobFinished = statusLabel => + statusLabel === STATUS.FAILED || + statusLabel === STATUS.SUCCEEDED || + statusLabel === STATUS.CANCELLED; + +const extractErrorMessage = response => + // eslint-disable-next-line camelcase + response?.data?.error?.full_messages?.[0] || + response?.data?.error?.message || + __('Unknown error.'); + +export const getJobInvocation = (url, pollTimeoutRef) => dispatch => { + let cancelled = false; - dispatch(fetchData); + const poll = () => { + if (cancelled) return; + dispatch( + APIActions.get({ + key: JOB_INVOCATION_KEY, + params: { include_permissions: true, include_hosts: false }, + url, + handleSuccess: ({ data }) => { + if (cancelled) return; + // eslint-disable-next-line camelcase + if (!isJobFinished(data?.status_label)) { + pollTimeoutRef.current = setTimeout(poll, POLL_INTERVAL); + } else { + pollTimeoutRef.current = null; + } + }, + handleError: () => { + if (cancelled) return; + pollTimeoutRef.current = null; + }, + errorToast: ({ response }) => extractErrorMessage(response), + }) + ); + }; + + stopJobInvocationPolling(pollTimeoutRef); + + // Cancel previous poll cycle's in-flight requests + if (pollTimeoutRef.cancelPreviousCycle) { + pollTimeoutRef.cancelPreviousCycle(); + } + pollTimeoutRef.cancelPreviousCycle = () => { + cancelled = true; + }; + + poll(); }; -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 = pollTimeoutRef => { + clearTimeout(pollTimeoutRef.current); + pollTimeoutRef.current = null; }; export const cancelJob = (jobId, force) => dispatch => { @@ -54,10 +73,6 @@ export const cancelJob = (jobId, force) => dispatch => { force ? sprintf(__('Trying to abort the job %s.'), jobId) : sprintf(__('Trying to cancel the job %s.'), jobId); - const errorToast = response => - force - ? sprintf(__(`Could not abort the job %s: ${response}`), jobId) - : sprintf(__(`Could not cancel the job %s: ${response}`), jobId); const url = force ? `/job_invocations/${jobId}/cancel?force=true` : `/job_invocations/${jobId}/cancel`; @@ -67,12 +82,17 @@ export const cancelJob = (jobId, force) => dispatch => { url, key: CANCEL_JOB, errorToast: ({ response }) => - errorToast( - // eslint-disable-next-line camelcase - response?.data?.error?.full_messages || - response?.data?.error?.message || - 'Unknown error.' - ), + force + ? sprintf( + __('Could not abort the job %s: %s'), + jobId, + extractErrorMessage(response) + ) + : sprintf( + __('Could not cancel the job %s: %s'), + jobId, + extractErrorMessage(response) + ), handleSuccess: () => { dispatch( addToast({ @@ -81,40 +101,16 @@ 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) : sprintf(__('Recurring logic %s enabled successfully.'), recurrenceId); - const errorToast = response => - enabled - ? sprintf( - __(`Could not disable recurring logic %s: ${response}`), - recurrenceId - ) - : sprintf( - __(`Could not enable recurring logic %s: ${response}`), - recurrenceId - ); const url = `/foreman_tasks/api/recurring_logics/${recurrenceId}`; dispatch( APIActions.put({ @@ -123,25 +119,24 @@ export const enableRecurringLogic = ( 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)), + enabled + ? sprintf( + __('Could not disable recurring logic %s: %s'), + recurrenceId, + extractErrorMessage(response) + ) + : sprintf( + __('Could not enable recurring logic %s: %s'), + recurrenceId, + 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 => - sprintf( - __(`Could not cancel recurring logic %s: ${response}`), - recurrenceId - ); const url = `/foreman_tasks/recurring_logics/${recurrenceId}/cancel`; dispatch( APIActions.post({ @@ -149,13 +144,11 @@ export const cancelRecurringLogic = (recurrenceId, jobId) => dispatch => { key: CANCEL_RECURRING_LOGIC, successToast, errorToast: ({ response }) => - errorToast( - // eslint-disable-next-line camelcase - response?.data?.error?.full_messages || - response?.data?.error?.message || - 'Unknown error.' + sprintf( + __('Could not cancel recurring logic %s: %s'), + recurrenceId, + extractErrorMessage(response) ), - handleSuccess: () => dispatch(updateJob(jobId)), }) ); }; diff --git a/webpack/JobInvocationDetail/JobInvocationConstants.js b/webpack/JobInvocationDetail/JobInvocationConstants.js index 191249394..2ae505420 100644 --- a/webpack/JobInvocationDetail/JobInvocationConstants.js +++ b/webpack/JobInvocationDetail/JobInvocationConstants.js @@ -6,9 +6,7 @@ import { useForemanHostDetailsPageUrl } from 'foremanReact/Root/Context/ForemanC import JobStatusIcon from '../react_app/components/RecentJobsCard/JobStatusIcon'; export const JOB_INVOCATION_KEY = 'JOB_INVOCATION_KEY'; -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'; @@ -17,14 +15,12 @@ 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 AUTO_REFRESH_INTERVAL_MS = 5000; 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 785ad1fc1..f860fbd45 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, - LIST_TEMPLATE_INVOCATIONS, STATUS_UPPERCASE, AWAITING_STATUS_FILTER, AUTO_REFRESH_INTERVAL_MS, @@ -163,11 +162,24 @@ const JobInvocationHostTable = ({ [initialFilter, urlSearchQuery] ); + const [hostPermissions, setHostPermissions] = useState({}); + const updateHostsState = useCallback(data => { const ids = data.data.results.map(i => i.id); setApiResponse(data.data); setAllHostsIds(ids); setStatus(STATUS_UPPERCASE.RESOLVED); + + const resultsWithPermissions = data.data.results.filter(r => r.permissions); + if (resultsWithPermissions.length > 0) { + setHostPermissions(prev => { + const updated = { ...prev }; + resultsWithPermissions.forEach(r => { + updated[r.id] = { task: r.task, permissions: r.permissions }; + }); + return updated; + }); + } }, []); // Call hosts data with params @@ -229,7 +241,10 @@ const JobInvocationHostTable = ({ finalParams.search = filterSearch; } - currentPollParams.current = finalParams; + finalParams.include_permissions = true; + + currentPollParams.current = { ...finalParams }; + delete currentPollParams.current.include_permissions; clearTimeout(pollTimeoutId.current); pollTimeoutId.current = null; @@ -258,20 +273,12 @@ const JobInvocationHostTable = ({ const initializedRef = useRef(false); useEffect(() => { if (!initializedRef.current) { - dispatch( - APIActions.get({ - key: LIST_TEMPLATE_INVOCATIONS, - url: `/job_invocations/${id}/hosts`, - params: {}, - }) - ); - if (initialFilter === '') { onFilterUpdate('all_statuses'); } initializedRef.current = true; } - }, [dispatch, id, initialFilter, onFilterUpdate]); + }, [initialFilter, onFilterUpdate]); useEffect(() => { const filterChanged = initialFilter !== prevFilter.current; @@ -506,7 +513,12 @@ const JobInvocationHostTable = ({ {columns[k].wrapper(result)} ))} - + 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}`); 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; diff --git a/webpack/JobInvocationDetail/JobInvocationToolbarButtons.js b/webpack/JobInvocationDetail/JobInvocationToolbarButtons.js index 7869d331c..5bb6d1deb 100644 --- a/webpack/JobInvocationDetail/JobInvocationToolbarButtons.js +++ b/webpack/JobInvocationDetail/JobInvocationToolbarButtons.js @@ -141,9 +141,7 @@ const JobInvocationToolbarButtons = ({ jobId, data }) => { - dispatch( - enableRecurringLogic(recurrence?.id, recurringEnabled, jobId) - ) + dispatch(enableRecurringLogic(recurrence?.id, recurringEnabled)) } key="change-enabled-recurring" component="button" @@ -159,9 +157,7 @@ const JobInvocationToolbarButtons = ({ jobId, data }) => { , - dispatch(cancelRecurringLogic(recurrence?.id, jobId)) - } + onClick={() => dispatch(cancelRecurringLogic(recurrence?.id))} key="cancel-recurring" component="button" isDisabled={ @@ -174,7 +170,7 @@ const JobInvocationToolbarButtons = ({ jobId, data }) => { , ] : [], - [recurrence, recurringEnabled, canEditRecurringLogic, dispatch, jobId] + [recurrence, recurringEnabled, canEditRecurringLogic, dispatch] ); const dropdownItems = useMemo( diff --git a/webpack/JobInvocationDetail/TemplateInvocation.js b/webpack/JobInvocationDetail/TemplateInvocation.js index 9045be218..5592db373 100644 --- a/webpack/JobInvocationDetail/TemplateInvocation.js +++ b/webpack/JobInvocationDetail/TemplateInvocation.js @@ -11,6 +11,7 @@ import { showTemplateInvocationUrl, templateInvocationPageUrl, GET_TEMPLATE_INVOCATION, + AUTO_REFRESH_INTERVAL_MS, } from './JobInvocationConstants'; import { selectTemplateInvocationStatus, @@ -67,7 +68,7 @@ export const TemplateInvocation = ({ showCommand, setShowCommand, }) => { - const intervalRef = useRef(null); + const timeoutRef = useRef(null); const templateURL = showTemplateInvocationUrl(hostID, jobID); const hostDetailsPageUrl = useForemanHostDetailsPageUrl(); @@ -81,43 +82,48 @@ export const TemplateInvocation = ({ }, [response]); useEffect(() => { - const dispatchFetch = () => { + let cancelled = false; + + const schedulePoll = () => { + if (cancelled) return; dispatch( APIActions.get({ url: templateURL, key: `${GET_TEMPLATE_INVOCATION}_${hostID}`, + handleSuccess: ({ data }) => { + if (cancelled) return; + const isFinished = data?.finished ?? true; + // eslint-disable-next-line camelcase + const autoRefresh = data?.auto_refresh || false; + if (!isFinished && autoRefresh) { + timeoutRef.current = setTimeout( + schedulePoll, + AUTO_REFRESH_INTERVAL_MS + ); + } else { + timeoutRef.current = null; + } + }, + handleError: () => { + if (cancelled) return; + timeoutRef.current = null; + }, }) ); }; - if (intervalRef.current) { - clearInterval(intervalRef.current); - intervalRef.current = null; - } + 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); + if (responseRef.current?.finished) return undefined; + schedulePoll(); } return () => { - if (intervalRef.current) { - clearInterval(intervalRef.current); - } + cancelled = true; + clearTimeout(timeoutRef.current); + timeoutRef.current = null; }; }, [isExpanded, dispatch, templateURL, hostID]); diff --git a/webpack/JobInvocationDetail/TemplateInvocationComponents/TemplateActionButtons.js b/webpack/JobInvocationDetail/TemplateInvocationComponents/TemplateActionButtons.js index f9242f646..0eade9c3d 100644 --- a/webpack/JobInvocationDetail/TemplateInvocationComponents/TemplateActionButtons.js +++ b/webpack/JobInvocationDetail/TemplateInvocationComponents/TemplateActionButtons.js @@ -1,12 +1,11 @@ import React from 'react'; import PropTypes from 'prop-types'; -import { useDispatch, useSelector } from 'react-redux'; +import { useDispatch } from 'react-redux'; import { Flex, FlexItem, Button } from '@patternfly/react-core'; 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 './index.scss'; const actions = ({ @@ -81,11 +80,9 @@ const actions = ({ }, }); -export const RowActions = ({ hostID, jobID }) => { +export const RowActions = ({ hostID, jobID, task, permissions }) => { const dispatch = useDispatch(); - const response = useSelector(selectTemplateInvocationList)?.[hostID]; - if (!response?.permissions) return null; - const { task, permissions } = response; + if (!permissions) return null; const { id: taskID, cancellable: taskCancellable } = task || {}; const getActions = actions({ taskID, @@ -216,4 +213,18 @@ TemplateActionButtons.defaultProps = { RowActions.propTypes = { hostID: PropTypes.oneOfType([PropTypes.string, PropTypes.number]).isRequired, jobID: PropTypes.oneOfType([PropTypes.string, PropTypes.number]).isRequired, + task: PropTypes.shape({ + id: PropTypes.oneOfType([PropTypes.string, PropTypes.number]), + cancellable: PropTypes.bool, + }), + permissions: PropTypes.shape({ + view_foreman_tasks: PropTypes.bool, + cancel_job_invocations: PropTypes.bool, + execute_jobs: PropTypes.bool, + }), +}; + +RowActions.defaultProps = { + task: null, + permissions: null, }; diff --git a/webpack/JobInvocationDetail/__tests__/JobInvocationHostTablePolling.test.js b/webpack/JobInvocationDetail/__tests__/JobInvocationHostTablePolling.test.js index 94140cfd7..18de3f68a 100644 --- a/webpack/JobInvocationDetail/__tests__/JobInvocationHostTablePolling.test.js +++ b/webpack/JobInvocationDetail/__tests__/JobInvocationHostTablePolling.test.js @@ -136,13 +136,13 @@ describe('JobInvocationHostTable polling', () => { it('schedules a poll after the initial fetch succeeds', () => { renderTable(); - expect(hostsCalls.length).toBe(1); + expect(hostsCalls).toHaveLength(1); act(() => { flushPendingCallbacks(); }); - expect(hostsCalls.length).toBe(1); + expect(hostsCalls).toHaveLength(1); act(() => { jest.advanceTimersByTime(5000); @@ -152,7 +152,7 @@ describe('JobInvocationHostTable polling', () => { flushPendingCallbacks(); }); - expect(hostsCalls.length).toBe(2); + expect(hostsCalls).toHaveLength(2); act(() => { jest.advanceTimersByTime(5000); @@ -162,13 +162,13 @@ describe('JobInvocationHostTable polling', () => { flushPendingCallbacks(); }); - expect(hostsCalls.length).toBe(3); + expect(hostsCalls).toHaveLength(3); }); it('does not schedule a poll when jobFinished is true', () => { renderTable({ jobFinished: true }); - expect(hostsCalls.length).toBe(1); + expect(hostsCalls).toHaveLength(1); act(() => { flushPendingCallbacks(); @@ -182,7 +182,7 @@ describe('JobInvocationHostTable polling', () => { flushPendingCallbacks(); }); - expect(hostsCalls.length).toBe(1); + expect(hostsCalls).toHaveLength(1); }); it('stops polling when jobFinished transitions to true', () => { @@ -192,7 +192,7 @@ describe('JobInvocationHostTable polling', () => { flushPendingCallbacks(); }); - expect(hostsCalls.length).toBe(1); + expect(hostsCalls).toHaveLength(1); const store = createStore(); const history = createMemoryHistory(); @@ -233,7 +233,7 @@ describe('JobInvocationHostTable polling', () => { flushPendingCallbacks(); }); - expect(hostsCalls.length).toBe(callsAtTransition); + expect(hostsCalls).toHaveLength(callsAtTransition); }); it('cleans up the poll timer on unmount', () => { @@ -255,7 +255,7 @@ describe('JobInvocationHostTable polling', () => { flushPendingCallbacks(); }); - expect(hostsCalls.length).toBe(callsBeforeUnmount); + expect(hostsCalls).toHaveLength(callsBeforeUnmount); }); it('restarts polling when filter changes', () => { @@ -317,7 +317,7 @@ describe('JobInvocationHostTable polling', () => { renderTable(); - expect(hostsCalls.length).toBe(1); + expect(hostsCalls).toHaveLength(1); act(() => { flushPendingCallbacks(); @@ -331,6 +331,6 @@ describe('JobInvocationHostTable polling', () => { flushPendingCallbacks(); }); - expect(hostsCalls.length).toBe(1); + expect(hostsCalls).toHaveLength(1); }); }); diff --git a/webpack/JobInvocationDetail/__tests__/JobInvocationPolling.test.js b/webpack/JobInvocationDetail/__tests__/JobInvocationPolling.test.js new file mode 100644 index 000000000..e00520feb --- /dev/null +++ b/webpack/JobInvocationDetail/__tests__/JobInvocationPolling.test.js @@ -0,0 +1,200 @@ +import { createStore, applyMiddleware } from 'redux'; +import thunk from 'redux-thunk'; +import { APIActions } from 'foremanReact/redux/API'; +import { + getJobInvocation, + stopJobInvocationPolling, +} from '../JobInvocationActions'; +import { JOB_INVOCATION_KEY } from '../JobInvocationConstants'; + +jest.useFakeTimers(); + +const reducer = (state = {}) => state; +const makeStore = () => createStore(reducer, applyMiddleware(thunk)); +const makeRef = () => ({ current: null }); + +const runningData = { status_label: 'running' }; +const succeededData = { status_label: 'succeeded' }; +const failedData = { status_label: 'failed' }; +const cancelledData = { status_label: 'cancelled' }; + +let apiGetSpy; + +const setupGetMock = responseData => { + apiGetSpy = jest + .spyOn(APIActions, 'get') + .mockImplementation(({ handleSuccess, ...action }) => dispatch => { + handleSuccess && handleSuccess({ data: responseData }); + return dispatch({ type: 'MOCK_GET', ...action }); + }); +}; + +const url = '/api/job_invocations/1'; + +describe('job invocation polling', () => { + let ref; + + beforeEach(() => { + ref = makeRef(); + jest.clearAllTimers(); + }); + + afterEach(() => { + if (apiGetSpy) { + apiGetSpy.mockRestore(); + } + }); + + it('sends include_permissions and include_hosts on the initial fetch', () => { + setupGetMock(succeededData); + const store = makeStore(); + + store.dispatch(getJobInvocation(url, ref)); + + expect(apiGetSpy).toHaveBeenCalledTimes(1); + expect(apiGetSpy.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 = makeStore(); + + store.dispatch(getJobInvocation(url, ref)); + expect(apiGetSpy).toHaveBeenCalledTimes(1); + + jest.advanceTimersByTime(5000); + expect(apiGetSpy).toHaveBeenCalledTimes(2); + }); + + it('includes include_permissions=true in every poll call', () => { + setupGetMock(runningData); + const store = makeStore(); + + store.dispatch(getJobInvocation(url, ref)); + jest.advanceTimersByTime(5000); + + expect(apiGetSpy).toHaveBeenCalledTimes(2); + expect(apiGetSpy.mock.calls[1][0].params).toMatchObject({ + include_hosts: false, + include_permissions: true, + }); + }); + + it('stops polling when job succeeds', () => { + setupGetMock(succeededData); + const store = makeStore(); + + store.dispatch(getJobInvocation(url, ref)); + expect(apiGetSpy).toHaveBeenCalledTimes(1); + + jest.advanceTimersByTime(5000); + expect(apiGetSpy).toHaveBeenCalledTimes(1); + }); + + it('stops polling when job fails', () => { + setupGetMock(failedData); + const store = makeStore(); + + store.dispatch(getJobInvocation(url, ref)); + jest.advanceTimersByTime(5000); + + expect(apiGetSpy).toHaveBeenCalledTimes(1); + }); + + it('stops polling when job is cancelled', () => { + setupGetMock(cancelledData); + const store = makeStore(); + + store.dispatch(getJobInvocation(url, ref)); + jest.advanceTimersByTime(5000); + + expect(apiGetSpy).toHaveBeenCalledTimes(1); + }); + + it('stops polling on fetch error', () => { + apiGetSpy = jest + .spyOn(APIActions, 'get') + .mockImplementation(({ handleError, ...action }) => dispatch => { + handleError && handleError(); + return dispatch({ type: 'MOCK_GET', ...action }); + }); + const store = makeStore(); + + store.dispatch(getJobInvocation(url, ref)); + jest.advanceTimersByTime(5000); + + expect(apiGetSpy).toHaveBeenCalledTimes(1); + }); + + it('stopJobInvocationPolling cancels a pending timeout', () => { + setupGetMock(runningData); + const store = makeStore(); + + store.dispatch(getJobInvocation(url, ref)); + expect(apiGetSpy).toHaveBeenCalledTimes(1); + + stopJobInvocationPolling(ref); + jest.advanceTimersByTime(5000); + + expect(apiGetSpy).toHaveBeenCalledTimes(1); + }); + + it('cancels pending timeout before starting a new poll when called multiple times', () => { + setupGetMock(runningData); + const store = makeStore(); + + store.dispatch(getJobInvocation(url, ref)); + expect(apiGetSpy).toHaveBeenCalledTimes(1); + + // Second call before the timeout fires cancels the first timeout and starts fresh + store.dispatch(getJobInvocation(url, ref)); + expect(apiGetSpy).toHaveBeenCalledTimes(2); + + jest.advanceTimersByTime(5000); + + // Only the second poll chain fires (first was cancelled) + expect(apiGetSpy).toHaveBeenCalledTimes(3); + }); + + it('keeps polling as long as the job is running', () => { + setupGetMock(runningData); + const store = makeStore(); + + store.dispatch(getJobInvocation(url, ref)); + jest.advanceTimersByTime(15000); + + expect(apiGetSpy).toHaveBeenCalledTimes(4); + }); + + it('cancels in-flight callback when new poll starts before previous resolves', () => { + let firstCallHandleSuccess; + let callCount = 0; + apiGetSpy = jest + .spyOn(APIActions, 'get') + .mockImplementation(({ handleSuccess, handleError }) => dispatch => { + callCount++; + if (callCount === 1) { + firstCallHandleSuccess = handleSuccess; + } + return dispatch({ type: 'MOCK_GET' }); + }); + + const store = makeStore(); + store.dispatch(getJobInvocation(url, ref)); + expect(apiGetSpy).toHaveBeenCalledTimes(1); + + // Start a new poll cycle before the first resolves + store.dispatch(getJobInvocation(url, ref)); + expect(apiGetSpy).toHaveBeenCalledTimes(2); + + // Old callback from first call tries to schedule next poll + firstCallHandleSuccess({ data: runningData }); + + // Timeout should not be set by the stale callback + expect(ref.current).toBeNull(); + }); +}); diff --git a/webpack/JobInvocationDetail/__tests__/MainInformation.test.js b/webpack/JobInvocationDetail/__tests__/MainInformation.test.js index 76ce24b39..1b293bdc1 100644 --- a/webpack/JobInvocationDetail/__tests__/MainInformation.test.js +++ b/webpack/JobInvocationDetail/__tests__/MainInformation.test.js @@ -30,7 +30,6 @@ import { CHANGE_ENABLED_RECURRING_LOGIC, GET_REPORT_TEMPLATES, GET_REPORT_TEMPLATE_INPUTS, - GET_TASK, JOB_INVOCATION_KEY, } from '../JobInvocationConstants'; @@ -47,7 +46,7 @@ jest.mock('../JobInvocationActions', () => { return { ...actual, getJobInvocation: jest.fn(() => () => undefined), - getTask: jest.fn(() => () => undefined), + stopJobInvocationPolling: jest.fn(), }; }); @@ -109,7 +108,6 @@ const createJobInvocationDetailState = ({ jobInvocation = jobInvocationData, jobInvocationStatus = STATUS.RESOLVED, jobInvocationError = null, - includeTaskState = true, } = {}) => { const jobInvocationResponse = jobInvocationError || JSON.parse(JSON.stringify(jobInvocation)); @@ -125,15 +123,6 @@ const createJobInvocationDetailState = ({ response: jobInvocationResponse, status: jobInvocationStatus, }, - ...(includeTaskState && { - [GET_TASK]: { - response: { - available_actions: { cancellable: true }, - ...(jobInvocationResponse.task || {}), - }, - status: STATUS.RESOLVED, - }, - }), }, }; }; @@ -168,11 +157,8 @@ describe('JobInvocationDetailPage', () => { setupApiMocks(); usePermissions.mockReturnValue(true); - const { getJobInvocation, getTask } = jest.requireMock( - '../JobInvocationActions' - ); + const { getJobInvocation } = jest.requireMock('../JobInvocationActions'); getJobInvocation.mockImplementation(() => () => undefined); - getTask.mockImplementation(() => () => undefined); }); afterEach(() => { @@ -189,7 +175,6 @@ describe('JobInvocationDetailPage', () => { renderJobInvocationDetailPage( createJobInvocationDetailState({ jobInvocation: scheduledJobInvocation, - includeTaskState: false, }), { jobId: jobInvocationDataScheduled.id } ); @@ -324,15 +309,7 @@ describe('JobInvocationDetailPage', () => { }); it('should dispatch global actions', async () => { - const actualActions = jest.requireActual('../JobInvocationActions'); - const { getTask } = jest.requireMock('../JobInvocationActions'); - - getTask.mockImplementation(taskId => dispatch => - actualActions.getTask(taskId)(dispatch) - ); - const jobId = jobInvocationDataRecurring.id; - const taskId = jobInvocationDataRecurring.task.id; const recurrenceId = jobInvocationDataRecurring.recurrence.id; const { store } = renderJobInvocationDetailPage( @@ -348,12 +325,6 @@ describe('JobInvocationDetailPage', () => { url: '/api/report_templates', }) ); - expect(api.get).toHaveBeenCalledWith( - expect.objectContaining({ - key: GET_TASK, - url: `/foreman_tasks/api/tasks/${taskId}`, - }) - ); expect(api.get).toHaveBeenCalledWith( expect.objectContaining({ key: GET_REPORT_TEMPLATE_INPUTS, @@ -367,9 +338,9 @@ describe('JobInvocationDetailPage', () => { store.dispatch(cancelJob(jobId, false)); store.dispatch(cancelJob(jobId, true)); - store.dispatch(enableRecurringLogic(recurrenceId, true, jobId)); - store.dispatch(enableRecurringLogic(recurrenceId, false, jobId)); - store.dispatch(cancelRecurringLogic(recurrenceId, jobId)); + store.dispatch(enableRecurringLogic(recurrenceId, true)); + store.dispatch(enableRecurringLogic(recurrenceId, false)); + store.dispatch(cancelRecurringLogic(recurrenceId)); expect(APIActions.post).toHaveBeenNthCalledWith( 1, diff --git a/webpack/JobInvocationDetail/__tests__/TemplateInvocationPolling.test.js b/webpack/JobInvocationDetail/__tests__/TemplateInvocationPolling.test.js new file mode 100644 index 000000000..4521e2688 --- /dev/null +++ b/webpack/JobInvocationDetail/__tests__/TemplateInvocationPolling.test.js @@ -0,0 +1,254 @@ +import React from 'react'; +import { createStore, applyMiddleware } from 'redux'; +import thunk from 'redux-thunk'; +import { Provider } from 'react-redux'; +import { render, act } from '@testing-library/react'; +import '@testing-library/jest-dom/extend-expect'; +import * as api from 'foremanReact/redux/API'; +import * as selectors from '../JobInvocationSelectors'; +import { TemplateInvocation } from '../TemplateInvocation'; +import { mockTemplateInvocationResponse } from './fixtures'; + +jest.spyOn(api, 'get'); +jest.mock('../JobInvocationSelectors'); + +jest.mock('foremanReact/components/ToastsList', () => ({ + addToast: jest.fn(payload => ({ type: 'ADD_TOAST', payload })), +})); + +describe('TemplateInvocation polling', () => { + const noop = () => {}; + const reducer = (state = {}) => state; + const makeStore = () => createStore(reducer, applyMiddleware(thunk)); + + const pollingProps = { + hostID: '1', + jobID: '1', + isInTableView: false, + isExpanded: true, + hostName: 'example-host', + hostProxy: { name: 'example-proxy', href: '#' }, + showOutputType: { stderr: true, stdout: true, debug: true }, + setShowOutputType: noop, + showTemplatePreview: false, + setShowTemplatePreview: noop, + showCommand: false, + setShowCommand: noop, + }; + + let apiGetSpy; + + beforeEach(() => { + jest.useFakeTimers({ legacyFakeTimers: true }); + selectors.selectTemplateInvocationStatus.mockImplementation(() => () => + 'RESOLVED' + ); + selectors.selectTemplateInvocation.mockImplementation(() => () => + mockTemplateInvocationResponse + ); + apiGetSpy = jest + .spyOn(api.APIActions, 'get') + .mockImplementation(({ handleSuccess }) => { + handleSuccess && + handleSuccess({ data: { finished: false, auto_refresh: true } }); + return { type: 'MOCK_GET' }; + }); + }); + + afterEach(() => { + jest.clearAllTimers(); + jest.useRealTimers(); + apiGetSpy.mockRestore(); + }); + + it('fetches on mount when isExpanded is true', () => { + const localStore = makeStore(); + render( + + + + ); + expect(apiGetSpy).toHaveBeenCalledTimes(1); + }); + + it('does not fetch on mount when isExpanded is false', () => { + const localStore = makeStore(); + render( + + + + ); + expect(apiGetSpy).not.toHaveBeenCalled(); + }); + + it('schedules next poll via setTimeout when auto_refresh is true and not finished', () => { + const localStore = makeStore(); + render( + + + + ); + expect(apiGetSpy).toHaveBeenCalledTimes(1); + + act(() => jest.advanceTimersByTime(5000)); + expect(apiGetSpy).toHaveBeenCalledTimes(2); + }); + + it('does not schedule next poll when finished is true', () => { + apiGetSpy.mockImplementation(({ handleSuccess }) => { + handleSuccess && + handleSuccess({ data: { finished: true, auto_refresh: true } }); + return { type: 'MOCK_GET' }; + }); + const localStore = makeStore(); + render( + + + + ); + expect(apiGetSpy).toHaveBeenCalledTimes(1); + + act(() => jest.advanceTimersByTime(5000)); + expect(apiGetSpy).toHaveBeenCalledTimes(1); + }); + + it('does not schedule next poll when auto_refresh is false', () => { + apiGetSpy.mockImplementation(({ handleSuccess }) => { + handleSuccess && + handleSuccess({ data: { finished: false, auto_refresh: false } }); + return { type: 'MOCK_GET' }; + }); + const localStore = makeStore(); + render( + + + + ); + expect(apiGetSpy).toHaveBeenCalledTimes(1); + + act(() => jest.advanceTimersByTime(5000)); + expect(apiGetSpy).toHaveBeenCalledTimes(1); + }); + + it('clears the polling timeout and sets cancelled on unmount', () => { + const localStore = makeStore(); + const { unmount } = render( + + + + ); + expect(apiGetSpy).toHaveBeenCalledTimes(1); + + unmount(); + act(() => jest.advanceTimersByTime(5000)); + expect(apiGetSpy).toHaveBeenCalledTimes(1); + }); + + it('cancels in-flight callback when unmounted before handleSuccess runs', () => { + let capturedHandleSuccess; + apiGetSpy.mockImplementation(({ handleSuccess }) => { + capturedHandleSuccess = handleSuccess; + return { type: 'MOCK_GET' }; + }); + + const localStore = makeStore(); + const { unmount } = render( + + + + ); + expect(apiGetSpy).toHaveBeenCalledTimes(1); + + unmount(); + + act(() => { + capturedHandleSuccess({ data: { finished: false, auto_refresh: true } }); + jest.advanceTimersByTime(5000); + }); + + expect(apiGetSpy).toHaveBeenCalledTimes(1); + }); + + it('re-fetches when isExpanded changes from false to true', () => { + const localStore = makeStore(); + const { rerender } = render( + + + + ); + expect(apiGetSpy).not.toHaveBeenCalled(); + + rerender( + + + + ); + expect(apiGetSpy).toHaveBeenCalledTimes(1); + }); + + it('does not re-fetch on expand when response is already finished', () => { + selectors.selectTemplateInvocation.mockImplementation(() => () => ({ + ...mockTemplateInvocationResponse, + finished: true, + })); + apiGetSpy.mockImplementation(({ handleSuccess }) => { + handleSuccess && + handleSuccess({ data: { finished: true, auto_refresh: false } }); + return { type: 'MOCK_GET' }; + }); + + const localStore = makeStore(); + const { rerender } = render( + + + + ); + expect(apiGetSpy).not.toHaveBeenCalled(); + + rerender( + + + + ); + // Selector already returns finished=true → guard skips dispatchFetch + expect(apiGetSpy).not.toHaveBeenCalled(); + + rerender( + + + + ); + rerender( + + + + ); + // Second expand: still finished → still no fetch + expect(apiGetSpy).not.toHaveBeenCalled(); + }); + + it('cancels existing poll and starts fresh when isExpanded changes', () => { + const localStore = makeStore(); + const { rerender } = render( + + + + ); + expect(apiGetSpy).toHaveBeenCalledTimes(1); + + rerender( + + + + ); + act(() => jest.advanceTimersByTime(5000)); + expect(apiGetSpy).toHaveBeenCalledTimes(1); + + rerender( + + + + ); + expect(apiGetSpy).toHaveBeenCalledTimes(2); + }); +}); diff --git a/webpack/JobInvocationDetail/__tests__/fixtures.js b/webpack/JobInvocationDetail/__tests__/fixtures.js index 9ad17a956..30523b33e 100644 --- a/webpack/JobInvocationDetail/__tests__/fixtures.js +++ b/webpack/JobInvocationDetail/__tests__/fixtures.js @@ -42,6 +42,7 @@ export const jobInvocationData = { id: '37ad5ead-51de-4798-bc73-a17687c4d5aa', state: 'stopped', started_at: '2024-01-01 12:34:56 +0100', + cancellable: true, }, template_invocations: [ { @@ -113,6 +114,7 @@ export const jobInvocationDataRecurring = { task: { id: '37ad5ead-51de-4798-bc73-a17687c4d5aa', state: 'scheduled', + cancellable: true, }, mode: 'recurring', recurrence: { diff --git a/webpack/JobInvocationDetail/index.js b/webpack/JobInvocationDetail/index.js index a4260516c..d940bb797 100644 --- a/webpack/JobInvocationDetail/index.js +++ b/webpack/JobInvocationDetail/index.js @@ -5,13 +5,12 @@ import { PageSectionVariants, Skeleton, } from '@patternfly/react-core'; -import React, { useEffect, useMemo, useState } from 'react'; +import React, { useEffect, useMemo, useRef, useState } from 'react'; import { translate as __, documentLocale } from 'foremanReact/common/I18n'; 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 { STATUS as API_STATUS } from 'foremanReact/constants'; import { selectAPIErrorMessage, @@ -25,12 +24,15 @@ import JobInvocationOverview from './JobInvocationOverview'; import JobInvocationSystemStatusChart from './JobInvocationSystemStatusChart'; import JobInvocationEmptyState from './JobInvocationEmptyState'; import JobInvocationToolbarButtons from './JobInvocationToolbarButtons'; -import { getJobInvocation, getTask } from './JobInvocationActions'; +import { + getJobInvocation, + stopJobInvocationPolling, + isJobFinished, +} from './JobInvocationActions'; import './JobInvocationDetail.scss'; import { DATE_OPTIONS, JOB_INVOCATION_KEY, - STATUS, STATUS_UPPERCASE, } from './JobInvocationConstants'; import { selectItems } from './JobInvocationSelectors'; @@ -50,11 +52,8 @@ 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; + const finished = isJobFinished(statusLabel); + const pollTimeoutRef = useRef(null); const jobInvocationApiStatus = useSelector(state => selectAPIStatus(state, JOB_INVOCATION_KEY) ); @@ -82,21 +81,11 @@ const JobInvocationDetailPage = ({ } useEffect(() => { - dispatch(getJobInvocation(`/api/job_invocations/${id}`)); - if (finished && !autoRefresh) { - dispatch(stopInterval(JOB_INVOCATION_KEY)); - } + dispatch(getJobInvocation(`/api/job_invocations/${id}`, pollTimeoutRef)); return () => { - dispatch(stopInterval(JOB_INVOCATION_KEY)); + stopJobInvocationPolling(pollTimeoutRef); }; - }, [dispatch, id, finished, autoRefresh]); - - const taskId = task?.id; - useEffect(() => { - if (taskId !== undefined) { - dispatch(getTask(`${taskId}`)); - } - }, [dispatch, taskId]); + }, [dispatch, id]); const apiFailed = jobInvocationApiStatus === API_STATUS.ERROR;