From 77d896c3c0e1dc07809c35b3e542f1e609d4101c Mon Sep 17 00:00:00 2001
From: Mason Malone <mmalone@adobe.com>
Date: Thu, 12 Sep 2024 15:48:49 -0700
Subject: [PATCH] fix(ui): Submit button grayed out on details page. Fixes
 #13453

The React 18 upgrade in
https://github.com/argoproj/argo-workflows/pull/13069 introduced a bug
on the details page where the "Submit" button is disabled immediately on
page load. Specifically, I believe this is happening due to batching
changes that affect the following two `useEffect()` calls, which are
common to all the details pages modified in this PR:
```
    useEffect(() => {
        (async () => {
            try {
                const newEventSource = await services.eventSource.get(name, namespace);
                setEventSource(newEventSource);
                setEdited(false); // set back to false
                setError(null);
            } catch (err) {
                setError(err);
            }
        })();
    }, [name, namespace]);

    useEffect(() => setEdited(true), [eventSource]);
```

The first runs immediately and invokes `setEventSource(newEventSource)`,
which causes the second `useEffect()` call to be fired, since the
`eventSource` dependency has changed. Since both are invoking
`setEdited()`, this is basically a race condition, since the state of
`edited` is going to depend on whether these calls are batched together
are fired separately.

This PR fixes that by removing the second `useEffect()` call, which
eliminates the race condition. Instead, we only call `setEdited(true)`
when the editor is modified.

Signed-off-by: Mason Malone <mmalone@adobe.com>
---
 .../cluster-workflow-template-details.tsx                  | 7 +++++--
 ui/src/app/cron-workflows/cron-workflow-details.tsx        | 7 +++++--
 ui/src/app/event-sources/event-source-details.tsx          | 7 +++++--
 ui/src/app/sensors/sensor-details.tsx                      | 7 +++++--
 .../app/workflow-templates/workflow-template-details.tsx   | 7 +++++--
 5 files changed, 25 insertions(+), 10 deletions(-)

diff --git a/ui/src/app/cluster-workflow-templates/cluster-workflow-template-details.tsx b/ui/src/app/cluster-workflow-templates/cluster-workflow-template-details.tsx
index 1675e50ace64..542df148d78c 100644
--- a/ui/src/app/cluster-workflow-templates/cluster-workflow-template-details.tsx
+++ b/ui/src/app/cluster-workflow-templates/cluster-workflow-template-details.tsx
@@ -42,7 +42,10 @@ export function ClusterWorkflowTemplateDetails({history, location, match}: Route
         [history]
     );
 
-    useEffect(() => setEdited(true), [template]);
+    const editTemplate = (template: ClusterWorkflowTemplate) => {
+        setEdited(true);
+        setTemplate(template);
+    };
     useEffect(() => {
         history.push(historyUrl('cluster-workflow-templates/{name}', {name, sidePanel, tab}));
     }, [name, sidePanel, tab]);
