From 1b6e72e49edb334af00dedce992b8b027240d435 Mon Sep 17 00:00:00 2001
From: Anton Gilgur <agilgur5@gmail.com>
Date: Sat, 4 Nov 2023 17:09:02 -0400
Subject: [PATCH] refactor: invert conditionals for less nesting in
 `includeScriptOutput`

- I didn't get a chance to review e5d8c53575923b4fb0f0d3a64614388248645e4d before it was merged in, but it added more nesting when there was already some unnecessary nesting

- invert the `boundaryID` conditional from there for less nesting
- and invert the `boundaryNode` conditional as well for less nesting

- less nesting = less code complexity

- **NOTE**: there is a small semantic difference here from the previous code from 1f6b19f3ab9f8758684bb6c93289d57c5dd1d963
  - when there is an error retrieving `boundaryNode`, the error will now be returned
  - previously it was not returned, but the purpose of the PR was to expose these errors as they are indicative of an underlying logic bug
    - so I think this is still within the intent of the PR, but tagging the author just in case

Signed-off-by: Anton Gilgur <agilgur5@gmail.com>
---
 workflow/controller/operator.go | 41 +++++++++++++++++----------------
 1 file changed, 21 insertions(+), 20 deletions(-)

diff --git a/workflow/controller/operator.go b/workflow/controller/operator.go
index 98ebbaf7fbc8..4fa05a1c6dde 100644
--- a/workflow/controller/operator.go
+++ b/workflow/controller/operator.go
@@ -3688,29 +3688,30 @@ func (woc *wfOperationCtx) deletePDBResource(ctx context.Context) error {
 // Check if the output of this node is referenced elsewhere in the Workflow. If so, make sure to include it during
 // execution.
 func (woc *wfOperationCtx) includeScriptOutput(nodeName, boundaryID string) (bool, error) {
-	if boundaryID != "" {
-		if boundaryNode, err := woc.wf.Status.Nodes.Get(boundaryID); err == nil {
-			tmplCtx, err := woc.createTemplateContext(boundaryNode.GetTemplateScope())
-			if err != nil {
-				return false, err
-			}
-			_, parentTemplate, templateStored, err := tmplCtx.ResolveTemplate(boundaryNode)
-			if err != nil {
-				return false, err
-			}
-			// A new template was stored during resolution, persist it
-			if templateStored {
-				woc.updated = true
-			}
+	if boundaryID == "" {
+		return false, nil
+	}
+	boundaryNode, err := woc.wf.Status.Nodes.Get(boundaryID)
+	if err != nil {
+		woc.log.Errorf("was unable to obtain node for %s", boundaryID)
+		return false, err
+	}
 
-			name := getStepOrDAGTaskName(nodeName)
-			return hasOutputResultRef(name, parentTemplate), nil
-		} else {
-			woc.log.Errorf("was unable to obtain node for %s", boundaryID)
-		}
+	tmplCtx, err := woc.createTemplateContext(boundaryNode.GetTemplateScope())
+	if err != nil {
+		return false, err
+	}
+	_, parentTemplate, templateStored, err := tmplCtx.ResolveTemplate(boundaryNode)
+	if err != nil {
+		return false, err
+	}
+	// A new template was stored during resolution, persist it
+	if templateStored {
+		woc.updated = true
 	}
 
-	return false, nil
+	name := getStepOrDAGTaskName(nodeName)
+	return hasOutputResultRef(name, parentTemplate), nil
 }
 
 func (woc *wfOperationCtx) fetchWorkflowSpec() (wfv1.WorkflowSpecHolder, error) {