fix(core): fix overrideTimeout behavior on existing stages (#7190)

spinnaker · Jul 8, 2019 · 7d2c4e1 · 7d2c4e1
1 parent e26f797
commit 7d2c4e1
Showing 1 changed file with 35 additions and 30 deletions.
diff --git a/app/scripts/modules/core/src/pipeline/config/stages/overrideTimeout/OverrideTimeout.tsx b/app/scripts/modules/core/src/pipeline/config/stages/overrideTimeout/OverrideTimeout.tsx
@@ -2,7 +2,7 @@ import * as React from 'react';
 
 import { IStage } from 'core/domain';
 import { CheckboxInput, NumberInput } from 'core/presentation';
-import { HelpContentsRegistry, HelpField } from 'core/help';
+import { HelpField } from 'core/help';
 
 const { useEffect, useState } = React;
 
@@ -22,30 +22,20 @@ export const OverrideTimeout = (props: IOverrideTimeoutConfigProps) => {
   const [overrideTimeout, setOverrideTimeout] = useState(false);
   const [configurable, setConfigurable] = useState(false);
   const [defaults, setDefaults] = useState({ hours: 0, minutes: 0 });
-  const helpContent = HelpContentsRegistry.getHelpField('pipeline.config.timeout');
 
   useEffect(() => {
-    setOverrideValues(overrideTimeout);
-  }, [props.stageConfig]);
+    stageChanged();
+  }, [props.stageConfig, props.stageTimeoutMs]);
 
-  const setOverrideValues = (newOverrideTimeout: boolean) => {
-    const stageDefaults = props.stageConfig ? props.stageConfig.defaultTimeoutMs : null;
-    const originalOverrideTimeout = newOverrideTimeout === true;
-    const shouldRemoveOverride = originalOverrideTimeout === false;
-
-    setConfigurable(!!stageDefaults);
-    setDefaults(toHoursAndMinutes(stageDefaults));
-
-    if (shouldRemoveOverride) {
-      props.updateStageField({ stageTimeoutMs: undefined });
-    } else if (originalOverrideTimeout || props.stageTimeoutMs !== undefined) {
-      // Either vm.overrideTimeout was originally true, or forcing to true because stageTimeoutMs is defined
-      setOverrideTimeout(true);
-      props.updateStageField({
-        stageTimeoutMs: props.stageTimeoutMs || stageDefaults,
-      });
-      setHours(toHoursAndMinutes(props.stageTimeoutMs).hours);
-      setMinutes(toHoursAndMinutes(props.stageTimeoutMs).minutes);
+  const stageChanged = () => {
+    const { stageConfig, stageTimeoutMs } = props;
+    const defaultTimeoutMs = stageConfig && stageConfig.defaultTimeoutMs;
+    setConfigurable(!!defaultTimeoutMs);
+    setDefaults(toHoursAndMinutes(defaultTimeoutMs));
+    if (stageTimeoutMs !== undefined) {
+      enableTimeout();
+    } else {
+      clearTimeout();
     }
   };
 
@@ -56,6 +46,25 @@ export const OverrideTimeout = (props: IOverrideTimeoutConfigProps) => {
     props.updateStageField({ stageTimeoutMs: timeout });
   };
 
+  const toggleTimeout = () => {
+    overrideTimeout ? clearTimeout() : enableTimeout();
+  };
+
+  const clearTimeout = () => {
+    props.updateStageField({ stageTimeoutMs: undefined });
+    setOverrideTimeout(false);
+  };
+
+  const enableTimeout = () => {
+    const stageDefaults = props.stageConfig ? props.stageConfig.defaultTimeoutMs : null;
+    setOverrideTimeout(true);
+    props.updateStageField({
+      stageTimeoutMs: props.stageTimeoutMs || stageDefaults,
+    });
+    setHours(toHoursAndMinutes(props.stageTimeoutMs).hours);
+    setMinutes(toHoursAndMinutes(props.stageTimeoutMs).minutes);
+  };
+
   function toHoursAndMinutes(ms: number) {
     if (!ms) {
       return { hours: 0, minutes: 0 };
@@ -88,15 +97,11 @@ export const OverrideTimeout = (props: IOverrideTimeoutConfigProps) => {
                         {defaults.minutes} {defaults.minutes > 1 ? 'minutes' : 'minute'}
                       </span>
                     )}
-                    )<HelpField content={helpContent} />
+                    ) <HelpField id="pipeline.config.timeout" />
                   </>
                 }
                 value={overrideTimeout}
-                onChange={() => {
-                  const newOverrideTimeout = !overrideTimeout;
-                  setOverrideTimeout(newOverrideTimeout);
-                  setOverrideValues(newOverrideTimeout);
-                }}
+                onChange={toggleTimeout}
               />
             </div>
           </div>
@@ -114,7 +119,7 @@ export const OverrideTimeout = (props: IOverrideTimeoutConfigProps) => {
                     synchronizeTimeout(e.target.value, minutes);
                   }}
                   value={hours}
-                />
+                />{' '}
                 hours
                 <NumberInput
                   inputClassName={'form-control input-sm inline-number with-space-before'}
@@ -124,7 +129,7 @@ export const OverrideTimeout = (props: IOverrideTimeoutConfigProps) => {
                     synchronizeTimeout(hours, e.target.value);
                   }}
                   value={minutes}
-                />
+                />{' '}
                 minutes to complete
               </div>
             </div>