@@ -132,7 +135,7 @@ export function ClusterWorkflowTemplateDetails({history, location, match}: Route
                 {!template ? (
                     <Loading />
                 ) : (
-                    <ClusterWorkflowTemplateEditor template={template} onChange={setTemplate} onError={setError} onTabSelected={setTab} selectedTabKey={tab} />
+                    <ClusterWorkflowTemplateEditor template={template} onChange={editTemplate} onError={setError} onTabSelected={setTab} selectedTabKey={tab} />
                 )}
             </>
             {template && (
diff --git a/ui/src/app/cron-workflows/cron-workflow-details.tsx b/ui/src/app/cron-workflows/cron-workflow-details.tsx
index 07017f23cc23..bba417175b6b 100644
--- a/ui/src/app/cron-workflows/cron-workflow-details.tsx
+++ b/ui/src/app/cron-workflows/cron-workflow-details.tsx
@@ -70,7 +70,10 @@ export function CronWorkflowDetails({match, location, history}: RouteComponentPr
             .catch(setError);
     }, [namespace, name]);
 
-    useEffect(() => setEdited(true), [cronWorkflow]);
+    const editCronWorkflow = (cronWorkflow: CronWorkflow) => {
+        setEdited(true);
+        setCronWorkflow(cronWorkflow);
+    };
 
     useEffect(() => {
         (async () => {
@@ -214,7 +217,7 @@ export function CronWorkflowDetails({match, location, history}: RouteComponentPr
                 {!cronWorkflow ? (
                     <Loading />
                 ) : (
-                    <CronWorkflowEditor cronWorkflow={cronWorkflow} onChange={setCronWorkflow} onError={setError} selectedTabKey={tab} onTabSelected={setTab} />
+                    <CronWorkflowEditor cronWorkflow={cronWorkflow} onChange={editCronWorkflow} onError={setError} selectedTabKey={tab} onTabSelected={setTab} />
                 )}
                 <SlidingPanel isShown={!!sidePanel} onClose={() => setSidePanel(null)}>
                     {sidePanel === 'share' && <WidgetGallery namespace={namespace} label={'workflows.argoproj.io/cron-workflow=' + name} />}
diff --git a/ui/src/app/event-sources/event-source-details.tsx b/ui/src/app/event-sources/event-source-details.tsx
index 0601dbd75421..cd932a53ccf0 100644
--- a/ui/src/app/event-sources/event-source-details.tsx
+++ b/ui/src/app/event-sources/event-source-details.tsx
@@ -78,7 +78,10 @@ export function EventSourceDetails({history, location, match}: RouteComponentPro
         })();
     }, [name, namespace]);
 
-    useEffect(() => setEdited(true), [eventSource]);
+    const editEventSource = (eventSource: EventSource) => {
+        setEventSource(eventSource);
+        setEdited(true);
+    };
 
     useCollectEvent('openedEventSourceDetails');
 
@@ -143,7 +146,7 @@ export function EventSourceDetails({history, location, match}: RouteComponentPro
                 {!eventSource ? (
                     <Loading />
                 ) : (
-                    <EventSourceEditor eventSource={eventSource} onChange={setEventSource} onError={setError} onTabSelected={setTab} selectedTabKey={tab} />
+                    <EventSourceEditor eventSource={eventSource} onChange={editEventSource} onError={setError} onTabSelected={setTab} selectedTabKey={tab} />
                 )}
             </>
             <SlidingPanel isShown={!!selected} onClose={() => setSelectedNode(null)}>
diff --git a/ui/src/app/sensors/sensor-details.tsx b/ui/src/app/sensors/sensor-details.tsx
index bbd14aef8f65..92852a3d2232 100644
--- a/ui/src/app/sensors/sensor-details.tsx
+++ b/ui/src/app/sensors/sensor-details.tsx
@@ -64,7 +64,10 @@ export function SensorDetails({match, location, history}: RouteComponentProps<an
             .catch(setError);
     }, [namespace, name]);
 
-    useEffect(() => setEdited(true), [sensor]);
+    const editSensor = (sensor: Sensor) => {
+        setEdited(true);
+        setSensor(sensor);
+    };
 
     useCollectEvent('openedSensorDetails');
 
@@ -129,7 +132,7 @@ export function SensorDetails({match, location, history}: RouteComponentProps<an
             }}>
             <>
                 <ErrorNotice error={error} />
-                {!sensor ? <Loading /> : <SensorEditor sensor={sensor} onChange={setSensor} onError={setError} selectedTabKey={tab} onTabSelected={setTab} />}
+                {!sensor ? <Loading /> : <SensorEditor sensor={sensor} onChange={editSensor} onError={setError} selectedTabKey={tab} onTabSelected={setTab} />}
             </>
             {!!selectedLogNode && (
                 <SensorSidePanel
diff --git a/ui/src/app/workflow-templates/workflow-template-details.tsx b/ui/src/app/workflow-templates/workflow-template-details.tsx
index 129228ee13fe..519793d52fb0 100644
--- a/ui/src/app/workflow-templates/workflow-template-details.tsx
+++ b/ui/src/app/workflow-templates/workflow-template-details.tsx
@@ -54,7 +54,10 @@ export function WorkflowTemplateDetails({history, location, match}: RouteCompone
     const [template, setTemplate] = useState<WorkflowTemplate>();
     const [edited, setEdited] = useState(false);
 
-    useEffect(() => setEdited(true), [template]);
+    const editTemplate = (template: WorkflowTemplate) => {
+        setTemplate(template);
+        setEdited(true);
+    };
 
     useEffect(() => {
         services.workflowTemplate
@@ -122,7 +125,7 @@ export function WorkflowTemplateDetails({history, location, match}: RouteCompone
             }}>
             <>
                 <ErrorNotice error={error} />
-                {!template ? <Loading /> : <WorkflowTemplateEditor template={template} onChange={setTemplate} onError={setError} onTabSelected={setTab} selectedTabKey={tab} />}
+                {!template ? <Loading /> : <WorkflowTemplateEditor template={template} onChange={editTemplate} onError={setError} onTabSelected={setTab} selectedTabKey={tab} />}
             </>
             {template && (
                 <SlidingPanel isShown={!!sidePanel} onClose={() => setSidePanel(null)} isMiddle={sidePanel === 'submit'}>