Fixes #38899 - Enable react-hooks/exhaustive-deps eslint rule

webpack/JobInvocationDetail/JobInvocationHostTable.js

@@ -24,7 +24,13 @@ import TableIndexPage from 'foremanReact/components/PF4/TableIndexPage/TableInde
 import { getControllerSearchProps } from 'foremanReact/constants';
 import { Icon } from 'patternfly-react';
 import PropTypes from 'prop-types';
-import React, { useEffect, useMemo, useState, useRef } from 'react';
+import React, {
+  useEffect,
+  useMemo,
+  useState,
+  useRef,
+  useCallback,
+} from 'react';
 import { FormattedMessage } from 'react-intl';
 import { useHistory } from 'react-router-dom';
 import { useForemanSettings } from 'foremanReact/Root/Context/ForemanContext';
@@ -93,19 +99,22 @@ const JobInvocationHostTable = ({
   });
 
   // Search filter
-  const constructFilter = (filter = initialFilter, search = urlSearchQuery) => {
-    const dropdownFilterClause =
-      filter && filter !== 'all_statuses'
-        ? `job_invocation.result = ${filter}`
-        : null;
-    const parts = [dropdownFilterClause, search];
-    return parts
-      .filter(x => x)
-      .map(fragment => `(${fragment})`)
-      .join(' AND ');
-  };
+  const constructFilter = useCallback(
+    (filter = initialFilter, search = urlSearchQuery) => {
+      const dropdownFilterClause =
+        filter && filter !== 'all_statuses'
+          ? `job_invocation.result = ${filter}`
+          : null;
+      const parts = [dropdownFilterClause, search];
+      return parts
+        .filter(x => x)
+        .map(fragment => `(${fragment})`)
+        .join(' AND ');
+    },
+    [initialFilter, urlSearchQuery]
+  );
 
-  const handleResponse = (data, key) => {
+  const handleResponse = useCallback((data, key) => {
     if (key === JOB_INVOCATION_HOSTS) {
       const ids = data.data.results.map(i => i.id);
 
@@ -114,76 +123,94 @@ const JobInvocationHostTable = ({
     }
 
     setStatus(STATUS_UPPERCASE.RESOLVED);
-  };
+  }, []);
 
   // Call hosts data with params
-  const makeApiCall = (requestParams, callParams = {}) => {
-    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),
-        errorToast: ({ response }) =>
-          response?.data?.error?.full_messages?.[0] || response,
-      })
-    );
-  };
+  const makeApiCall = useCallback(
+    (requestParams, callParams = {}) => {
+      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),
+          errorToast: ({ response }) =>
+            response?.data?.error?.full_messages?.[0] || response,
+        })
+      );
+    },
+    [dispatch, id, handleResponse]
+  );
 
-  const filterApiCall = newAPIOptions => {
-    const newParams = newAPIOptions?.params ?? newAPIOptions ?? {};
+  const filterApiCall = useCallback(
+    newAPIOptions => {
+      const newParams = newAPIOptions?.params ?? newAPIOptions ?? {};
 
-    const filterSearch = constructFilter(
-      initialFilter,
-      newParams.search ?? urlSearchQuery
-    );
-
-    const finalParams = {
-      ...defaultParams,
-      ...newParams,
-    };
-
-    if (filterSearch === AWAITING_STATUS_FILTER) {
-      finalParams.awaiting = 'true';
-    } else if (filterSearch !== '') {
-      finalParams.search = filterSearch;
-    }
+      const filterSearch = constructFilter(
+        initialFilter,
+        newParams.search ?? urlSearchQuery
+      );
 
-    makeApiCall(finalParams, { key: JOB_INVOCATION_HOSTS });
+      const finalParams = {
+        ...defaultParams,
+        ...newParams,
+      };
 
-    const urlSearchParams = new URLSearchParams(window.location.search);
+      if (filterSearch === AWAITING_STATUS_FILTER) {
+        finalParams.awaiting = 'true';
+      } else if (filterSearch !== '') {
+        finalParams.search = filterSearch;
+      }
 
-    ['page', 'per_page', 'order'].forEach(key => {
-      if (finalParams[key]) urlSearchParams.set(key, finalParams[key]);
-    });
+      makeApiCall(finalParams, { key: JOB_INVOCATION_HOSTS });
 
-    history.push({ search: urlSearchParams.toString() });
-  };
+      const urlSearchParams = new URLSearchParams(window.location.search);
+
+      ['page', 'per_page', 'order'].forEach(key => {
+        if (finalParams[key]) urlSearchParams.set(key, finalParams[key]);
+      });
+
+      history.push({ search: urlSearchParams.toString() });
+    },
+    [
+      initialFilter,
+      urlSearchQuery,
+      defaultParams,
+      makeApiCall,
+      history,
+      constructFilter,
+    ]
+  );
 
   // Filter change
