From bc50b356f673527214e363197d2f0053ef06c934 Mon Sep 17 00:00:00 2001 From: Rob Zolkos Date: Mon, 8 Jun 2026 11:07:51 -0400 Subject: [PATCH 1/2] Validate column color aliases in the CLI --- internal/commands/column.go | 22 ++- internal/commands/column_color.go | 48 +++++++ internal/commands/column_color_test.go | 184 +++++++++++++++++++++++++ internal/commands/column_test.go | 6 +- 4 files changed, 251 insertions(+), 9 deletions(-) create mode 100644 internal/commands/column_color.go create mode 100644 internal/commands/column_color_test.go diff --git a/internal/commands/column.go b/internal/commands/column.go index b387d04..72a3b97 100644 --- a/internal/commands/column.go +++ b/internal/commands/column.go @@ -131,10 +131,15 @@ var columnCreateCmd = &cobra.Command{ return newRequiredFlagError("name") } + color, err := normalizeColumnColor(columnCreateColor) + if err != nil { + return err + } + ac := getSDK() req := &generated.CreateColumnRequest{Name: columnCreateName} - if columnCreateColor != "" { - req.Color = columnCreateColor + if color != "" { + req.Color = color } data, resp, err := ac.Columns().Create(cmd.Context(), boardID, req) @@ -199,12 +204,17 @@ var columnUpdateCmd = &cobra.Command{ columnID := args[0] + color, err := normalizeColumnColor(columnUpdateColor) + if err != nil { + return err + } + req := &generated.UpdateColumnRequest{} if columnUpdateName != "" { req.Name = columnUpdateName } - if columnUpdateColor != "" { - req.Color = columnUpdateColor + if color != "" { + req.Color = color } data, _, err := getSDK().Columns().Update(cmd.Context(), boardID, columnID, req) @@ -343,13 +353,13 @@ func init() { // Create columnCreateCmd.Flags().StringVar(&columnCreateBoard, "board", "", "Board ID (required)") columnCreateCmd.Flags().StringVar(&columnCreateName, "name", "", "Column name (required)") - columnCreateCmd.Flags().StringVar(&columnCreateColor, "color", "", "Column color") + columnCreateCmd.Flags().StringVar(&columnCreateColor, "color", "", "Column color ("+columnColorNamesHelp+"; or API color value)") columnCmd.AddCommand(columnCreateCmd) // Update columnUpdateCmd.Flags().StringVar(&columnUpdateBoard, "board", "", "Board ID (required)") columnUpdateCmd.Flags().StringVar(&columnUpdateName, "name", "", "Column name") - columnUpdateCmd.Flags().StringVar(&columnUpdateColor, "color", "", "Column color") + columnUpdateCmd.Flags().StringVar(&columnUpdateColor, "color", "", "Column color ("+columnColorNamesHelp+"; or API color value)") columnCmd.AddCommand(columnUpdateCmd) // Delete diff --git a/internal/commands/column_color.go b/internal/commands/column_color.go new file mode 100644 index 0000000..effb305 --- /dev/null +++ b/internal/commands/column_color.go @@ -0,0 +1,48 @@ +package commands + +import ( + "fmt" + "strings" + + "github.com/basecamp/fizzy-cli/internal/errors" +) + +type columnColor struct { + Name string + Value string +} + +var columnColors = []columnColor{ + {Name: "blue", Value: "var(--color-card-default)"}, + {Name: "gray", Value: "var(--color-card-1)"}, + {Name: "tan", Value: "var(--color-card-2)"}, + {Name: "yellow", Value: "var(--color-card-3)"}, + {Name: "lime", Value: "var(--color-card-4)"}, + {Name: "aqua", Value: "var(--color-card-5)"}, + {Name: "violet", Value: "var(--color-card-6)"}, + {Name: "purple", Value: "var(--color-card-7)"}, + {Name: "pink", Value: "var(--color-card-8)"}, +} + +var columnColorNamesHelp = func() string { + names := make([]string, len(columnColors)) + for i, color := range columnColors { + names[i] = color.Name + } + return strings.Join(names, ", ") +}() + +func normalizeColumnColor(input string) (string, error) { + color := strings.TrimSpace(input) + if color == "" { + return "", nil + } + + for _, valid := range columnColors { + if strings.EqualFold(color, valid.Name) || color == valid.Value { + return valid.Value, nil + } + } + + return "", errors.NewInvalidArgsError(fmt.Sprintf("--color must be one of: %s; or an API color value like %s (got %q)", columnColorNamesHelp, columnColors[0].Value, input)) +} diff --git a/internal/commands/column_color_test.go b/internal/commands/column_color_test.go new file mode 100644 index 0000000..aed3ec2 --- /dev/null +++ b/internal/commands/column_color_test.go @@ -0,0 +1,184 @@ +package commands + +import ( + "strings" + "testing" + + "github.com/basecamp/fizzy-cli/internal/client" + "github.com/basecamp/fizzy-cli/internal/errors" +) + +func TestNormalizeColumnColor(t *testing.T) { + tests := []struct { + name string + in string + want string + }{ + {name: "blue alias", in: "blue", want: "var(--color-card-default)"}, + {name: "gray alias", in: "gray", want: "var(--color-card-1)"}, + {name: "tan alias", in: "tan", want: "var(--color-card-2)"}, + {name: "yellow alias", in: "yellow", want: "var(--color-card-3)"}, + {name: "lime alias", in: "lime", want: "var(--color-card-4)"}, + {name: "aqua alias", in: "aqua", want: "var(--color-card-5)"}, + {name: "violet alias", in: "violet", want: "var(--color-card-6)"}, + {name: "purple alias", in: "purple", want: "var(--color-card-7)"}, + {name: "pink alias", in: "pink", want: "var(--color-card-8)"}, + {name: "mixed-case alias", in: "AqUa", want: "var(--color-card-5)"}, + {name: "trimmed alias", in: " aqua ", want: "var(--color-card-5)"}, + {name: "API value", in: "var(--color-card-5)", want: "var(--color-card-5)"}, + {name: "blank", in: "", want: ""}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := normalizeColumnColor(tt.in) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if got != tt.want { + t.Fatalf("normalizeColumnColor(%q) = %q, want %q", tt.in, got, tt.want) + } + }) + } +} + +func TestNormalizeColumnColorRejectsUnknownColor(t *testing.T) { + _, err := normalizeColumnColor("Teal") + assertExitCode(t, err, errors.ExitInvalidArgs) + + if err == nil { + t.Fatal("expected error") + } + for _, name := range []string{"blue", "gray", "tan", "yellow", "lime", "aqua", "violet", "purple", "pink"} { + if !strings.Contains(err.Error(), name) { + t.Fatalf("expected error to include %s, got %v", name, err) + } + } +} + +func TestColumnCreateRejectsUnknownColor(t *testing.T) { + mock := NewMockClient() + SetTestModeWithSDK(mock) + SetTestConfig("token", "account", "https://api.example.com") + defer resetTest() + + columnCreateBoard = "123" + columnCreateName = "Test" + columnCreateColor = "Teal" + err := columnCreateCmd.RunE(columnCreateCmd, []string{}) + columnCreateBoard = "" + columnCreateName = "" + columnCreateColor = "" + + assertExitCode(t, err, errors.ExitInvalidArgs) + if len(mock.PostCalls) != 0 { + t.Fatalf("expected no POST calls, got %d", len(mock.PostCalls)) + } +} + +func TestColumnUpdateNormalizesColorAlias(t *testing.T) { + mock := NewMockClient() + mock.PatchResponse = &client.APIResponse{ + StatusCode: 200, + Data: map[string]any{ + "id": "col-1", + "name": "Updated Column", + "color": map[string]any{ + "name": "Aqua", + "value": "var(--color-card-5)", + }, + }, + } + + SetTestModeWithSDK(mock) + SetTestConfig("token", "account", "https://api.example.com") + defer resetTest() + + columnUpdateBoard = "123" + columnUpdateColor = "AQUA" + err := columnUpdateCmd.RunE(columnUpdateCmd, []string{"col-1"}) + columnUpdateBoard = "" + columnUpdateColor = "" + + assertExitCode(t, err, 0) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if len(mock.PatchCalls) != 1 { + t.Fatalf("expected 1 PATCH call, got %d", len(mock.PatchCalls)) + } + body := mock.PatchCalls[0].Body.(map[string]any) + if body["color"] != "var(--color-card-5)" { + t.Fatalf("expected color 'var(--color-card-5)', got %v", body["color"]) + } +} + +func TestColumnUpdateAcceptsAPIColorValue(t *testing.T) { + mock := NewMockClient() + mock.PatchResponse = &client.APIResponse{ + StatusCode: 200, + Data: map[string]any{"id": "col-1", "name": "Updated Column"}, + } + + SetTestModeWithSDK(mock) + SetTestConfig("token", "account", "https://api.example.com") + defer resetTest() + + columnUpdateBoard = "123" + columnUpdateColor = "var(--color-card-8)" + err := columnUpdateCmd.RunE(columnUpdateCmd, []string{"col-1"}) + columnUpdateBoard = "" + columnUpdateColor = "" + + assertExitCode(t, err, 0) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + body := mock.PatchCalls[0].Body.(map[string]any) + if body["color"] != "var(--color-card-8)" { + t.Fatalf("expected color 'var(--color-card-8)', got %v", body["color"]) + } +} + +func TestColumnUpdateRejectsUnknownColor(t *testing.T) { + mock := NewMockClient() + SetTestModeWithSDK(mock) + SetTestConfig("token", "account", "https://api.example.com") + defer resetTest() + + columnUpdateBoard = "123" + columnUpdateColor = "Teal" + err := columnUpdateCmd.RunE(columnUpdateCmd, []string{"col-1"}) + columnUpdateBoard = "" + columnUpdateColor = "" + + assertExitCode(t, err, errors.ExitInvalidArgs) + if len(mock.PatchCalls) != 0 { + t.Fatalf("expected no PATCH calls, got %d", len(mock.PatchCalls)) + } +} + +func TestColumnColorHelpIncludesLowercaseFriendlyNames(t *testing.T) { + for _, cmd := range []string{"create", "update"} { + t.Run(cmd, func(t *testing.T) { + var flagUsage string + switch cmd { + case "create": + flagUsage = columnCreateCmd.Flags().Lookup("color").Usage + case "update": + flagUsage = columnUpdateCmd.Flags().Lookup("color").Usage + } + + for _, name := range []string{"blue", "gray", "tan", "yellow", "lime", "aqua", "violet", "purple", "pink"} { + if !strings.Contains(flagUsage, name) { + t.Fatalf("expected --color help to include %s, got %q", name, flagUsage) + } + } + for _, name := range []string{"Blue", "Gray", "Tan", "Yellow", "Lime", "Aqua", "Violet", "Purple", "Pink"} { + if strings.Contains(flagUsage, name) { + t.Fatalf("expected --color help to use lowercase aliases, got %q", flagUsage) + } + } + }) + } +} diff --git a/internal/commands/column_test.go b/internal/commands/column_test.go index 3073077..8a54f10 100644 --- a/internal/commands/column_test.go +++ b/internal/commands/column_test.go @@ -235,7 +235,7 @@ func TestColumnCreate(t *testing.T) { columnCreateBoard = "123" columnCreateName = "Test" - columnCreateColor = "blue" + columnCreateColor = "aqua" err := columnCreateCmd.RunE(columnCreateCmd, []string{}) columnCreateBoard = "" columnCreateName = "" @@ -247,8 +247,8 @@ func TestColumnCreate(t *testing.T) { t.Fatalf("unexpected error: %v", err) } body := mock.PostCalls[0].Body.(map[string]any) - if body["color"] != "blue" { - t.Errorf("expected color 'blue', got '%v'", body["color"]) + if body["color"] != "var(--color-card-5)" { + t.Errorf("expected color 'var(--color-card-5)', got '%v'", body["color"]) } }) } From ca58ef1d3d56a1af083dc426b86e6e956b0e2da5 Mon Sep 17 00:00:00 2001 From: Rob Zolkos Date: Mon, 8 Jun 2026 11:13:36 -0400 Subject: [PATCH 2/2] Clarify supported column color values --- internal/commands/column.go | 4 ++-- internal/commands/column_color.go | 2 +- internal/commands/column_color_test.go | 6 ++++++ 3 files changed, 9 insertions(+), 3 deletions(-) diff --git a/internal/commands/column.go b/internal/commands/column.go index 72a3b97..d91adcc 100644 --- a/internal/commands/column.go +++ b/internal/commands/column.go @@ -353,13 +353,13 @@ func init() { // Create columnCreateCmd.Flags().StringVar(&columnCreateBoard, "board", "", "Board ID (required)") columnCreateCmd.Flags().StringVar(&columnCreateName, "name", "", "Column name (required)") - columnCreateCmd.Flags().StringVar(&columnCreateColor, "color", "", "Column color ("+columnColorNamesHelp+"; or API color value)") + columnCreateCmd.Flags().StringVar(&columnCreateColor, "color", "", "Column color ("+columnColorNamesHelp+"; or supported API color value)") columnCmd.AddCommand(columnCreateCmd) // Update columnUpdateCmd.Flags().StringVar(&columnUpdateBoard, "board", "", "Board ID (required)") columnUpdateCmd.Flags().StringVar(&columnUpdateName, "name", "", "Column name") - columnUpdateCmd.Flags().StringVar(&columnUpdateColor, "color", "", "Column color ("+columnColorNamesHelp+"; or API color value)") + columnUpdateCmd.Flags().StringVar(&columnUpdateColor, "color", "", "Column color ("+columnColorNamesHelp+"; or supported API color value)") columnCmd.AddCommand(columnUpdateCmd) // Delete diff --git a/internal/commands/column_color.go b/internal/commands/column_color.go index effb305..d180dbf 100644 --- a/internal/commands/column_color.go +++ b/internal/commands/column_color.go @@ -44,5 +44,5 @@ func normalizeColumnColor(input string) (string, error) { } } - return "", errors.NewInvalidArgsError(fmt.Sprintf("--color must be one of: %s; or an API color value like %s (got %q)", columnColorNamesHelp, columnColors[0].Value, input)) + return "", errors.NewInvalidArgsError(fmt.Sprintf("--color must be one of: %s; or a supported API color value like %s (got %q)", columnColorNamesHelp, columnColors[0].Value, input)) } diff --git a/internal/commands/column_color_test.go b/internal/commands/column_color_test.go index aed3ec2..56788a5 100644 --- a/internal/commands/column_color_test.go +++ b/internal/commands/column_color_test.go @@ -54,6 +54,9 @@ func TestNormalizeColumnColorRejectsUnknownColor(t *testing.T) { t.Fatalf("expected error to include %s, got %v", name, err) } } + if !strings.Contains(err.Error(), "supported API color value") { + t.Fatalf("expected error to clarify supported API color values, got %v", err) + } } func TestColumnCreateRejectsUnknownColor(t *testing.T) { @@ -179,6 +182,9 @@ func TestColumnColorHelpIncludesLowercaseFriendlyNames(t *testing.T) { t.Fatalf("expected --color help to use lowercase aliases, got %q", flagUsage) } } + if !strings.Contains(flagUsage, "supported API color value") { + t.Fatalf("expected --color help to clarify supported API color values, got %q", flagUsage) + } }) } }