diff --git a/cmd/contacts.go b/cmd/contacts.go index c686235..ce70bbb 100644 --- a/cmd/contacts.go +++ b/cmd/contacts.go @@ -388,6 +388,26 @@ func runContactsDelete(cmd *cobra.Command, args []string) error { func runContactsUpdate(cmd *cobra.Command, args []string) error { p := printer.New(os.Stdout, GetFormat()) + + // Validate flags before creating API client + var updateFields []string + if cmd.Flags().Changed("name") { + updateFields = append(updateFields, "names") + } + if cmd.Flags().Changed("email") { + updateFields = append(updateFields, "emailAddresses") + } + if cmd.Flags().Changed("phone") { + updateFields = append(updateFields, "phoneNumbers") + } + if cmd.Flags().Changed("organization") || cmd.Flags().Changed("title") { + updateFields = append(updateFields, "organizations") + } + + if len(updateFields) == 0 { + return p.PrintError(fmt.Errorf("at least one field to update must be specified (--name, --email, --phone, --organization, --title)")) + } + ctx := context.Background() factory, err := client.NewFactory(ctx) @@ -409,25 +429,6 @@ func runContactsUpdate(cmd *cobra.Command, args []string) error { title, _ := cmd.Flags().GetString("title") etag, _ := cmd.Flags().GetString("etag") - // Determine which fields the user wants to update - var updateFields []string - if cmd.Flags().Changed("name") { - updateFields = append(updateFields, "names") - } - if cmd.Flags().Changed("email") { - updateFields = append(updateFields, "emailAddresses") - } - if cmd.Flags().Changed("phone") { - updateFields = append(updateFields, "phoneNumbers") - } - if cmd.Flags().Changed("organization") || cmd.Flags().Changed("title") { - updateFields = append(updateFields, "organizations") - } - - if len(updateFields) == 0 { - return p.PrintError(fmt.Errorf("at least one field to update must be specified (--name, --email, --phone, --organization, --title)")) - } - // Fetch existing contact — include metadata for source/etag required by the API fetchFields := append(updateFields, "metadata") existing, err := svc.People.Get(resourceName). diff --git a/cmd/forms.go b/cmd/forms.go index af34c9b..0fdb2d7 100644 --- a/cmd/forms.go +++ b/cmd/forms.go @@ -436,6 +436,23 @@ func runFormsCreate(cmd *cobra.Command, args []string) error { func runFormsUpdate(cmd *cobra.Command, args []string) error { p := printer.New(os.Stdout, GetFormat()) + + // Validate flags before creating API client + filePath, _ := cmd.Flags().GetString("file") + title, _ := cmd.Flags().GetString("title") + description, _ := cmd.Flags().GetString("description") + + hasFile := cmd.Flags().Changed("file") + hasTitle := cmd.Flags().Changed("title") + hasDescription := cmd.Flags().Changed("description") + + if hasFile && (hasTitle || hasDescription) { + return p.PrintError(fmt.Errorf("--file cannot be combined with --title or --description")) + } + if !hasFile && !hasTitle && !hasDescription { + return p.PrintError(fmt.Errorf("provide --file, --title, or --description")) + } + ctx := context.Background() factory, err := client.NewFactory(ctx) @@ -449,17 +466,6 @@ func runFormsUpdate(cmd *cobra.Command, args []string) error { } formID := args[0] - filePath, _ := cmd.Flags().GetString("file") - title, _ := cmd.Flags().GetString("title") - description, _ := cmd.Flags().GetString("description") - - hasFile := cmd.Flags().Changed("file") - hasTitle := cmd.Flags().Changed("title") - hasDescription := cmd.Flags().Changed("description") - - if hasFile && (hasTitle || hasDescription) { - return p.PrintError(fmt.Errorf("--file cannot be combined with --title or --description")) - } var batchReq forms.BatchUpdateFormRequest @@ -500,8 +506,6 @@ func runFormsUpdate(cmd *cobra.Command, args []string) error { } batchReq.Requests = requests - } else { - return p.PrintError(fmt.Errorf("provide --file, --title, or --description")) } resp, err := svc.Forms.BatchUpdate(formID, &batchReq).Do() diff --git a/cmd/sheets.go b/cmd/sheets.go index 18d67b1..fe84fa6 100644 --- a/cmd/sheets.go +++ b/cmd/sheets.go @@ -2815,6 +2815,21 @@ func mapConditionType(rule string) (string, error) { func runSheetsAddConditionalFormat(cmd *cobra.Command, args []string) error { p := printer.New(os.Stdout, GetFormat()) + + // Validate flags before creating API client + rule, _ := cmd.Flags().GetString("rule") + value, _ := cmd.Flags().GetString("value") + + conditionType, err := mapConditionType(rule) + if err != nil { + return p.PrintError(err) + } + + needsValue := map[string]bool{">": true, "<": true, "=": true, "!=": true, "contains": true, "not-contains": true, "formula": true} + if needsValue[rule] && value == "" { + return p.PrintError(fmt.Errorf("--value is required for rule type %q", rule)) + } + ctx := context.Background() factory, err := client.NewFactory(ctx) @@ -2829,24 +2844,11 @@ func runSheetsAddConditionalFormat(cmd *cobra.Command, args []string) error { spreadsheetID := args[0] rangeStr := args[1] - rule, _ := cmd.Flags().GetString("rule") - value, _ := cmd.Flags().GetString("value") bgColor, _ := cmd.Flags().GetString("bg-color") textColor, _ := cmd.Flags().GetString("color") bold, _ := cmd.Flags().GetBool("bold") italic, _ := cmd.Flags().GetBool("italic") - conditionType, err := mapConditionType(rule) - if err != nil { - return p.PrintError(err) - } - - // Validate --value is provided for rules that require it - needsValue := map[string]bool{">": true, "<": true, "=": true, "!=": true, "contains": true, "not-contains": true, "formula": true} - if needsValue[rule] && value == "" { - return p.PrintError(fmt.Errorf("--value is required for rule type %q", rule)) - } - _, gridRange, err := parseRange(svc, spreadsheetID, rangeStr) if err != nil { return p.PrintError(err) @@ -3024,6 +3026,13 @@ func runSheetsListConditionalFormats(cmd *cobra.Command, args []string) error { func runSheetsDeleteConditionalFormat(cmd *cobra.Command, args []string) error { p := printer.New(os.Stdout, GetFormat()) + + // Validate flags before creating API client + index, _ := cmd.Flags().GetInt64("index") + if index < 0 { + return p.PrintError(fmt.Errorf("--index must be >= 0, got %d", index)) + } + ctx := context.Background() factory, err := client.NewFactory(ctx) @@ -3038,11 +3047,6 @@ func runSheetsDeleteConditionalFormat(cmd *cobra.Command, args []string) error { spreadsheetID := args[0] sheetName, _ := cmd.Flags().GetString("sheet") - index, _ := cmd.Flags().GetInt64("index") - - if index < 0 { - return p.PrintError(fmt.Errorf("--index must be >= 0, got %d", index)) - } sheetID, err := getSheetID(svc, spreadsheetID, sheetName) if err != nil { diff --git a/cmd/slides.go b/cmd/slides.go index a6db179..a61226a 100644 --- a/cmd/slides.go +++ b/cmd/slides.go @@ -3108,6 +3108,15 @@ func runSlidesUngroup(cmd *cobra.Command, args []string) error { func runSlidesThumbnail(cmd *cobra.Command, args []string) error { p := printer.New(os.Stdout, GetFormat()) + + // Validate flags before creating API client + size, _ := cmd.Flags().GetString("size") + validSizes := map[string]bool{"SMALL": true, "MEDIUM": true, "LARGE": true} + sizeUpper := strings.ToUpper(size) + if !validSizes[sizeUpper] { + return p.PrintError(fmt.Errorf("invalid size '%s': must be SMALL, MEDIUM, or LARGE", size)) + } + ctx := context.Background() factory, err := client.NewFactory(ctx) @@ -3122,16 +3131,8 @@ func runSlidesThumbnail(cmd *cobra.Command, args []string) error { presentationID := args[0] slideFlag, _ := cmd.Flags().GetString("slide") - size, _ := cmd.Flags().GetString("size") downloadPath, _ := cmd.Flags().GetString("download") - // Validate size before any API calls - validSizes := map[string]bool{"SMALL": true, "MEDIUM": true, "LARGE": true} - sizeUpper := strings.ToUpper(size) - if !validSizes[sizeUpper] { - return p.PrintError(fmt.Errorf("invalid size '%s': must be SMALL, MEDIUM, or LARGE", size)) - } - // Resolve slide flag: could be a slide object ID or a 1-based number pageObjectID := slideFlag if num, err := strconv.Atoi(slideFlag); err == nil && num > 0 { diff --git a/cmd/validation_test.go b/cmd/validation_test.go new file mode 100644 index 0000000..888aae4 --- /dev/null +++ b/cmd/validation_test.go @@ -0,0 +1,173 @@ +package cmd + +import ( + "bytes" + "encoding/json" + "os" + "strings" + "testing" +) + +// captureOutput captures stdout during a function call and returns the output. +func captureOutput(t *testing.T, fn func()) string { + t.Helper() + old := os.Stdout + r, w, err := os.Pipe() + if err != nil { + t.Fatalf("failed to create pipe: %v", err) + } + os.Stdout = w + + fn() + + w.Close() + os.Stdout = old + + var buf bytes.Buffer + buf.ReadFrom(r) + return buf.String() +} + +// extractError parses JSON output and returns the "error" field if present. +func extractError(output string) string { + var result map[string]interface{} + if err := json.Unmarshal([]byte(strings.TrimSpace(output)), &result); err != nil { + return "" + } + if errMsg, ok := result["error"].(string); ok { + return errMsg + } + return "" +} + +// --- Forms validation tests --- + +func TestFormsUpdate_Validation(t *testing.T) { + // Test no-flags case first (before any flags are mutated on the shared command) + t.Run("no_flags", func(t *testing.T) { + cmd := findSubcommand(formsCmd, "update") + if cmd == nil { + t.Fatal("forms update command not found") + } + + output := captureOutput(t, func() { + cmd.RunE(cmd, []string{"form-id"}) + }) + + errMsg := extractError(output) + if !strings.Contains(errMsg, "provide --file, --title, or --description") { + t.Errorf("expected missing-flags error, got output: %s", output) + } + }) + + t.Run("file_and_title_conflict", func(t *testing.T) { + cmd := findSubcommand(formsCmd, "update") + if cmd == nil { + t.Fatal("forms update command not found") + } + + cmd.Flags().Set("file", "test.json") + cmd.Flags().Set("title", "New Title") + + output := captureOutput(t, func() { + cmd.RunE(cmd, []string{"form-id"}) + }) + + errMsg := extractError(output) + if !strings.Contains(errMsg, "cannot be combined") { + t.Errorf("expected conflict error, got output: %s", output) + } + }) +} + +// --- Sheets validation tests --- + +func TestSheetsAddConditionalFormat_ValueRequired(t *testing.T) { + rulesNeedingValue := []string{">", "<", "=", "!=", "contains", "not-contains", "formula"} + for _, rule := range rulesNeedingValue { + t.Run(rule, func(t *testing.T) { + cmd := findSubcommand(sheetsCmd, "add-conditional-format") + if cmd == nil { + t.Fatal("sheets add-conditional-format command not found") + } + + cmd.Flags().Set("rule", rule) + defer cmd.Flags().Set("rule", "") + + output := captureOutput(t, func() { + cmd.RunE(cmd, []string{"spreadsheet-id", "Sheet1!A1:A10"}) + }) + + errMsg := extractError(output) + if !strings.Contains(errMsg, "--value is required") { + t.Errorf("expected --value required error for rule %q, got output: %s", rule, output) + } + }) + } +} + +func TestSheetsDeleteConditionalFormat_NegativeIndex(t *testing.T) { + cmd := findSubcommand(sheetsCmd, "delete-conditional-format") + if cmd == nil { + t.Fatal("sheets delete-conditional-format command not found") + } + + cmd.Flags().Set("sheet", "Sheet1") + cmd.Flags().Set("index", "-1") + defer func() { + cmd.Flags().Set("sheet", "") + cmd.Flags().Set("index", "0") + }() + + output := captureOutput(t, func() { + cmd.RunE(cmd, []string{"spreadsheet-id"}) + }) + + errMsg := extractError(output) + if !strings.Contains(errMsg, "--index must be >= 0") { + t.Errorf("expected negative index error, got output: %s", output) + } +} + +// --- Slides validation tests --- + +func TestSlidesThumbnail_InvalidSize(t *testing.T) { + cmd := findSubcommand(slidesCmd, "thumbnail") + if cmd == nil { + t.Fatal("slides thumbnail command not found") + } + + cmd.Flags().Set("slide", "1") + cmd.Flags().Set("size", "XLARGE") + defer func() { + cmd.Flags().Set("slide", "") + cmd.Flags().Set("size", "MEDIUM") + }() + + output := captureOutput(t, func() { + cmd.RunE(cmd, []string{"presentation-id"}) + }) + + errMsg := extractError(output) + if !strings.Contains(errMsg, "invalid size") { + t.Errorf("expected invalid size error, got output: %s", output) + } +} + +// --- Contacts validation tests --- + +func TestContactsUpdate_NoFlags(t *testing.T) { + cmd := findSubcommand(contactsCmd, "update") + if cmd == nil { + t.Fatal("contacts update command not found") + } + + output := captureOutput(t, func() { + cmd.RunE(cmd, []string{"people/c123"}) + }) + + errMsg := extractError(output) + if !strings.Contains(errMsg, "at least one field") { + t.Errorf("expected missing-flags error, got output: %s", output) + } +}