diff --git a/app/controllers/api/v2/job_invocations_controller.rb b/app/controllers/api/v2/job_invocations_controller.rb index 0bf640111..ba22257fa 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,22 @@ 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 + can_cancel_job_invocations = authorized_for(:permission => :cancel_job_invocations, :auth_object => @job_invocation) + can_create_job_invocations = authorized_for(controller: :job_invocations, action: :create) + can_execute_on_infra_hosts = User.current.can?(:execute_jobs_on_infrastructure_hosts) + @permissions_by_host = hosts.to_h do |host| + task = @task_by_host[host.id] + [host.id, { + :view_foreman_tasks => task && authorized_for(:permission => :view_foreman_tasks, :auth_object => task), + :cancel_job_invocations => can_cancel_job_invocations, + :execute_jobs => can_create_job_invocations && (!host.infrastructure_host? || can_execute_on_infra_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/test/functional/api/v2/job_invocations_controller_test.rb b/test/functional/api/v2/job_invocations_controller_test.rb index 2b9d84e2b..1008f8f4d 100644 --- a/test/functional/api/v2/job_invocations_controller_test.rb +++ b/test/functional/api/v2/job_invocations_controller_test.rb @@ -67,6 +67,14 @@ class JobInvocationsControllerTest < ActionController::TestCase assert_nil result['template_invocations'] end + test 'should include cancellable field in task node' do + get :show, params: { :id => @invocation.id } + assert_response :success + result = ActiveSupport::JSON.decode(@response.body) + assert result.key?('task'), 'task node should be present in show response' + assert result['task'].key?('cancellable'), 'cancellable field should be present in task node' + end + test 'should include job_status per host when host_status=true' do invocation = FactoryBot.create(:job_invocation, :with_template, :with_task) invocation.template_invocations << FactoryBot.create(:template_invocation, :with_task, :with_host, :job_invocation => invocation) @@ -284,19 +292,67 @@ class JobInvocationsControllerTest < ActionController::TestCase end describe '#hosts' do + 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 + 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 or permissions nodes without include_permissions' do + get :hosts, params: { :id => @hosts_invocation.id } + assert_response :success + result = ActiveSupport::JSON.decode(@response.body) + assert_not_empty result['results'] + result['results'].each do |host| + assert_not host.key?('task'), 'task node should be absent when include_permissions is not set' + assert_not host.key?('permissions'), 'permissions node should be absent when include_permissions is not set' + end + end + + test 'should include task and permissions nodes when include_permissions=true' do + get :hosts, params: { :id => @hosts_invocation.id, :include_permissions => true } + assert_response :success + result = ActiveSupport::JSON.decode(@response.body) + assert_not_empty result['results'] + result['results'].each do |host| + assert host.key?('task'), 'task node should be present when include_permissions=true' + assert host.key?('permissions'), 'permissions node should be present when include_permissions=true' + assert host['task'].key?('id') + assert host['task'].key?('cancellable') + assert host['permissions'].key?('view_foreman_tasks') + assert host['permissions'].key?('cancel_job_invocations') + assert host['permissions'].key?('execute_jobs') + end + end + + test 'should handle nil run_host_job_task with include_permissions=true' do + # Create a host without a run_host_job_task (template invocation without :with_task) 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) } + ti = FactoryBot.create(:template_invocation, :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.targeting.hosts = [ti.host] invocation.save! - get :hosts, params: { :id => invocation.id, :page => 2, :per_page => 1 } + get :hosts, params: { :id => invocation.id, :include_permissions => true } 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'] }) + assert_not_empty result['results'] + host_result = result['results'].first + assert host_result.key?('task'), 'task node should be present' + assert_nil host_result['task'], 'task should be nil when run_host_job_task is absent' + assert host_result.key?('permissions'), 'permissions node should be present' + refute host_result['permissions']['view_foreman_tasks'] end end diff --git a/webpack/JobInvocationDetail/JobInvocationActions.js b/webpack/JobInvocationDetail/JobInvocationActions.js index 131696068..d523226fb 100644 --- a/webpack/JobInvocationDetail/JobInvocationActions.js +++ b/webpack/JobInvocationDetail/JobInvocationActions.js @@ -1,52 +1,57 @@ 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( +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 = {}, pollTimeoutRef = { current: null }) => { + dispatch( get({ key: JOB_INVOCATION_KEY, - params: { include_permissions: true, include_hosts: false }, + params: { include_hosts: false, include_permissions: true, ...params }, url, + handleSuccess: ({ data }) => { + if (!isJobFinished(data.status_label)) { + pollTimeoutRef.current = setTimeout( + () => fetchJobInvocation(dispatch, url, {}, pollTimeoutRef), + 5000 + ); + } else { + pollTimeoutRef.current = null; + } + }, handleError: () => { - dispatch(stopInterval(JOB_INVOCATION_KEY)); + pollTimeoutRef.current = 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, pollTimeoutRef) => dispatch => { + stopJobInvocationPolling(pollTimeoutRef); + fetchJobInvocation(dispatch, url, { include_permissions: true }, pollTimeoutRef); }; -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 => { @@ -56,8 +61,8 @@ export const cancelJob = (jobId, force) => dispatch => { : 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); + ? sprintf(__('Could not abort the job %s: %s'), jobId, response) + : sprintf(__('Could not cancel the job %s: %s'), jobId, response); const url = force ? `/job_invocations/${jobId}/cancel?force=true` : `/job_invocations/${jobId}/cancel`; @@ -66,13 +71,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,21 +80,11 @@ 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, @@ -122,19 +111,15 @@ 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)), + handleSuccess: () => { + fetchJobInvocation(dispatch, `/api/job_invocations/${jobId}`); + }, }) ); }; -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 +133,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 12d6deb11..51010b6af 100644 --- a/webpack/JobInvocationDetail/JobInvocationConstants.js +++ b/webpack/JobInvocationDetail/JobInvocationConstants.js @@ -7,8 +7,8 @@ import JobStatusIcon from '../react_app/components/RecentJobsCard/JobStatusIcon' export const JOB_INVOCATION_KEY = 'JOB_INVOCATION_KEY'; export const UPDATE_JOB = 'UPDATE_JOB'; +export const CURRENT_PERMISSIONS = 'CURRENT_PERMISSIONS'; 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,13 +17,10 @@ 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 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..9be1259f5 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,16 @@ const JobInvocationHostTable = ({ // Expansive items const [expandedHost, setExpandedHost] = useState(new Set()); - const prevStatusLabel = useRef(statusLabel); + const prevJobFinished = useRef(jobFinished); + const prevFilter = useRef(initialFilter); + const prevId = useRef(id); + const pollTimeoutId = useRef(null); + const currentPollParams = useRef({}); + const cachedPermissions = useRef({}); + const jobFinishedRef = useRef(jobFinished); + useEffect(() => { + jobFinishedRef.current = jobFinished; + }, [jobFinished]); const [hostInvocationStates, setHostInvocationStates] = useState({}); @@ -153,33 +160,60 @@ const JobInvocationHostTable = ({ [initialFilter, urlSearchQuery] ); - const handleResponse = useCallback((data, key) => { - if (key === JOB_INVOCATION_HOSTS) { - const ids = data.data.results.map(i => i.id); + const updateHostsState = useCallback((data, isPoll) => { + const { results } = data.data; - setApiResponse(data.data); - setAllHostsIds(ids); + if (!isPoll) { + cachedPermissions.current = Object.fromEntries( + results.map(result => [result.id, result.permissions]) + ); } + const mergedResults = results.map(result => ({ + ...result, + permissions: cachedPermissions.current[result.id], + })); + + const ids = mergedResults.map(i => i.id); + setApiResponse({ ...data.data, results: mergedResults }); + setAllHostsIds(ids); setStatus(STATUS_UPPERCASE.RESOLVED); }, []); // Call hosts data with params const makeApiCall = useCallback( - (requestParams, callParams = {}) => { + (requestParams, { isPoll = false } = {}) => { 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: { + ...(!isPoll && { include_permissions: true }), + ...requestParams, + }, + handleSuccess: data => { + updateHostsState(data, isPoll); + if (!jobFinishedRef.current) { + pollTimeoutId.current = setTimeout( + () => makeApiCall(currentPollParams.current, { isPoll: true }), + 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 +236,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 +260,37 @@ 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(); - - if (statusLabel !== prevStatusLabel.current) { - prevStatusLabel.current = statusLabel; + const filterChanged = initialFilter !== prevFilter.current; + const statusChanged = jobFinished !== prevJobFinished.current; + const idChanged = id !== prevId.current; + + if ((filterChanged || statusChanged || idChanged) && initialFilter !== '') { + prevFilter.current = initialFilter; + prevJobFinished.current = jobFinished; + prevId.current = id; filterApiCall(); } - }, [initialFilter, statusLabel, id, filterApiCall]); + }, [initialFilter, jobFinished, id, filterApiCall]); + + useEffect( + () => () => { + clearTimeout(pollTimeoutId.current); + }, + [] + ); const { updateSearchQuery: updateSearchQueryBulk, @@ -403,7 +435,7 @@ const JobInvocationHostTable = ({ , 0 ? fetchBulkParams() : null} @@ -472,7 +504,11 @@ const JobInvocationHostTable = ({ {columns[k].wrapper(result)} ))} - + {}, - statusLabel: undefined, + jobFinished: false, }; export default JobInvocationHostTable; diff --git a/webpack/JobInvocationDetail/JobInvocationSelectors.js b/webpack/JobInvocationDetail/JobInvocationSelectors.js index e43bfd208..26835ada9 100644 --- a/webpack/JobInvocationDetail/JobInvocationSelectors.js +++ b/webpack/JobInvocationDetail/JobInvocationSelectors.js @@ -5,18 +5,17 @@ import { } from 'foremanReact/redux/API/APISelectors'; 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}`); @@ -27,3 +26,21 @@ export const selectTemplateInvocationStatus = hostID => state => 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); + +export const selectHasPermission = permissionRequired => state => { + const status = selectAPIStatus(state, CURRENT_PERMISSIONS); + const selectCurrentPermissions = selectCurrentPermisions(state)?.results; + return status === APIStatus.RESOLVED + ? selectCurrentPermissions?.some( + permission => permission.name === permissionRequired + ) + : false; +}; diff --git a/webpack/JobInvocationDetail/JobInvocationToolbarButtons.js b/webpack/JobInvocationDetail/JobInvocationToolbarButtons.js index 5fadf906b..0e9805483 100644 --- a/webpack/JobInvocationDetail/JobInvocationToolbarButtons.js +++ b/webpack/JobInvocationDetail/JobInvocationToolbarButtons.js @@ -136,7 +136,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" @@ -154,7 +154,7 @@ const JobInvocationToolbarButtons = ({ jobId, data }) => { - dispatch(cancelRecurringLogic(recurrence?.id, jobId)) + dispatch(cancelRecurringLogic(recurrence?.id)) } key="cancel-recurring" component="button" @@ -168,7 +168,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..ca1616f36 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,48 @@ export const TemplateInvocation = ({ const response = useSelector(selectTemplateInvocation(hostID)); const dispatch = useDispatch(); + const timeoutRef = useRef(null); const responseRef = useRef(response); - useEffect(() => { - responseRef.current = response; - }, [response]); + responseRef.current = response; 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)) { + if (responseRef.current?.finished !== true) { 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); } 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..1ee915e46 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 = ({ @@ -81,11 +81,12 @@ const actions = ({ }, }); -export const RowActions = ({ hostID, jobID }) => { +export const RowActions = ({ hostID, jobID, permissions: permissionsProp }) => { const dispatch = useDispatch(); - const response = useSelector(selectTemplateInvocationList)?.[hostID]; - if (!response?.permissions) return null; - const { task, permissions } = response; + const response = useSelector(selectHostFromJobInvocationHosts(hostID)); + const permissions = permissionsProp ?? response?.permissions; + if (!permissions) return null; + const { task } = response || {}; const { id: taskID, cancellable: taskCancellable } = task || {}; const getActions = actions({ taskID, @@ -216,4 +217,13 @@ TemplateActionButtons.defaultProps = { RowActions.propTypes = { hostID: PropTypes.oneOfType([PropTypes.string, PropTypes.number]).isRequired, jobID: PropTypes.oneOfType([PropTypes.string, PropTypes.number]).isRequired, + permissions: PropTypes.shape({ + view_foreman_tasks: PropTypes.bool, + cancel_job_invocations: PropTypes.bool, + execute_jobs: PropTypes.bool, + }), +}; + +RowActions.defaultProps = { + permissions: null, }; diff --git a/webpack/JobInvocationDetail/__tests__/JobInvocationHostTablePolling.test.js b/webpack/JobInvocationDetail/__tests__/JobInvocationHostTablePolling.test.js new file mode 100644 index 000000000..5aa042d51 --- /dev/null +++ b/webpack/JobInvocationDetail/__tests__/JobInvocationHostTablePolling.test.js @@ -0,0 +1,317 @@ +import React from 'react'; +import { Provider } from 'react-redux'; +import { render, act } from '@testing-library/react'; +import '@testing-library/jest-dom/extend-expect'; +import { createStore, applyMiddleware } from 'redux'; +import thunk from 'redux-thunk'; +import * as api from 'foremanReact/redux/API'; +import JobInvocationHostTable from '../JobInvocationHostTable'; + +jest.mock( + 'foremanReact/components/PF4/TableIndexPage/TableIndexPage', + () => jest.fn(() =>
), + { virtual: true } +); +jest.mock( + 'foremanReact/components/PF4/TableIndexPage/Table/Table', + () => ({ Table: jest.fn(() =>
) }), + { virtual: true } +); +jest.mock( + 'foremanReact/components/PF4/TableIndexPage/Table/TableHooks', + () => ({ + useBulkSelect: jest.fn(() => ({ + updateSearchQuery: jest.fn(), + fetchBulkParams: jest.fn(() => ''), + inclusionSet: new Set(), + exclusionSet: new Set(), + selectAll: jest.fn(), + selectPage: jest.fn(), + selectNone: jest.fn(), + selectedCount: 0, + selectOne: jest.fn(), + areAllRowsOnPageSelected: jest.fn(() => false), + areAllRowsSelected: jest.fn(() => false), + isSelected: jest.fn(() => false), + })), + useUrlParams: jest.fn(() => ({})), + }), + { virtual: true } +); +jest.mock( + 'foremanReact/components/PF4/TableIndexPage/Table/helpers', + () => ({ getPageStats: jest.fn(() => ({ pageRowCount: 0 })) }), + { virtual: true } +); +jest.mock( + 'foremanReact/components/PF4/TableIndexPage/RowSelectTd', + () => ({ RowSelectTd: jest.fn(() => null) }), + { virtual: true } +); +jest.mock( + 'foremanReact/components/PF4/TableIndexPage/Table/SelectAllCheckbox', + () => jest.fn(() => null), + { virtual: true } +); +jest.mock('foremanReact/constants', () => ({ + getControllerSearchProps: jest.fn(() => ({ + autocomplete: { url: '' }, + })), +})); +jest.mock('foremanReact/Root/Context/ForemanContext', () => ({ + useForemanSettings: jest.fn(() => ({ perPage: 20 })), + useForemanHostDetailsPageUrl: jest.fn(() => '/new/hosts/'), +})); +jest.mock('react-router-dom', () => ({ + useHistory: jest.fn(() => ({ push: jest.fn() })), +})); +jest.mock('../CheckboxesActions', () => ({ + CheckboxesActions: jest.fn(() => null), +})); +jest.mock('../DropdownFilter', () => jest.fn(() => null)); +jest.mock('../JobInvocationConstants', () => ({ + __esModule: true, + default: jest.fn(() => ({})), + JOB_INVOCATION_HOSTS: 'JOB_INVOCATION_HOSTS', + AWAITING_STATUS_FILTER: '(job_invocation.result = N/A)', + STATUS_UPPERCASE: { + RESOLVED: 'RESOLVED', + ERROR: 'ERROR', + PENDING: 'PENDING', + }, + showTemplateInvocationUrl: jest.fn(() => ''), + templateInvocationPageUrl: jest.fn(() => ''), + GET_TEMPLATE_INVOCATION: 'GET_TEMPLATE_INVOCATION', +})); +jest.mock('../OpenAllInvocationsModal', () => ({ + PopupAlert: jest.fn(() => null), +})); + +const noop = () => {}; +const reducer = (state = {}) => state; +const makeStore = () => createStore(reducer, applyMiddleware(thunk)); + +const defaultProps = { + id: '42', + targeting: {}, + initialFilter: 'all_statuses', + jobFinished: false, + onFilterUpdate: noop, +}; + +describe('JobInvocationHostTable polling', () => { + let apiGetSpy; + + beforeEach(() => { + jest.useFakeTimers({ legacyFakeTimers: true }); + jest + .requireMock('foremanReact/components/PF4/TableIndexPage/TableIndexPage') + .mockClear(); + apiGetSpy = jest + .spyOn(api.APIActions, 'get') + .mockImplementation(({ handleSuccess, ...action }) => { + handleSuccess && + handleSuccess({ data: { results: [], total: 0, subtotal: 0 } }); + return { type: 'MOCK_GET', ...action }; + }); + }); + + afterEach(() => { + jest.clearAllTimers(); + jest.useRealTimers(); + apiGetSpy.mockRestore(); + }); + + it('starts polling when filter becomes non-empty', () => { + const store = makeStore(); + const { rerender } = render( + + + + ); + + expect(apiGetSpy).not.toHaveBeenCalled(); + + rerender( + + + + ); + + expect(apiGetSpy).toHaveBeenCalledTimes(1); + }); + + const renderAndTriggerFetch = (store, props = {}) => { + const result = render( + + + + ); + result.rerender( + + + + ); + return result; + }; + + it('schedules next poll via setTimeout when job is not finished', () => { + const store = makeStore(); + renderAndTriggerFetch(store); + + expect(apiGetSpy).toHaveBeenCalledTimes(1); + + act(() => jest.advanceTimersByTime(5000)); + + expect(apiGetSpy).toHaveBeenCalledTimes(2); + }); + + it('does not schedule next poll when job is finished', () => { + const store = makeStore(); + renderAndTriggerFetch(store, { jobFinished: true }); + + expect(apiGetSpy).toHaveBeenCalledTimes(1); + + act(() => jest.advanceTimersByTime(5000)); + + expect(apiGetSpy).toHaveBeenCalledTimes(1); + }); + + it('respects jobFinishedRef — stops polling when jobFinished prop becomes true between polls', () => { + const store = makeStore(); + const { rerender } = renderAndTriggerFetch(store, { jobFinished: false }); + + expect(apiGetSpy).toHaveBeenCalledTimes(1); + + // Changing jobFinished triggers statusChanged, so filterApiCall runs once more. + // But jobFinishedRef is now true, so that call does NOT schedule another poll. + rerender( + + + + ); + + expect(apiGetSpy).toHaveBeenCalledTimes(2); + + act(() => jest.advanceTimersByTime(5000)); + + // No further calls — polling has stopped. + expect(apiGetSpy).toHaveBeenCalledTimes(2); + }); + + it('clears the polling timeout on unmount', () => { + const store = makeStore(); + const { unmount } = renderAndTriggerFetch(store); + + expect(apiGetSpy).toHaveBeenCalledTimes(1); + unmount(); + + act(() => jest.advanceTimersByTime(5000)); + + expect(apiGetSpy).toHaveBeenCalledTimes(1); + }); + + describe('include_permissions behaviour', () => { + const hostWithPermissions = { + id: 1, + permissions: { + view_foreman_tasks: true, + cancel_job_invocations: true, + execute_jobs: true, + }, + }; + + it('sends include_permissions on the first (non-poll) call', () => { + const store = makeStore(); + renderAndTriggerFetch(store); + + expect(apiGetSpy.mock.calls[0][0].params).toMatchObject({ + include_permissions: true, + }); + }); + + it('omits include_permissions on poll calls', () => { + const store = makeStore(); + renderAndTriggerFetch(store); + + act(() => jest.advanceTimersByTime(5000)); + + expect(apiGetSpy.mock.calls[1][0].params).not.toHaveProperty( + 'include_permissions' + ); + }); + + it('preserves permissions from the initial call in subsequent poll responses', () => { + apiGetSpy.mockImplementation(({ handleSuccess, params }) => { + handleSuccess && + handleSuccess({ + data: { + results: params.include_permissions ? [hostWithPermissions] : [{ id: 1 }], + total: 1, + subtotal: 1, + }, + }); + return { type: 'MOCK_GET' }; + }); + + const store = makeStore(); + renderAndTriggerFetch(store); + + // Advance to the first poll — this call has no include_permissions, + // so the raw result has no permissions field. + act(() => jest.advanceTimersByTime(5000)); + + const MockTableIndexPage = jest.requireMock( + 'foremanReact/components/PF4/TableIndexPage/TableIndexPage' + ); + const replacementResponse = + MockTableIndexPage.mock.calls[ + MockTableIndexPage.mock.calls.length - 1 + ][0].replacementResponse; + + expect(replacementResponse.response.results[0].permissions).toEqual( + hostWithPermissions.permissions + ); + }); + }); + + it('uses the current id in the api url after id prop changes', () => { + const store = makeStore(); + renderAndTriggerFetch(store, { id: '42' }); + + expect(apiGetSpy).toHaveBeenCalledTimes(1); + expect(apiGetSpy.mock.calls[0][0].url).toContain('/42/'); + }); + + it('re-fetches when id prop changes', () => { + const store = makeStore(); + const { rerender } = renderAndTriggerFetch(store, { id: '42' }); + + expect(apiGetSpy).toHaveBeenCalledTimes(1); + expect(apiGetSpy.mock.calls[0][0].url).toContain('/42/'); + + rerender( + + + + ); + + expect(apiGetSpy).toHaveBeenCalledTimes(2); + expect(apiGetSpy.mock.calls[1][0].url).toContain('/99/'); + }); +}); diff --git a/webpack/JobInvocationDetail/__tests__/JobInvocationPolling.test.js b/webpack/JobInvocationDetail/__tests__/JobInvocationPolling.test.js new file mode 100644 index 000000000..3a7e07436 --- /dev/null +++ b/webpack/JobInvocationDetail/__tests__/JobInvocationPolling.test.js @@ -0,0 +1,187 @@ +import { createStore, applyMiddleware } from 'redux'; +import configureMockStore from 'redux-mock-store'; +import thunk from 'redux-thunk'; +import * as api from 'foremanReact/redux/API'; +import { + getJobInvocation, + stopJobInvocationPolling, + enableRecurringLogic, +} from '../JobInvocationActions'; +import { JOB_INVOCATION_KEY } from '../JobInvocationConstants'; + +jest.spyOn(api, 'get'); +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' }; + +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', () => { + let ref; + + beforeEach(() => { + ref = makeRef(); + api.get.mockReset(); + jest.clearAllTimers(); + }); + + it('sends include_permissions and include_hosts on the initial fetch', () => { + setupGetMock(succeededData); + const store = makeStore(); + + store.dispatch(getJobInvocation(url, ref)); + + 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 = makeStore(); + + store.dispatch(getJobInvocation(url, ref)); + expect(api.get).toHaveBeenCalledTimes(1); + + jest.advanceTimersByTime(5000); + expect(api.get).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(api.get).toHaveBeenCalledTimes(2); + expect(api.get.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(api.get).toHaveBeenCalledTimes(1); + + jest.advanceTimersByTime(5000); + expect(api.get).toHaveBeenCalledTimes(1); + }); + + it('stops polling when job fails', () => { + setupGetMock(failedData); + const store = makeStore(); + + store.dispatch(getJobInvocation(url, ref)); + jest.advanceTimersByTime(5000); + + expect(api.get).toHaveBeenCalledTimes(1); + }); + + it('stops polling when job is cancelled', () => { + setupGetMock(cancelledData); + const store = makeStore(); + + store.dispatch(getJobInvocation(url, ref)); + 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 = makeStore(); + + store.dispatch(getJobInvocation(url, ref)); + jest.advanceTimersByTime(5000); + + expect(api.get).toHaveBeenCalledTimes(1); + }); + + it('stopJobInvocationPolling cancels a pending timeout', () => { + setupGetMock(runningData); + const store = makeStore(); + + store.dispatch(getJobInvocation(url, ref)); + expect(api.get).toHaveBeenCalledTimes(1); + + stopJobInvocationPolling(ref); + jest.advanceTimersByTime(5000); + + expect(api.get).toHaveBeenCalledTimes(1); + }); + + it('getJobInvocation cancels any existing poll before starting a new one', () => { + setupGetMock(runningData); + const store = makeStore(); + + store.dispatch(getJobInvocation(url, ref)); + expect(api.get).toHaveBeenCalledTimes(1); + + // Second call before the timeout fires should cancel the pending poll + store.dispatch(getJobInvocation(url, ref)); + 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 = makeStore(); + + store.dispatch(getJobInvocation(url, ref)); + jest.advanceTimersByTime(15000); + + expect(api.get).toHaveBeenCalledTimes(4); + }); +}); + +describe('enableRecurringLogic', () => { + const mockStore = configureMockStore([thunk]); + + it('re-fetches job invocation after successful disable/enable', () => { + jest + .spyOn(api.APIActions, 'put') + .mockImplementation(({ handleSuccess, ...action }) => { + handleSuccess && handleSuccess(); + return { type: 'MOCK_PUT', ...action }; + }); + api.get.mockImplementation(({ ...action }) => ({ type: 'get', ...action })); + + const store = mockStore({}); + store.dispatch(enableRecurringLogic(1, true, 42)); + + const actions = store.getActions(); + const getAction = actions.find(a => a.key === JOB_INVOCATION_KEY); + expect(getAction).toBeDefined(); + expect(getAction.url).toBe('/api/job_invocations/42'); + + api.APIActions.put.mockRestore(); + api.get.mockReset(); + }); +}); 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/index.js b/webpack/JobInvocationDetail/index.js index 085aee8f9..7125bf209 100644 --- a/webpack/JobInvocationDetail/index.js +++ b/webpack/JobInvocationDetail/index.js @@ -5,19 +5,19 @@ 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, selectAPIHttpStatus, selectAPIStatus, } from 'foremanReact/redux/API/APISelectors'; +import { useAPI } from 'foremanReact/common/hooks/API/APIHooks'; import { JobAdditionInfo } from './JobAdditionInfo'; import JobInvocationHostTable from './JobInvocationHostTable'; @@ -25,13 +25,18 @@ import JobInvocationOverview from './JobInvocationOverview'; import JobInvocationSystemStatusChart from './JobInvocationSystemStatusChart'; import JobInvocationEmptyState from './JobInvocationEmptyState'; 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'; import { selectItems } from './JobInvocationSelectors'; @@ -42,6 +47,7 @@ const JobInvocationDetailPage = ({ history, }) => { const dispatch = useDispatch(); + const pollTimeoutRef = useRef(null); const items = useSelector(selectItems); const { description, @@ -50,11 +56,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; const jobInvocationApiStatus = useSelector(state => selectAPIStatus(state, JOB_INVOCATION_KEY) ); @@ -64,6 +65,9 @@ const JobInvocationDetailPage = ({ const jobInvocationHttpStatus = useSelector(state => selectAPIHttpStatus(state, JOB_INVOCATION_KEY) ); + useAPI('get', currentPermissionsUrl, { + key: CURRENT_PERMISSIONS, + }); const [selectedFilter, setSelectedFilter] = useState(''); const handleFilterChange = newFilter => { @@ -82,21 +86,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; @@ -216,10 +210,8 @@ const JobInvocationDetailPage = ({