Fix [Job Artifacts] artifacts tab fails to load for workflow job (#2995)

mlrun · Jan 8, 2025 · 396c30f · 396c30f
1 parent 8a82259
commit 396c30f
Show file tree

Hide file tree

Showing 8 changed files with 76 additions and 48 deletions.
diff --git a/src/components/DetailsArtifacts/DetailsArtifacts.js b/src/components/DetailsArtifacts/DetailsArtifacts.js
@@ -68,6 +68,7 @@ const DetailsArtifacts = ({
 
   const dispatch = useDispatch()
   const artifactsStore = useSelector(store => store.artifactsStore)
+  const isJobLoading = useSelector(store => Boolean(store.jobsStore.jobLoadingCounter))
 
   const showArtifact = useCallback(
     id => {
@@ -148,7 +149,7 @@ const DetailsArtifacts = ({
 
       if (workflowId) {
         return dispatch(
-          fetchJob({
+          fetchJob({ // todo remove this request in ML-8608 and use data from the selectedJob (it will be up to date)
             project: job.project || params.projectName,
             jobId: params.jobId,
             iter: iteration
@@ -166,39 +167,39 @@ const DetailsArtifacts = ({
             }
           }
         )
-      }
+      } else {
+        if (iteration) {
+          config.params.iter = iteration
+        }
 
-      if (iteration) {
-        config.params.iter = iteration
+        dispatch(
+          fetchArtifacts({
+            project: job.project || params.projectName,
+            filters: {},
+            config,
+            setRequestErrorMessage
+          })
+        )
+          .unwrap()
+          .then(result => {
+            if (result) {
+              setArtifactsPreviewContent(generateArtifactsPreviewContent(job, result))
+            }
+          })
       }
-
-      dispatch(
-        fetchArtifacts({
-          project: job.project || params.projectName,
-          filters: {},
-          config,
-          setRequestErrorMessage
-        })
-      )
-        .unwrap()
-        .then(result => {
-          if (result) {
-            setArtifactsPreviewContent(generateArtifactsPreviewContent(job, result))
-          }
-        })
     },
     [dispatch, params.jobId, params.projectName]
   )
 
   useEffect(() => {
-    if (params.jobId === selectedItem.uid || isDetailsPopUp) {
+    if ((params.jobId === selectedItem.uid || isDetailsPopUp) && !isJobLoading) {
       if (selectedItem.iterationStats?.length > 0 && iteration) {
         getJobArtifacts(selectedItem, iteration)
       } else if (selectedItem.iterationStats?.length === 0) {
         getJobArtifacts(selectedItem, null)
       }
     }
-  }, [getJobArtifacts, iteration, params.jobId, params.projectName, selectedItem, isDetailsPopUp])
+  }, [getJobArtifacts, iteration, params.jobId, params.projectName, selectedItem, isDetailsPopUp, isJobLoading])
 
   useEffect(() => {
     return () => {
@@ -208,7 +209,7 @@ const DetailsArtifacts = ({
   }, [params.jobId, params.projectName, selectedItem, iteration])
 
   return artifactsStore.loading ? (
-    <Loader />
+    <Loader section />
   ) : artifactsPreviewContent.length === 0 ? (
     <NoData message={requestErrorMessage} />
   ) : (

diff --git a/src/components/JobWizard/JobWizard.js b/src/components/JobWizard/JobWizard.js
@@ -545,6 +545,7 @@ const JobWizard = ({
             {(functionsStore.loading ||
               functionsStore.funcLoading ||
               jobsStore.loading ||
+              Boolean(jobsStore.jobLoadingCounter) ||
               projectIsLoading) && <Loader />}
           </>
         )

diff --git a/src/components/Jobs/Jobs.js b/src/components/Jobs/Jobs.js
@@ -143,7 +143,10 @@ const Jobs = () => {
     return defaultsDeep(
       {
         [MONITOR_JOBS_TAB]: {
-          handleRefresh: handleRefreshJobs
+          handleRefresh: (...args) => {
+            setSelectedJob({})
+            handleRefreshJobs(...args)
+          }
         },
         [MONITOR_WORKFLOWS_TAB]: {
           handleRefresh: getWorkflows
@@ -231,7 +234,11 @@ const Jobs = () => {
                     onClick: () => handleMonitoring(selectedJob, true)
                   }
                 ]}
-                autoRefreshIsStopped={jobWizardIsOpened || jobsStore.loading}
+                autoRefreshIsStopped={
+                  jobWizardIsOpened ||
+                  jobsStore.loading ||
+                  Boolean(jobsStore.jobLoadingCounter)
+                }
                 autoRefreshStopTrigger={!isEmpty(selectedJob)}
                 filters={filters}
                 filtersConfig={initialTabData[selectedTab].filtersConfig}
@@ -292,10 +299,8 @@ const Jobs = () => {
               >
                 <Outlet />
               </JobsContext.Provider>
-              {(jobsStore.loading ||
-                workflowsStore.workflows.loading ||
+              {(Boolean(jobsStore.jobLoadingCounter) ||
                 workflowsStore.activeWorkflow.loading ||
-                functionsStore.loading ||
                 functionsStore.funcLoading) && <Loader />}
             </div>
           </div>

diff --git a/src/components/ProjectsJobsMonitoring/ProjectsJobsMonitoring.js b/src/components/ProjectsJobsMonitoring/ProjectsJobsMonitoring.js
@@ -30,6 +30,7 @@ import ActionBar from '../ActionBar/ActionBar'
 import JobsMonitoringFilters from './JobsMonitoring/JobsMonitoringFilters'
 import ScheduledMonitoringFilters from './ScheduledMonitoring/ScheduledMonitoringFilters'
 import WorkflowsMonitoringFilters from './WorkflowsMonitoring/WorkflowsMonitoringFilters'
+import Loader from '../../common/Loader/Loader'
 
 import { STATS_TOTAL_CARD, tabs } from './projectsJobsMotinoring.util'
 import {
@@ -67,6 +68,8 @@ const ProjectsJobsMonitoring = () => {
   const navigate = useNavigate()
   const artifactsStore = useSelector(store => store.artifactsStore)
   const jobsStore = useSelector(store => store.jobsStore)
+  const workflowsStore = useSelector(store => store.workflowsStore)
+  const functionsStore = useSelector(store => store.functionsStore)
 
   const jobsFiltersConfig = useMemo(
     () => getJobsFiltersConfig(params.jobName, true),
@@ -145,7 +148,10 @@ const ProjectsJobsMonitoring = () => {
     return defaultsDeep(
       {
         [JOBS_MONITORING_JOBS_TAB]: {
-          handleRefresh: handleRefreshJobs
+          handleRefresh: (...args) => {
+            setSelectedJob({})
+            handleRefreshJobs(...args)
+          }
         },
         [JOBS_MONITORING_WORKFLOWS_TAB]: {
           handleRefresh: getWorkflows
@@ -182,7 +188,10 @@ const ProjectsJobsMonitoring = () => {
               <ActionBar
                 autoRefreshIsEnabled={selectedTab === JOBS_MONITORING_JOBS_TAB}
                 autoRefreshIsStopped={
-                  jobWizardIsOpened || jobsStore.loading || !isEmpty(selectedJob)
+                  jobWizardIsOpened ||
+                  jobsStore.loading ||
+                  Boolean(jobsStore.jobLoadingCounter) ||
+                  !isEmpty(selectedJob)
                 }
                 filters={filters}
                 filtersConfig={tabData[selectedTab].filtersConfig}
@@ -242,6 +251,9 @@ const ProjectsJobsMonitoring = () => {
               >
                 <Outlet />
               </ProjectJobsMonitoringContext.Provider>
+              {(Boolean(jobsStore.jobLoadingCounter) ||
+                workflowsStore.activeWorkflow.loading ||
+                functionsStore.funcLoading) && <Loader />}
             </div>
           </div>
         )}

diff --git a/src/components/ProjectsJobsMonitoring/WorkflowsMonitoring/WorkflowsMonitoring.js b/src/components/ProjectsJobsMonitoring/WorkflowsMonitoring/WorkflowsMonitoring.js
@@ -24,7 +24,6 @@ import { isEmpty } from 'lodash'
 
 import WorkflowsTable from '../../../elements/WorkflowsTable/WorkflowsTable'
 import { ProjectJobsMonitoringContext } from '../ProjectsJobsMonitoring'
-import Loader from '../../../common/Loader/Loader'
 
 import {
   JOBS_MONITORING_PAGE,
@@ -46,8 +45,6 @@ const WorkflowsMonitoring = ({ fetchFunctionLogs }) => {
   const [itemIsSelected, setItemIsSelected] = useState(false)
   const [selectedJob, setSelectedJob] = useState({})
   const workflowsStore = useSelector(state => state.workflowsStore)
-  const jobIsLoading = useSelector(store => store.jobsStore.loading)
-  const funcIsLoading = useSelector(store => store.functionsStore.funcLoading)
   const params = useParams()
   const dispatch = useDispatch()
   const { isStagingMode } = useMode()
@@ -109,7 +106,6 @@ const WorkflowsMonitoring = ({ fetchFunctionLogs }) => {
 
   return (
     <>
-      {(jobIsLoading || funcIsLoading || workflowsStore.activeWorkflow.loading) && <Loader />}
       <WorkflowsTable
         backLink={`/projects/*/${JOBS_MONITORING_PAGE}/${JOBS_MONITORING_WORKFLOWS_TAB}${window.location.search}`}
         context={ProjectJobsMonitoringContext}

diff --git a/src/elements/JobsTable/JobsTable.js b/src/elements/JobsTable/JobsTable.js
@@ -17,7 +17,7 @@ illegal under applicable law, and the grant of the foregoing license
 under the Apache 2.0 license is conditioned upon your compliance with
 such restriction.
 */
-import React, { useMemo, useCallback, useEffect } from 'react'
+import React, { useMemo, useCallback, useEffect, useLayoutEffect } from 'react'
 import PropTypes from 'prop-types'
 import { isEmpty } from 'lodash'
 import { useDispatch, useSelector } from 'react-redux'
@@ -311,7 +311,7 @@ const JobsTable = React.forwardRef(
       setJobWizardMode
     ])
 
-    useEffect(() => {
+    useLayoutEffect(() => {
       checkForSelectedJob(
         paginatedJobs,
         params.jobName,

diff --git a/src/elements/WorkflowsTable/WorkflowsTable.js b/src/elements/WorkflowsTable/WorkflowsTable.js
@@ -687,10 +687,10 @@ const WorkflowsTable = React.forwardRef(
     return (
       <>
         {workflowsStore.workflows.loading && <Loader />}
-        {(!workflowsStore.workflows.loading &&
-          !params.workflowId &&
-          workflowsStore.workflows.data.length === 0) ||
-        requestErrorMessage ? (
+        {workflowsStore.workflows.loading ? null : (!workflowsStore.workflows.loading &&
+            !params.workflowId &&
+            workflowsStore.workflows.data.length === 0) ||
+          requestErrorMessage ? (
           <NoData
             message={getNoDataMessage(
               filters,

diff --git a/src/reducers/jobReducer.js b/src/reducers/jobReducer.js
@@ -30,6 +30,7 @@ import { getNewJobErrorMsg } from '../components/JobWizard/JobWizard.util'
 const initialState = {
   jobsData: [],
   job: {},
+  jobLoadingCounter: 0,
   jobFunc: {},
   jobRuns: [],
   jobs: [],
@@ -313,27 +314,39 @@ const jobsSlice = createSlice({
       state.loading = false
     })
     builder.addCase(fetchAllJobRuns.rejected, hideLoading)
-    builder.addCase(fetchJob.pending, showLoading)
+    builder.addCase(fetchJob.pending, (state, action) => {
+      state.jobLoadingCounter++
+    })
     builder.addCase(fetchJob.fulfilled, (state, action) => {
       state.error = null
       state.job = action.payload
-      state.loading = false
+      state.jobLoadingCounter--
+    })
+    builder.addCase(fetchJob.rejected, (state, action) => {
+      state.jobLoadingCounter--
+    })
+    builder.addCase(fetchJobFunction.pending, (state, action) => {
+      state.jobLoadingCounter++
     })
-    builder.addCase(fetchJob.rejected, hideLoading)
-    builder.addCase(fetchJobFunction.pending, showLoading)
     builder.addCase(fetchJobFunction.fulfilled, (state, action) => {
       state.error = null
       state.jobFunc = action.payload
-      state.loading = false
+      state.jobLoadingCounter--
+    })
+    builder.addCase(fetchJobFunction.rejected, (state, action) => {
+      state.jobLoadingCounter--
+    })
+    builder.addCase(fetchJobFunctions.rejected, (state, action) => {
+      state.jobLoadingCounter--
+    })
+    builder.addCase(fetchJobFunctions.pending, (state, action) => {
+      state.jobLoadingCounter++
     })
-    builder.addCase(fetchJobFunction.rejected, hideLoading)
-    builder.addCase(fetchJobFunctions.pending, showLoading)
     builder.addCase(fetchJobFunctions.fulfilled, (state, action) => {
       state.error = null
       state.jobFunc = action.payload
-      state.loading = false
+      state.jobLoadingCounter--
     })
-    builder.addCase(fetchJobFunctions.rejected, hideLoading)
     builder.addCase(fetchJobLogs.pending, (state, action) => {
       state.logs.loading = true
       state.logs.error = null