-  const handleFilterChange = newFilter => {
-    onFilterUpdate(newFilter);
-  };
+  const handleFilterChange = useCallback(
+    newFilter => {
+      onFilterUpdate(newFilter);
+    },
+    [onFilterUpdate]
+  );
 
   // Effects
   // run after mount
+  const initializedRef = useRef(false);
   useEffect(() => {
-    // Job Invo template load
-    makeApiCall(
-      {},
-      {
-        url: `/job_invocations/${id}/hosts`,
-        key: LIST_TEMPLATE_INVOCATIONS,
+    if (!initializedRef.current) {
+      // Job Invo template load
+      makeApiCall(
+        {},
+        {
+          url: `/job_invocations/${id}/hosts`,
+          key: LIST_TEMPLATE_INVOCATIONS,
+        }
+      );
+
+      if (initialFilter === '') {
+        onFilterUpdate('all_statuses');
       }
-    );
-
-    if (initialFilter === '') {
-      onFilterUpdate('all_statuses');
+      initializedRef.current = true;
     }
-
-    // eslint-disable-next-line react-hooks/exhaustive-deps
-  }, []);
+  }, [makeApiCall, id, initialFilter, onFilterUpdate]);
 
   useEffect(() => {
     if (initialFilter !== '') filterApiCall();
@@ -192,8 +219,7 @@ const JobInvocationHostTable = ({
       prevStatusLabel.current = statusLabel;
       filterApiCall();
     }
-    // eslint-disable-next-line react-hooks/exhaustive-deps
-  }, [initialFilter, statusLabel, id]);
+  }, [initialFilter, statusLabel, id, filterApiCall]);
 
   const {
     updateSearchQuery: updateSearchQueryBulk,

webpack/JobInvocationDetail/index.js

@@ -79,15 +79,14 @@ const JobInvocationDetailPage = ({
     return () => {
       dispatch(stopInterval(JOB_INVOCATION_KEY));
     };
-    // eslint-disable-next-line react-hooks/exhaustive-deps
   }, [dispatch, id, finished, autoRefresh]);
 
+  const taskId = task?.id;
   useEffect(() => {
-    if (task?.id !== undefined) {
-      dispatch(getTask(`${task?.id}`));
+    if (taskId !== undefined) {
+      dispatch(getTask(`${taskId}`));
     }
-    // eslint-disable-next-line react-hooks/exhaustive-deps
-  }, [dispatch, task?.id]);
+  }, [dispatch, taskId]);
 
   const pageStatus =
     items.id === undefined

webpack/JobWizard/JobWizard.js

@@ -89,9 +89,6 @@ export const JobWizard = ({ rerunData }) => {
         concurrency_control = {},
       },
     }) => {
-      if (category !== job_category) {
-        setCategory(job_category);
-      }
       const advancedTemplateValues = {};
       const defaultTemplateValues = {};
       const inputs = template_inputs;
@@ -131,8 +128,7 @@ export const JobWizard = ({ rerunData }) => {
         };
       });
     },
-    // eslint-disable-next-line react-hooks/exhaustive-deps
-    [category.length]
+    [setTemplateValues, setAdvancedValues]
   );
   useEffect(() => {
     if (rerunData) {
@@ -153,8 +149,7 @@ export const JobWizard = ({ rerunData }) => {
         },
       });
     }
