Skip to content

Commit fa5af9b

Browse files
authored
Validate column colors in the CLI (#174)
* Validate column color aliases in the CLI * Clarify supported column color values
1 parent 79b5f6f commit fa5af9b

4 files changed

Lines changed: 257 additions & 9 deletions

File tree

internal/commands/column.go

Lines changed: 16 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -131,10 +131,15 @@ var columnCreateCmd = &cobra.Command{
131131
return newRequiredFlagError("name")
132132
}
133133

134+
color, err := normalizeColumnColor(columnCreateColor)
135+
if err != nil {
136+
return err
137+
}
138+
134139
ac := getSDK()
135140
req := &generated.CreateColumnRequest{Name: columnCreateName}
136-
if columnCreateColor != "" {
137-
req.Color = columnCreateColor
141+
if color != "" {
142+
req.Color = color
138143
}
139144

140145
data, resp, err := ac.Columns().Create(cmd.Context(), boardID, req)
@@ -199,12 +204,17 @@ var columnUpdateCmd = &cobra.Command{
199204

200205
columnID := args[0]
201206

207+
color, err := normalizeColumnColor(columnUpdateColor)
208+
if err != nil {
209+
return err
210+
}
211+
202212
req := &generated.UpdateColumnRequest{}
203213
if columnUpdateName != "" {
204214
req.Name = columnUpdateName
205215
}
206-
if columnUpdateColor != "" {
207-
req.Color = columnUpdateColor
216+
if color != "" {
217+
req.Color = color
208218
}
209219

210220
data, _, err := getSDK().Columns().Update(cmd.Context(), boardID, columnID, req)
@@ -343,13 +353,13 @@ func init() {
343353
// Create
344354
columnCreateCmd.Flags().StringVar(&columnCreateBoard, "board", "", "Board ID (required)")
345355
columnCreateCmd.Flags().StringVar(&columnCreateName, "name", "", "Column name (required)")
346-
columnCreateCmd.Flags().StringVar(&columnCreateColor, "color", "", "Column color")
356+
columnCreateCmd.Flags().StringVar(&columnCreateColor, "color", "", "Column color ("+columnColorNamesHelp+"; or supported API color value)")
347357
columnCmd.AddCommand(columnCreateCmd)
348358

349359
// Update
350360
columnUpdateCmd.Flags().StringVar(&columnUpdateBoard, "board", "", "Board ID (required)")
351361
columnUpdateCmd.Flags().StringVar(&columnUpdateName, "name", "", "Column name")
352-
columnUpdateCmd.Flags().StringVar(&columnUpdateColor, "color", "", "Column color")
362+
columnUpdateCmd.Flags().StringVar(&columnUpdateColor, "color", "", "Column color ("+columnColorNamesHelp+"; or supported API color value)")
353363
columnCmd.AddCommand(columnUpdateCmd)
354364

355365
// Delete

internal/commands/column_color.go

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
package commands
2+
3+
import (
4+
"fmt"
5+
"strings"
6+
7+
"github.com/basecamp/fizzy-cli/internal/errors"
8+
)
9+
10+
type columnColor struct {
11+
Name string
12+
Value string
13+
}
14+
15+
var columnColors = []columnColor{
16+
{Name: "blue", Value: "var(--color-card-default)"},
17+
{Name: "gray", Value: "var(--color-card-1)"},
18+
{Name: "tan", Value: "var(--color-card-2)"},
19+
{Name: "yellow", Value: "var(--color-card-3)"},
20+
{Name: "lime", Value: "var(--color-card-4)"},
21+
{Name: "aqua", Value: "var(--color-card-5)"},
22+
{Name: "violet", Value: "var(--color-card-6)"},
23+
{Name: "purple", Value: "var(--color-card-7)"},
24+
{Name: "pink", Value: "var(--color-card-8)"},
25+
}
26+
27+
var columnColorNamesHelp = func() string {
28+
names := make([]string, len(columnColors))
29+
for i, color := range columnColors {
30+
names[i] = color.Name
31+
}
32+
return strings.Join(names, ", ")
33+
}()
34+
35+
func normalizeColumnColor(input string) (string, error) {
36+
color := strings.TrimSpace(input)
37+
if color == "" {
38+
return "", nil
39+
}
40+
41+
for _, valid := range columnColors {
42+
if strings.EqualFold(color, valid.Name) || color == valid.Value {
43+
return valid.Value, nil
44+
}
45+
}
46+
47+
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))
48+
}
Lines changed: 190 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,190 @@
1+
package commands
2+
3+
import (
4+
"strings"
5+
"testing"
6+
7+
"github.com/basecamp/fizzy-cli/internal/client"
8+
"github.com/basecamp/fizzy-cli/internal/errors"
9+
)
10+
11+
func TestNormalizeColumnColor(t *testing.T) {
12+
tests := []struct {
13+
name string
14+
in string
15+
want string
16+
}{
17+
{name: "blue alias", in: "blue", want: "var(--color-card-default)"},
18+
{name: "gray alias", in: "gray", want: "var(--color-card-1)"},
19+
{name: "tan alias", in: "tan", want: "var(--color-card-2)"},
20+
{name: "yellow alias", in: "yellow", want: "var(--color-card-3)"},
21+
{name: "lime alias", in: "lime", want: "var(--color-card-4)"},
22+
{name: "aqua alias", in: "aqua", want: "var(--color-card-5)"},
23+
{name: "violet alias", in: "violet", want: "var(--color-card-6)"},
24+
{name: "purple alias", in: "purple", want: "var(--color-card-7)"},
25+
{name: "pink alias", in: "pink", want: "var(--color-card-8)"},
26+
{name: "mixed-case alias", in: "AqUa", want: "var(--color-card-5)"},
27+
{name: "trimmed alias", in: " aqua ", want: "var(--color-card-5)"},
28+
{name: "API value", in: "var(--color-card-5)", want: "var(--color-card-5)"},
29+
{name: "blank", in: "", want: ""},
30+
}
31+
32+
for _, tt := range tests {
33+
t.Run(tt.name, func(t *testing.T) {
34+
got, err := normalizeColumnColor(tt.in)
35+
if err != nil {
36+
t.Fatalf("unexpected error: %v", err)
37+
}
38+
if got != tt.want {
39+
t.Fatalf("normalizeColumnColor(%q) = %q, want %q", tt.in, got, tt.want)
40+
}
41+
})
42+
}
43+
}
44+
45+
func TestNormalizeColumnColorRejectsUnknownColor(t *testing.T) {
46+
_, err := normalizeColumnColor("Teal")
47+
assertExitCode(t, err, errors.ExitInvalidArgs)
48+
49+
if err == nil {
50+
t.Fatal("expected error")
51+
}
52+
for _, name := range []string{"blue", "gray", "tan", "yellow", "lime", "aqua", "violet", "purple", "pink"} {
53+
if !strings.Contains(err.Error(), name) {
54+
t.Fatalf("expected error to include %s, got %v", name, err)
55+
}
56+
}
57+
if !strings.Contains(err.Error(), "supported API color value") {
58+
t.Fatalf("expected error to clarify supported API color values, got %v", err)
59+
}
60+
}
61+
62+
func TestColumnCreateRejectsUnknownColor(t *testing.T) {
63+
mock := NewMockClient()
64+
SetTestModeWithSDK(mock)
65+
SetTestConfig("token", "account", "https://api.example.com")
66+
defer resetTest()
67+
68+
columnCreateBoard = "123"
69+
columnCreateName = "Test"
70+
columnCreateColor = "Teal"
71+
err := columnCreateCmd.RunE(columnCreateCmd, []string{})
72+
columnCreateBoard = ""
73+
columnCreateName = ""
74+
columnCreateColor = ""
75+
76+
assertExitCode(t, err, errors.ExitInvalidArgs)
77+
if len(mock.PostCalls) != 0 {
78+
t.Fatalf("expected no POST calls, got %d", len(mock.PostCalls))
79+
}
80+
}
81+
82+
func TestColumnUpdateNormalizesColorAlias(t *testing.T) {
83+
mock := NewMockClient()
84+
mock.PatchResponse = &client.APIResponse{
85+
StatusCode: 200,
86+
Data: map[string]any{
87+
"id": "col-1",
88+
"name": "Updated Column",
89+
"color": map[string]any{
90+
"name": "Aqua",
91+
"value": "var(--color-card-5)",
92+
},
93+
},
94+
}
95+
96+
SetTestModeWithSDK(mock)
97+
SetTestConfig("token", "account", "https://api.example.com")
98+
defer resetTest()
99+
100+
columnUpdateBoard = "123"
101+
columnUpdateColor = "AQUA"
102+
err := columnUpdateCmd.RunE(columnUpdateCmd, []string{"col-1"})
103+
columnUpdateBoard = ""
104+
columnUpdateColor = ""
105+
106+
assertExitCode(t, err, 0)
107+
if err != nil {
108+
t.Fatalf("unexpected error: %v", err)
109+
}
110+
if len(mock.PatchCalls) != 1 {
111+
t.Fatalf("expected 1 PATCH call, got %d", len(mock.PatchCalls))
112+
}
113+
body := mock.PatchCalls[0].Body.(map[string]any)
114+
if body["color"] != "var(--color-card-5)" {
115+
t.Fatalf("expected color 'var(--color-card-5)', got %v", body["color"])
116+
}
117+
}
118+
119+
func TestColumnUpdateAcceptsAPIColorValue(t *testing.T) {
120+
mock := NewMockClient()
121+
mock.PatchResponse = &client.APIResponse{
122+
StatusCode: 200,
123+
Data: map[string]any{"id": "col-1", "name": "Updated Column"},
124+
}
125+
126+
SetTestModeWithSDK(mock)
127+
SetTestConfig("token", "account", "https://api.example.com")
128+
defer resetTest()
129+
130+
columnUpdateBoard = "123"
131+
columnUpdateColor = "var(--color-card-8)"
132+
err := columnUpdateCmd.RunE(columnUpdateCmd, []string{"col-1"})
133+
columnUpdateBoard = ""
134+
columnUpdateColor = ""
135+
136+
assertExitCode(t, err, 0)
137+
if err != nil {
138+
t.Fatalf("unexpected error: %v", err)
139+
}
140+
body := mock.PatchCalls[0].Body.(map[string]any)
141+
if body["color"] != "var(--color-card-8)" {
142+
t.Fatalf("expected color 'var(--color-card-8)', got %v", body["color"])
143+
}
144+
}
145+
146+
func TestColumnUpdateRejectsUnknownColor(t *testing.T) {
147+
mock := NewMockClient()
148+
SetTestModeWithSDK(mock)
149+
SetTestConfig("token", "account", "https://api.example.com")
150+
defer resetTest()
151+
152+
columnUpdateBoard = "123"
153+
columnUpdateColor = "Teal"
154+
err := columnUpdateCmd.RunE(columnUpdateCmd, []string{"col-1"})
155+
columnUpdateBoard = ""
156+
columnUpdateColor = ""
157+
158+
assertExitCode(t, err, errors.ExitInvalidArgs)
159+
if len(mock.PatchCalls) != 0 {
160+
t.Fatalf("expected no PATCH calls, got %d", len(mock.PatchCalls))
161+
}
162+
}
163+
164+
func TestColumnColorHelpIncludesLowercaseFriendlyNames(t *testing.T) {
165+
for _, cmd := range []string{"create", "update"} {
166+
t.Run(cmd, func(t *testing.T) {
167+
var flagUsage string
168+
switch cmd {
169+
case "create":
170+
flagUsage = columnCreateCmd.Flags().Lookup("color").Usage
171+
case "update":
172+
flagUsage = columnUpdateCmd.Flags().Lookup("color").Usage
173+
}
174+
175+
for _, name := range []string{"blue", "gray", "tan", "yellow", "lime", "aqua", "violet", "purple", "pink"} {
176+
if !strings.Contains(flagUsage, name) {
177+
t.Fatalf("expected --color help to include %s, got %q", name, flagUsage)
178+
}
179+
}
180+
for _, name := range []string{"Blue", "Gray", "Tan", "Yellow", "Lime", "Aqua", "Violet", "Purple", "Pink"} {
181+
if strings.Contains(flagUsage, name) {
182+
t.Fatalf("expected --color help to use lowercase aliases, got %q", flagUsage)
183+
}
184+
}
185+
if !strings.Contains(flagUsage, "supported API color value") {
186+
t.Fatalf("expected --color help to clarify supported API color values, got %q", flagUsage)
187+
}
188+
})
189+
}
190+
}

internal/commands/column_test.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -235,7 +235,7 @@ func TestColumnCreate(t *testing.T) {
235235

236236
columnCreateBoard = "123"
237237
columnCreateName = "Test"
238-
columnCreateColor = "blue"
238+
columnCreateColor = "aqua"
239239
err := columnCreateCmd.RunE(columnCreateCmd, []string{})
240240
columnCreateBoard = ""
241241
columnCreateName = ""
@@ -247,8 +247,8 @@ func TestColumnCreate(t *testing.T) {
247247
t.Fatalf("unexpected error: %v", err)
248248
}
249249
body := mock.PostCalls[0].Body.(map[string]any)
250-
if body["color"] != "blue" {
251-
t.Errorf("expected color 'blue', got '%v'", body["color"])
250+
if body["color"] != "var(--color-card-5)" {
251+
t.Errorf("expected color 'var(--color-card-5)', got '%v'", body["color"])
252252
}
253253
})
254254
}

0 commit comments

Comments
 (0)