Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 11 additions & 7 deletions cmd/workflows.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand All @@ -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")
Expand Down Expand Up @@ -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")
Expand Down
40 changes: 36 additions & 4 deletions cmd/workflows_nodes_write_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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)
}
})

Expand Down Expand Up @@ -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 {
Expand Down