-    // eslint-disable-next-line react-hooks/exhaustive-deps
-  }, [rerunData]);
+  }, [rerunData, setDefaults]);
   useEffect(() => {
     if (jobTemplateID) {
       dispatch(
@@ -199,8 +194,14 @@ export const JobWizard = ({ rerunData }) => {
         })
       );
     }
-    // eslint-disable-next-line react-hooks/exhaustive-deps
-  }, [rerunData, jobTemplateID, dispatch]);
+  }, [
+    rerunData,
+    jobTemplateID,
+    dispatch,
+    setDefaults,
+    setTemplateValues,
+    setAdvancedValues,
+  ]);
 
   const [isStartsBeforeError, setIsStartsBeforeError] = useState(false);
   const [isStartsAtError, setIsStartsAtError] = useState(false);
@@ -514,7 +515,7 @@ JobWizard.propTypes = {
     }),
     execution_timeout_interval: PropTypes.number,
     time_to_pickup: PropTypes.number,
-    remote_execution_feature_id: PropTypes.string,
+    remote_execution_feature_id: PropTypes.number,
     template_invocations: PropTypes.arrayOf(
       PropTypes.shape({
         template_id: PropTypes.number,

webpack/JobWizard/autofill.js

@@ -95,6 +95,14 @@ export const useAutoFill = ({
         });
       }
     }
-    // eslint-disable-next-line react-hooks/exhaustive-deps
-  }, [fills]);
+  }, [
+    fills,
+    setFills,
+    setSelectedTargets,
+    setHostsSearchQuery,
+    setJobTemplateID,
+    setTemplateValues,
+    setAdvancedValues,
+    dispatch,
+  ]);
 };

webpack/JobWizard/steps/AdvancedFields/__tests__/__snapshots__/AdvancedFields.test.js.snap

@@ -57,7 +57,7 @@ Array [
         "port": null,
         "preventInvalidHostname": false,
         "protocol": null,
-        "query": "resource=ForemanTasks%3A%3ATask&name=some+search",
+        "query": "resource=ForemanTasks%3A%3ATask",
         "urn": null,
         "username": null,
       },

webpack/JobWizard/steps/CategoryAndTemplate/index.js

@@ -55,8 +55,13 @@ const ConnectedCategoryAndTemplate = ({
         })
       );
     }
-    // eslint-disable-next-line react-hooks/exhaustive-deps
-  }, [jobCategoriesStatus]);
+  }, [
+    jobCategoriesStatus,
+    dispatch,
+    isCategoryPreselected,
+    setCategory,
+    setJobTemplate,
+  ]);
 
   const jobCategories = useSelector(selectJobCategories);
   const jobTemplatesSearch = useSelector(selectJobTemplatesSearch);
@@ -73,22 +78,29 @@ const ConnectedCategoryAndTemplate = ({
               per_page: 'all',
             }),
             handleSuccess: response => {
-              if (!jobTemplate)
-                setJobTemplate(
-                  current =>
-                    current ||
-                    Number(
-                      filterJobTemplates(response?.data?.results)[0]?.id
-                    ) ||
-                    null
-                );
+              const filteredTemplates = filterJobTemplates(
+                response?.data?.results
+              );
+              setJobTemplate(current => {
+                // Check if current template is in the new category's template list.
+                // This preserves the user's selection when changing categories on rerun,
+                // preventing the category from flashing and reverting back (Issue #38899).
+                // We check the state value (current) rather than the prop to avoid race conditions.
+                if (
+                  current &&
+                  filteredTemplates.some(template => template.id === current)
+                ) {
+                  return current;
+                }
+                // Otherwise, select the first template from the new category
+                return Number(filteredTemplates[0]?.id) || null;
+              });
             },
           })
         );
       }
     }
-    // eslint-disable-next-line react-hooks/exhaustive-deps
-  }, [category, dispatch]);
+  }, [category, dispatch, jobTemplatesSearch, setJobTemplate]);
 
   const jobTemplates = useSelector(selectJobTemplates);