chore(workflows): deprecate nodes create --before-node-id in favor of --to-node-id (#101)

Author: notnmeyer

cmd/workflows.go

@@ -622,9 +622,8 @@ var workflowsChangeMailingListCmd = &cobra.Command{
 // parseCreateWorkflowNodeFlags reads and validates the `workflows nodes create`
 // flags and builds a CreateWorkflowNodeRequest. Placement depends on insert
 // mode: "between" needs --from-node-id and --to-node-id; "before" inserts
-// before --before-node-id; "after" inserts after --from-node-id (valid only
-// when that node has exactly one outgoing connection). "before" is sent as
-// toNodeId because the API's beforeNodeId field is deprecated.
+// before --to-node-id; "after" inserts after --from-node-id (valid only when
+// that node has exactly one outgoing connection).
 func parseCreateWorkflowNodeFlags(cmd *cobra.Command) (loops.CreateWorkflowNodeRequest, error) {
 	nodeType, _ := cmd.Flags().GetString("node-type")
 	insertMode, _ := cmd.Flags().GetString("insert-mode")
@@ -650,10 +649,14 @@ func parseCreateWorkflowNodeFlags(cmd *cobra.Command) (loops.CreateWorkflowNodeR
 		req.FromNodeID = fromNodeID
 		req.ToNodeID = toNodeID
 	case loops.WorkflowInsertModeBefore:
-		if beforeNodeID == "" {
-			return loops.CreateWorkflowNodeRequest{}, fmt.Errorf("--insert-mode before requires --before-node-id")
+		target := toNodeID
+		if target == "" {
+			target = beforeNodeID
 		}
-		req.ToNodeID = beforeNodeID
+		if target == "" {
+			return loops.CreateWorkflowNodeRequest{}, fmt.Errorf("--insert-mode before requires --to-node-id")
+		}
+		req.ToNodeID = target
 	case loops.WorkflowInsertModeAfter:
 		if fromNodeID == "" {
 			return loops.CreateWorkflowNodeRequest{}, fmt.Errorf("--insert-mode after requires --from-node-id")
@@ -910,8 +913,9 @@ func init() {
 	workflowsNodesCreateCmd.Flags().String("node-type", "", fmt.Sprintf("Node type: %s", strings.Join(createWorkflowNodeTypes, ", ")))
 	workflowsNodesCreateCmd.Flags().String("insert-mode", "", "Insert mode: between, before, or after")
 	workflowsNodesCreateCmd.Flags().String("from-node-id", "", "Source node ID (insert-mode between or after)")
-	workflowsNodesCreateCmd.Flags().String("to-node-id", "", "Target node ID (insert-mode between)")
+	workflowsNodesCreateCmd.Flags().String("to-node-id", "", "Target node ID (insert-mode between or before)")
 	workflowsNodesCreateCmd.Flags().String("before-node-id", "", "Node ID to insert before (insert-mode before)")
+	workflowsNodesCreateCmd.Flags().MarkDeprecated("before-node-id", "use --to-node-id instead")
 	workflowsNodesCreateCmd.Flags().String("expected-revision-id", "", "Expected workflow revision ID (optimistic concurrency)")
 	workflowsNodesCreateCmd.MarkFlagRequired("node-type")
 	workflowsNodesCreateCmd.MarkFlagRequired("insert-mode")

cmd/workflows_nodes_write_test.go

@@ -47,7 +47,27 @@ func TestParseCreateWorkflowNodeFlags(t *testing.T) {
 		}
 	})
 
-	t.Run("before maps before-node-id to ToNodeID (not BeforeNodeID)", func(t *testing.T) {
+	t.Run("before maps to-node-id to ToNodeID (not BeforeNodeID)", func(t *testing.T) {
+		req, err := parseCreateWorkflowNodeFlags(newCreateNodeFlagsCmd(t, map[string]string{
+			"node-type":   loops.CreateWorkflowNodeTypeTimerAction,
+			"insert-mode": loops.WorkflowInsertModeBefore,
+			"to-node-id":  "n3",
+		}))
+		if err != nil {
+			t.Fatalf("unexpected error: %v", err)
+		}
+		if req.ToNodeID != "n3" {
+			t.Errorf("ToNodeID = %q, want n3", req.ToNodeID)
+		}
+		if req.BeforeNodeID != "" {
+			t.Errorf("BeforeNodeID = %q, want empty (deprecated field must not be sent)", req.BeforeNodeID)
+		}
+		if req.FromNodeID != "" {
+			t.Errorf("FromNodeID = %q, want empty", req.FromNodeID)
+		}
+	})
+
+	t.Run("before accepts deprecated before-node-id alias as ToNodeID", func(t *testing.T) {
 		req, err := parseCreateWorkflowNodeFlags(newCreateNodeFlagsCmd(t, map[string]string{
 			"node-type":      loops.CreateWorkflowNodeTypeTimerAction,
 			"insert-mode":    loops.WorkflowInsertModeBefore,
@@ -62,8 +82,20 @@ func TestParseCreateWorkflowNodeFlags(t *testing.T) {
 		if req.BeforeNodeID != "" {
 			t.Errorf("BeforeNodeID = %q, want empty (deprecated field must not be sent)", req.BeforeNodeID)
 		}
-		if req.FromNodeID != "" {
-			t.Errorf("FromNodeID = %q, want empty", req.FromNodeID)
+	})
+
+	t.Run("before prefers to-node-id over deprecated before-node-id", func(t *testing.T) {
+		req, err := parseCreateWorkflowNodeFlags(newCreateNodeFlagsCmd(t, map[string]string{
+			"node-type":      loops.CreateWorkflowNodeTypeTimerAction,
+			"insert-mode":    loops.WorkflowInsertModeBefore,
+			"to-node-id":     "n_new",
+			"before-node-id": "n_old",
+		}))
+		if err != nil {
+			t.Fatalf("unexpected error: %v", err)
+		}
+		if req.ToNodeID != "n_new" {
+			t.Errorf("ToNodeID = %q, want n_new", req.ToNodeID)
 		}
 	})
 
@@ -109,7 +141,7 @@ func TestParseCreateWorkflowNodeFlags(t *testing.T) {
 		{"unknown node-type", map[string]string{"node-type": "Nonsense", "insert-mode": loops.WorkflowInsertModeAfter, "from-node-id": "n1"}},
 		{"unknown insert-mode", map[string]string{"node-type": loops.CreateWorkflowNodeTypeTimerAction, "insert-mode": "sideways", "from-node-id": "n1"}},
 		{"between missing to", map[string]string{"node-type": loops.CreateWorkflowNodeTypeTimerAction, "insert-mode": loops.WorkflowInsertModeBetween, "from-node-id": "n1"}},
-		{"before missing before-node-id", map[string]string{"node-type": loops.CreateWorkflowNodeTypeTimerAction, "insert-mode": loops.WorkflowInsertModeBefore}},
+		{"before missing to-node-id", map[string]string{"node-type": loops.CreateWorkflowNodeTypeTimerAction, "insert-mode": loops.WorkflowInsertModeBefore}},
 		{"after missing from-node-id", map[string]string{"node-type": loops.CreateWorkflowNodeTypeTimerAction, "insert-mode": loops.WorkflowInsertModeAfter}},
 	}
 	for _, tc := range errCases {