Fixes #39253 - fix toolbar buttons remount

pondrejk · pondrejk · commit ec5e58ef70af · 2026-07-10T08:48:15.000-04:00
diff --git a/webpack/JobInvocationDetail/JobInvocationToolbarButtons.js b/webpack/JobInvocationDetail/JobInvocationToolbarButtons.js
@@ -1,5 +1,6 @@
+/* eslint-disable max-lines */
 import PropTypes from 'prop-types';
-import React, { useEffect, useState } from 'react';
+import React, { useCallback, useEffect, useMemo, useState } from 'react';
 import { useDispatch, useSelector } from 'react-redux';
 import { Button, Split, SplitItem } from '@patternfly/react-core';
 import { UndoIcon } from '@patternfly/react-icons';
@@ -44,21 +45,32 @@ const JobInvocationToolbarButtons = ({ jobId, data }) => {
   const [isActionOpen, setIsActionOpen] = useState(false);
   const [reportTemplateJobId, setReportTemplateJobId] = useState(undefined);
   const [templateInputId, setTemplateInputId] = useState(undefined);
-  const queryParams = new URLSearchParams({
-    [`report_template_report[input_values][${templateInputId}][value]`]: jobId,
-  });
   const dispatch = useDispatch();
+  const reportHref = useMemo(() => {
+    if (reportTemplateJobId === undefined || templateInputId === undefined) {
+      return undefined;
+    }
+    const queryParams = new URLSearchParams({
+      [`report_template_report[input_values][${templateInputId}][value]`]: jobId,
+    });
+    return foremanUrl(
+      `/templates/report_templates/${reportTemplateJobId}/generate?${queryParams.toString()}`
+    );
+  }, [jobId, reportTemplateJobId, templateInputId]);
 
-  const onActionFocus = () => {
+  const onActionFocus = useCallback(() => {
     const element = document.getElementById(
       `toggle-split-button-action-primary-${jobId}`
     );
-    element.focus();
-  };
-  const onActionSelect = () => {
+    if (element) {
+      element.focus();
+    }
+  }, [jobId]);
+  const onActionSelect = useCallback(() => {
     setIsActionOpen(false);
     onActionFocus();
-  };
+  }, [onActionFocus]);
+  const onActionToggle = useCallback((_event, val) => setIsActionOpen(val), []);
 
   useEffect(() => {
     let isMounted = true;
@@ -112,102 +124,151 @@ const JobInvocationToolbarButtons = ({ jobId, data }) => {
     };
   }, [dispatch, reportTemplateJobId]);
 
-  const recurrenceDropdownItems = recurrence
-    ? [
-        <DropdownSeparator ouiaId="dropdown-separator-1" key="separator-1" />,
-        <DropdownItem
-          ouiaId="change-enabled-recurring-dropdown-item"
-          onClick={() =>
-            dispatch(
-              enableRecurringLogic(recurrence?.id, recurringEnabled, jobId)
-            )
-          }
-          key="change-enabled-recurring"
-          component="button"
-          isDisabled={
-            recurrence?.id === undefined ||
-            recurrence?.state === 'cancelled' ||
-            !canEditRecurringLogic
-          }
-        >
-          {recurringEnabled ? __('Disable recurring') : __('Enable recurring')}
-        </DropdownItem>,
-        <DropdownItem
-          ouiaId="cancel-recurring-dropdown-item"
-          onClick={() => dispatch(cancelRecurringLogic(recurrence?.id, jobId))}
-          key="cancel-recurring"
-          component="button"
-          isDisabled={
-            recurrence?.id === undefined ||
-            recurrence?.state === 'cancelled' ||
-            !canEditRecurringLogic
-          }
-        >
-          {__('Cancel recurring')}
-        </DropdownItem>,
-      ]
-    : [];
+  const recurrenceDropdownItems = useMemo(
+    () =>
+      recurrence
+        ? [
+            <DropdownSeparator
+              ouiaId="dropdown-separator-1"
+              key="separator-1"
+            />,
+            <DropdownItem
+              ouiaId="change-enabled-recurring-dropdown-item"
+              onClick={() =>
+                dispatch(
+                  enableRecurringLogic(recurrence?.id, recurringEnabled, jobId)
+                )
+              }
+              key="change-enabled-recurring"
+              component="button"
+              isDisabled={
+                recurrence?.id === undefined ||
+                recurrence?.state === 'cancelled' ||
+                !canEditRecurringLogic
+              }
+            >
+              {recurringEnabled
+                ? __('Disable recurring')
+                : __('Enable recurring')}
+            </DropdownItem>,
+            <DropdownItem
+              ouiaId="cancel-recurring-dropdown-item"
+              onClick={() =>
+                dispatch(cancelRecurringLogic(recurrence?.id, jobId))
+              }
+              key="cancel-recurring"
+              component="button"
+              isDisabled={
+                recurrence?.id === undefined ||
+                recurrence?.state === 'cancelled' ||
+                !canEditRecurringLogic
+              }
+            >
+              {__('Cancel recurring')}
+            </DropdownItem>,
+          ]
+        : [],
+    [recurrence, recurringEnabled, canEditRecurringLogic, dispatch, jobId]
+  );
 
-  const dropdownItems = [
-    <DropdownItem
-      ouiaId="rerun-succeeded-dropdown-item"
-      href={foremanUrl(`/job_invocations/${jobId}/rerun?succeeded_only=1`)}
-      key="rerun-succeeded"
-      isDisabled={!canCreateJobInvocations || !(succeeded > 0)}
-      description="Rerun job on successful hosts"
-    >
-      {__('Rerun successful')}
-    </DropdownItem>,
-    <DropdownItem
-      ouiaId="rerun-failed-dropdown-item"
-      href={foremanUrl(`/job_invocations/${jobId}/rerun?failed_only=1`)}
-      key="rerun-failed"
-      isDisabled={!canCreateJobInvocations || !(failed > 0)}
-      description="Rerun job on failed hosts"
-    >
-      {__('Rerun failed')}
-    </DropdownItem>,
-    <DropdownItem
-      ouiaId="view-task-dropdown-item"
-      href={foremanUrl(`/foreman_tasks/tasks/${task?.id}`)}
-      key="view-task"
-      isDisabled={!canViewForemanTasks || task === undefined}
-      description="See details of latest task"
-    >
-      {__('View task')}
-    </DropdownItem>,
-    <DropdownSeparator ouiaId="dropdown-separator-0" key="separator-0" />,
-    <DropdownItem
-      ouiaId="cancel-dropdown-item"
-      onClick={() => dispatch(cancelJob(jobId, false))}
-      key="cancel"
-      component="button"
-      isDisabled={!canCancelJobInvocations || !isTaskCancelable}
-      description="Cancel job gracefully"
-    >
-      {__('Cancel')}
-    </DropdownItem>,
-    <DropdownItem
-      ouiaId="abort-dropdown-item"
-      onClick={() => dispatch(cancelJob(jobId, true))}
-      key="abort"
-      component="button"
-      isDisabled={!canCancelJobInvocations || !isTaskCancelable}
-      description="Cancel job immediately"
-    >
-      {__('Abort')}
-    </DropdownItem>,
-    ...recurrenceDropdownItems,
-    <DropdownSeparator ouiaId="dropdown-separator-2" key="separator-2" />,
-    <DropdownItem
-      ouiaId="legacy-ui-dropdown-item"
-      icon={<UndoIcon />}
-      href={`/legacy/job_invocations/${jobId}`}
-      key="legacy-ui"
-    >
-      {__('Legacy UI')}
-    </DropdownItem>,
-  ];
+  const dropdownItems = useMemo(
+    () => [
+      <DropdownItem
+        ouiaId="rerun-succeeded-dropdown-item"
+        href={foremanUrl(`/job_invocations/${jobId}/rerun?succeeded_only=1`)}
+        key="rerun-succeeded"
+        isDisabled={!canCreateJobInvocations || !(succeeded > 0)}
+        description="Rerun job on successful hosts"
+      >
+        {__('Rerun successful')}
+      </DropdownItem>,
+      <DropdownItem
+        ouiaId="rerun-failed-dropdown-item"
+        href={foremanUrl(`/job_invocations/${jobId}/rerun?failed_only=1`)}
+        key="rerun-failed"
+        isDisabled={!canCreateJobInvocations || !(failed > 0)}
+        description="Rerun job on failed hosts"
+      >
+        {__('Rerun failed')}
+      </DropdownItem>,
+      <DropdownItem
+        ouiaId="view-task-dropdown-item"
+        href={foremanUrl(`/foreman_tasks/tasks/${task?.id}`)}
+        key="view-task"
+        isDisabled={!canViewForemanTasks || task === undefined}
+        description="See details of latest task"
+      >
+        {__('View task')}
+      </DropdownItem>,
+      <DropdownSeparator ouiaId="dropdown-separator-0" key="separator-0" />,
+      <DropdownItem
+        ouiaId="cancel-dropdown-item"
+        onClick={() => dispatch(cancelJob(jobId, false))}
+        key="cancel"
+        component="button"
+        isDisabled={!canCancelJobInvocations || !isTaskCancelable}
+        description="Cancel job gracefully"
+      >
+        {__('Cancel')}
+      </DropdownItem>,
+      <DropdownItem
+        ouiaId="abort-dropdown-item"
+        onClick={() => dispatch(cancelJob(jobId, true))}
+        key="abort"
+        component="button"
+        isDisabled={!canCancelJobInvocations || !isTaskCancelable}
+        description="Cancel job immediately"
+      >
+        {__('Abort')}
+      </DropdownItem>,
+      ...recurrenceDropdownItems,
+      <DropdownSeparator ouiaId="dropdown-separator-2" key="separator-2" />,
+      <DropdownItem
+        ouiaId="legacy-ui-dropdown-item"
+        icon={<UndoIcon />}
+        href={`/legacy/job_invocations/${jobId}`}
+        key="legacy-ui"
+      >
+        {__('Legacy UI')}
+      </DropdownItem>,
+    ],
+    [
+      canCancelJobInvocations,
+      canCreateJobInvocations,
+      canViewForemanTasks,
+      dispatch,
+      failed,
+      isTaskCancelable,
+      jobId,
+      recurrenceDropdownItems,
+      succeeded,
+      task,
+    ]
+  );
+
+  const dropdownToggle = useMemo(
+    () => (
+      <DropdownToggle
+        ouiaId="toggle-button-action-primary"
+        id={`toggle-split-button-action-primary-${jobId}`}
+        splitButtonItems={[
+          <Button
+            component="a"
+            ouiaId="button-rerun-all"
+            key="rerun"
+            href={foremanUrl(`/job_invocations/${jobId}/rerun`)}
+            variant="control"
+            isDisabled={!canCreateJobInvocations}
+          >
+            {__(`Rerun all`)}
+          </Button>,
+        ]}
+        splitButtonVariant="action"
+        onToggle={onActionToggle}
+      />
+    ),
+    [canCreateJobInvocations, jobId, onActionToggle]
+  );
 
   return (
     <>
@@ -217,14 +278,12 @@ const JobInvocationToolbarButtons = ({ jobId, data }) => {
             component="a"
             ouiaId="button-create-report"
             className="button-create-report"
-            href={foremanUrl(
-              `/templates/report_templates/${reportTemplateJobId}/generate?${queryParams.toString()}`
-            )}
+            href={reportHref}
             variant="secondary"
             isDisabled={
               !canGenerateReportTemplates ||
               task?.state === STATUS.PENDING ||
-              templateInputId === undefined
+              reportHref === undefined
             }
           >
             {__(`Create report`)}
@@ -235,26 +294,7 @@ const JobInvocationToolbarButtons = ({ jobId, data }) => {
             ouiaId="job-invocation-global-actions-dropdown"
             onSelect={onActionSelect}
             position={DropdownPosition.right}
-            toggle={
-              <DropdownToggle
-                ouiaId="toggle-button-action-primary"
-                id={`toggle-split-button-action-primary-${jobId}`}
-                splitButtonItems={[
-                  <Button
-                    component="a"
-                    ouiaId="button-rerun-all"
-                    key="rerun"
-                    href={foremanUrl(`/job_invocations/${jobId}/rerun`)}
-                    variant="control"
-                    isDisabled={!canCreateJobInvocations}
-                  >
-                    {__(`Rerun all`)}
-                  </Button>,
-                ]}
-                splitButtonVariant="action"
-                onToggle={(_event, val) => setIsActionOpen(val)}
-              />
-            }
+            toggle={dropdownToggle}
             isOpen={isActionOpen}
             dropdownItems={dropdownItems}
           />
diff --git a/webpack/JobInvocationDetail/__tests__/MainInformation.test.js b/webpack/JobInvocationDetail/__tests__/MainInformation.test.js
@@ -211,6 +211,29 @@ describe('JobInvocationDetailPage', () => {
     ).toEqual(`/legacy/job_invocations/${jobId}`);
   });
 
+  it('keeps toolbar buttons mounted while job invocation data is refreshing', async () => {
+    const jobId = jobInvocationData.id;
+    const store = mockStore({
+      ...initialState,
+      JOB_INVOCATION_KEY: {
+        response: jobInvocationData,
+        status: STATUS.PENDING,
+      },
+    });
+
+    render(
+      <Provider store={store}>
+        <JobInvocationDetailPage
+          match={{ params: { id: `${jobId}` } }}
+          {...props}
+        />
+      </Provider>
+    );
+
+    expect(screen.getByText('Create report')).toBeInTheDocument();
+    expect(screen.getByText('Rerun all')).toBeInTheDocument();
+  });
+
   it('shows scheduled date', async () => {
     const store = mockStore(initialStateScheduled);
     render(
diff --git a/webpack/JobInvocationDetail/index.js b/webpack/JobInvocationDetail/index.js
@@ -153,7 +153,7 @@ const JobInvocationDetailPage = ({
         header={description}
         breadcrumbOptions={breadcrumbOptions}
         toolbarButtons={
-          jobInvocationApiStatus === API_STATUS.RESOLVED && (
+          items.id !== undefined && (
             <JobInvocationToolbarButtons jobId={id} data={items} />
           )
         }

Original file line number	Diff line number	Diff line change
`@@ -153,7 +153,7 @@ const JobInvocationDetailPage = ({`
`153`	`153`	`header={description}`
`154`	`154`	`breadcrumbOptions={breadcrumbOptions}`
`155`	`155`	`toolbarButtons={`
`156`		`- jobInvocationApiStatus === API_STATUS.RESOLVED && (`
	`156`	`+ items.id !== undefined && (`
`157`	`157`	`<JobInvocationToolbarButtons jobId={id} data={items} />`
`158`	`158`	`)`
`159`	`159`	`}`