diff --git a/internal/commands/root.go b/internal/commands/root.go index 3d6a398..5491a86 100644 --- a/internal/commands/root.go +++ b/internal/commands/root.go @@ -655,20 +655,6 @@ func printSuccess(data any) { } } -func printSuccessWithLocation(location string) { - switch out.EffectiveFormat() { - case output.FormatStyled: - writeOutputString(renderHumanData(nil, location, false)) - captureResponse() - case output.FormatMarkdown: - writeOutputString(renderHumanData(nil, location, true)) - captureResponse() - default: - recordOutputError(out.OK(nil, output.WithContext("location", location))) - captureResponse() - } -} - // breadcrumb creates a single breadcrumb. func breadcrumb(action, cmd, description string) Breadcrumb { return Breadcrumb{Action: action, Cmd: cmd, Description: description} diff --git a/internal/commands/webhook.go b/internal/commands/webhook.go index fb3b9d0..dc7ffb6 100644 --- a/internal/commands/webhook.go +++ b/internal/commands/webhook.go @@ -4,6 +4,7 @@ import ( "fmt" "strings" + "github.com/basecamp/fizzy-sdk/go/pkg/generated" "github.com/spf13/cobra" ) @@ -49,21 +50,43 @@ var webhookListCmd = &cobra.Command{ return err } - client := getClient() - path := fmt.Sprintf("/boards/%s/webhooks.json", boardID) - if webhookListPage > 0 { - path += fmt.Sprintf("?page=%d", webhookListPage) - } + ac := getSDK() + var items any + var linkNext string - resp, err := client.GetWithPagination(path, webhookListAll) - if err != nil { - return err + switch { + case webhookListAll: + path := fmt.Sprintf("/boards/%s/webhooks.json", boardID) + if webhookListPage > 0 { + path += fmt.Sprintf("?page=%d", webhookListPage) + } + pages, err := ac.GetAll(cmd.Context(), path) + if err != nil { + return convertSDKError(err) + } + items = jsonAnySlice(pages) + case webhookListPage > 0: + path := fmt.Sprintf("/boards/%s/webhooks.json?page=%d", boardID, webhookListPage) + resp, err := ac.Get(cmd.Context(), path) + if err != nil { + return convertSDKError(err) + } + var list []map[string]any + if err := resp.UnmarshalData(&list); err != nil { + return convertSDKError(err) + } + items = toSliceAny(list) + linkNext = parseSDKLinkNext(resp) + default: + data, resp, err := ac.Webhooks().List(cmd.Context(), boardID) + if err != nil { + return convertSDKError(err) + } + items = normalizeAny(data) + linkNext = parseSDKLinkNext(resp) } - count := 0 - if arr, ok := resp.Data.([]any); ok { - count = len(arr) - } + count := dataCount(items) summary := fmt.Sprintf("%d webhooks", count) if webhookListAll { summary += " (all)" @@ -76,7 +99,7 @@ var webhookListCmd = &cobra.Command{ breadcrumb("create", fmt.Sprintf("fizzy webhook create --board %s --name \"name\" --url \"url\"", boardID), "Create webhook"), } - hasNext := resp.LinkNext != "" + hasNext := linkNext != "" if hasNext { nextPage := webhookListPage + 1 if webhookListPage == 0 { @@ -85,7 +108,7 @@ var webhookListCmd = &cobra.Command{ breadcrumbs = append(breadcrumbs, breadcrumb("next", fmt.Sprintf("fizzy webhook list --board %s --page %d", boardID, nextPage), "Next page")) } - printListPaginated(resp.Data, webhookColumns, hasNext, resp.LinkNext, webhookListAll, summary, breadcrumbs) + printListPaginated(items, webhookColumns, hasNext, linkNext, webhookListAll, summary, breadcrumbs) return nil }, } @@ -185,15 +208,17 @@ var webhookShowCmd = &cobra.Command{ webhookID := args[0] - client := getClient() - resp, err := client.Get(fmt.Sprintf("/boards/%s/webhooks/%s.json", boardID, webhookID)) + ac := getSDK() + raw, _, err := ac.Webhooks().Get(cmd.Context(), boardID, webhookID) if err != nil { - return err + return convertSDKError(err) } + data := normalizeAny(raw) + summary := "Webhook" - if wh, ok := resp.Data.(map[string]any); ok { - if name, ok := wh["name"].(string); ok { + if wh, ok := data.(map[string]any); ok { + if name, ok := wh["name"].(string); ok && name != "" { summary = fmt.Sprintf("Webhook: %s", name) } } @@ -204,7 +229,7 @@ var webhookShowCmd = &cobra.Command{ breadcrumb("reactivate", fmt.Sprintf("fizzy webhook reactivate --board %s %s", boardID, webhookID), "Reactivate webhook"), } - printDetail(resp.Data, summary, breadcrumbs) + printDetail(data, summary, breadcrumbs) return nil }, } @@ -236,51 +261,37 @@ var webhookCreateCmd = &cobra.Command{ return newRequiredFlagError("url") } - webhookParams := map[string]any{ - "name": webhookCreateName, - "url": webhookCreateURL, - } - - if len(webhookCreateActions) > 0 { - webhookParams["subscribed_actions"] = webhookCreateActions + ac := getSDK() + req := &generated.CreateWebhookRequest{ + Name: webhookCreateName, + Url: webhookCreateURL, + SubscribedActions: webhookCreateActions, } - body := map[string]any{ - "webhook": webhookParams, + raw, resp, err := ac.Webhooks().Create(cmd.Context(), boardID, req) + if err != nil { + return convertSDKError(err) } - client := getClient() - resp, err := client.Post(fmt.Sprintf("/boards/%s/webhooks.json", boardID), body) - if err != nil { - return err + data := normalizeAny(raw) + webhookID := "" + if wh, ok := data.(map[string]any); ok { + webhookID = getStringField(wh, "id") } - if resp.Location != "" { - followResp, err := client.FollowLocation(resp.Location) - if err == nil && followResp != nil { - webhookID := "" - if wh, ok := followResp.Data.(map[string]any); ok { - if id, ok := wh["id"].(string); ok { - webhookID = id - } - } - - var breadcrumbs []Breadcrumb - if webhookID != "" { - breadcrumbs = []Breadcrumb{ - breadcrumb("show", fmt.Sprintf("fizzy webhook show --board %s %s", boardID, webhookID), "View webhook"), - breadcrumb("update", fmt.Sprintf("fizzy webhook update --board %s %s --name \"name\"", boardID, webhookID), "Update webhook"), - } - } - - printMutationWithLocation(followResp.Data, resp.Location, breadcrumbs) - return nil + var breadcrumbs []Breadcrumb + if webhookID != "" { + breadcrumbs = []Breadcrumb{ + breadcrumb("show", fmt.Sprintf("fizzy webhook show --board %s %s", boardID, webhookID), "View webhook"), + breadcrumb("update", fmt.Sprintf("fizzy webhook update --board %s %s --name \"name\"", boardID, webhookID), "Update webhook"), } - printSuccessWithLocation(resp.Location) - return nil } - printSuccess(resp.Data) + if location := resp.Headers.Get("Location"); location != "" { + printMutationWithLocation(data, location, breadcrumbs) + } else { + printMutation(data, "", breadcrumbs) + } return nil }, } @@ -307,23 +318,18 @@ var webhookUpdateCmd = &cobra.Command{ webhookID := args[0] - webhookParams := make(map[string]any) - + req := &generated.UpdateWebhookRequest{} if webhookUpdateName != "" { - webhookParams["name"] = webhookUpdateName + req.Name = webhookUpdateName } if len(webhookUpdateActions) > 0 { - webhookParams["subscribed_actions"] = webhookUpdateActions - } - - body := map[string]any{ - "webhook": webhookParams, + req.SubscribedActions = webhookUpdateActions } - client := getClient() - resp, err := client.Patch(fmt.Sprintf("/boards/%s/webhooks/%s.json", boardID, webhookID), body) + ac := getSDK() + raw, _, err := ac.Webhooks().Update(cmd.Context(), boardID, webhookID, req) if err != nil { - return err + return convertSDKError(err) } breadcrumbs := []Breadcrumb{ @@ -331,7 +337,7 @@ var webhookUpdateCmd = &cobra.Command{ breadcrumb("delete", fmt.Sprintf("fizzy webhook delete --board %s %s", boardID, webhookID), "Delete webhook"), } - printMutation(resp.Data, "", breadcrumbs) + printMutation(normalizeAny(raw), "", breadcrumbs) return nil }, } @@ -354,10 +360,9 @@ var webhookDeleteCmd = &cobra.Command{ return err } - client := getClient() - _, err = client.Delete(fmt.Sprintf("/boards/%s/webhooks/%s.json", boardID, args[0])) - if err != nil { - return err + ac := getSDK() + if _, err := ac.Webhooks().Delete(cmd.Context(), boardID, args[0]); err != nil { + return convertSDKError(err) } breadcrumbs := []Breadcrumb{ @@ -392,10 +397,15 @@ var webhookReactivateCmd = &cobra.Command{ webhookID := args[0] - client := getClient() - resp, err := client.Post(fmt.Sprintf("/boards/%s/webhooks/%s/activation.json", boardID, webhookID), nil) + ac := getSDK() + resp, err := ac.Webhooks().Activate(cmd.Context(), boardID, webhookID) if err != nil { - return err + return convertSDKError(err) + } + + data := normalizeAny(resp.Data) + if data == nil { + data = map[string]any{"id": webhookID, "active": true} } breadcrumbs := []Breadcrumb{ @@ -403,7 +413,7 @@ var webhookReactivateCmd = &cobra.Command{ breadcrumb("webhooks", fmt.Sprintf("fizzy webhook list --board %s", boardID), "List webhooks"), } - printMutation(resp.Data, "", breadcrumbs) + printMutation(data, "", breadcrumbs) return nil }, } diff --git a/internal/commands/webhook_test.go b/internal/commands/webhook_test.go index 9abfed0..328d04f 100644 --- a/internal/commands/webhook_test.go +++ b/internal/commands/webhook_test.go @@ -18,9 +18,9 @@ func TestWebhookList(t *testing.T) { }, } - result := SetTestMode(mock) + result := SetTestModeWithSDK(mock) SetTestConfig("token", "account", "https://api.example.com") - defer ResetTestMode() + defer resetTest() webhookListBoard = "board-1" err := webhookListCmd.RunE(webhookListCmd, []string{}) @@ -31,18 +31,18 @@ func TestWebhookList(t *testing.T) { t.Error("expected success response") } if len(mock.GetWithPaginationCalls) != 1 { - t.Errorf("expected 1 GetWithPagination call, got %d", len(mock.GetWithPaginationCalls)) + t.Fatalf("expected 1 GET call, got %d", len(mock.GetWithPaginationCalls)) } - if mock.GetWithPaginationCalls[0].Path != "/boards/board-1/webhooks.json" { - t.Errorf("expected path '/boards/board-1/webhooks.json', got '%s'", mock.GetWithPaginationCalls[0].Path) + if got := mock.GetWithPaginationCalls[0].Path; got != "/boards/board-1/webhooks.json" { + t.Errorf("expected path '/boards/board-1/webhooks.json', got '%s'", got) } }) t.Run("requires board", func(t *testing.T) { mock := NewMockClient() - SetTestMode(mock) + SetTestModeWithSDK(mock) SetTestConfig("token", "account", "https://api.example.com") - defer ResetTestMode() + defer resetTest() webhookListBoard = "" err := webhookListCmd.RunE(webhookListCmd, []string{}) @@ -52,9 +52,9 @@ func TestWebhookList(t *testing.T) { t.Run("requires authentication", func(t *testing.T) { mock := NewMockClient() - SetTestMode(mock) + SetTestModeWithSDK(mock) SetTestConfig("", "account", "https://api.example.com") - defer ResetTestMode() + defer resetTest() webhookListBoard = "board-1" err := webhookListCmd.RunE(webhookListCmd, []string{}) @@ -70,9 +70,9 @@ func TestWebhookList(t *testing.T) { Data: []any{}, } - SetTestMode(mock) + SetTestModeWithSDK(mock) SetTestConfig("token", "account", "https://api.example.com") - defer ResetTestMode() + defer resetTest() webhookListBoard = "board-1" webhookListPage = 3 @@ -81,8 +81,33 @@ func TestWebhookList(t *testing.T) { webhookListPage = 0 assertExitCode(t, err, 0) - if mock.GetWithPaginationCalls[0].Path != "/boards/board-1/webhooks.json?page=3" { - t.Errorf("expected path with page=3, got '%s'", mock.GetWithPaginationCalls[0].Path) + if got := mock.GetWithPaginationCalls[0].Path; got != "/boards/board-1/webhooks.json?page=3" { + t.Errorf("expected path with page=3, got '%s'", got) + } + }) + + t.Run("--all honors --page as the start page", func(t *testing.T) { + mock := NewMockClient() + mock.GetWithPaginationResponse = &client.APIResponse{ + StatusCode: 200, + Data: []any{}, + } + + SetTestModeWithSDK(mock) + SetTestConfig("token", "account", "https://api.example.com") + defer resetTest() + + webhookListBoard = "board-1" + webhookListPage = 2 + webhookListAll = true + err := webhookListCmd.RunE(webhookListCmd, []string{}) + webhookListBoard = "" + webhookListPage = 0 + webhookListAll = false + + assertExitCode(t, err, 0) + if got := mock.GetWithPaginationCalls[0].Path; got != "/boards/board-1/webhooks.json?page=2" { + t.Errorf("expected --all to start from --page=2, got '%s'", got) } }) } @@ -178,9 +203,9 @@ func TestWebhookShow(t *testing.T) { }, } - result := SetTestMode(mock) + result := SetTestModeWithSDK(mock) SetTestConfig("token", "account", "https://api.example.com") - defer ResetTestMode() + defer resetTest() webhookShowBoard = "board-1" err := webhookShowCmd.RunE(webhookShowCmd, []string{"wh-1"}) @@ -190,8 +215,11 @@ func TestWebhookShow(t *testing.T) { if !result.Response.OK { t.Error("expected success response") } - if mock.GetCalls[0].Path != "/boards/board-1/webhooks/wh-1.json" { - t.Errorf("expected path '/boards/board-1/webhooks/wh-1.json', got '%s'", mock.GetCalls[0].Path) + if len(mock.GetCalls) != 1 { + t.Fatalf("expected 1 GET call, got %d", len(mock.GetCalls)) + } + if got := mock.GetCalls[0].Path; got != "/boards/board-1/webhooks/wh-1" { + t.Errorf("expected path '/boards/board-1/webhooks/wh-1', got '%s'", got) } }) @@ -199,9 +227,9 @@ func TestWebhookShow(t *testing.T) { mock := NewMockClient() mock.GetError = errors.NewNotFoundError("Webhook not found") - SetTestMode(mock) + SetTestModeWithSDK(mock) SetTestConfig("token", "account", "https://api.example.com") - defer ResetTestMode() + defer resetTest() webhookShowBoard = "board-1" err := webhookShowCmd.RunE(webhookShowCmd, []string{"bad-id"}) @@ -216,11 +244,7 @@ func TestWebhookCreate(t *testing.T) { mock := NewMockClient() mock.PostResponse = &client.APIResponse{ StatusCode: 201, - Location: "https://api.example.com/boards/board-1/webhooks/wh-new", - Data: map[string]any{"id": "wh-new"}, - } - mock.FollowLocationResponse = &client.APIResponse{ - StatusCode: 200, + Location: "/boards/board-1/webhooks/wh-new", Data: map[string]any{ "id": "wh-new", "name": "My Hook", @@ -229,9 +253,9 @@ func TestWebhookCreate(t *testing.T) { }, } - result := SetTestMode(mock) + result := SetTestModeWithSDK(mock) SetTestConfig("token", "account", "https://api.example.com") - defer ResetTestMode() + defer resetTest() webhookCreateBoard = "board-1" webhookCreateName = "My Hook" @@ -250,12 +274,14 @@ func TestWebhookCreate(t *testing.T) { } body := mock.PostCalls[0].Body.(map[string]any) - webhookParams := body["webhook"].(map[string]any) - if webhookParams["name"] != "My Hook" { - t.Errorf("expected name 'My Hook', got '%v'", webhookParams["name"]) + if body["name"] != "My Hook" { + t.Errorf("expected name 'My Hook', got '%v'", body["name"]) } - if webhookParams["url"] != "https://example.com/hook" { - t.Errorf("expected url 'https://example.com/hook', got '%v'", webhookParams["url"]) + if body["url"] != "https://example.com/hook" { + t.Errorf("expected url 'https://example.com/hook', got '%v'", body["url"]) + } + if got := result.Response.Context["location"]; got != "/boards/board-1/webhooks/wh-new" { + t.Errorf("expected location context, got %v", got) } }) @@ -263,16 +289,12 @@ func TestWebhookCreate(t *testing.T) { mock := NewMockClient() mock.PostResponse = &client.APIResponse{ StatusCode: 201, - Location: "https://api.example.com/boards/board-1/webhooks/wh-new", - } - mock.FollowLocationResponse = &client.APIResponse{ - StatusCode: 200, Data: map[string]any{"id": "wh-new"}, } - SetTestMode(mock) + SetTestModeWithSDK(mock) SetTestConfig("token", "account", "https://api.example.com") - defer ResetTestMode() + defer resetTest() webhookCreateBoard = "board-1" webhookCreateName = "My Hook" @@ -287,8 +309,10 @@ func TestWebhookCreate(t *testing.T) { assertExitCode(t, err, 0) body := mock.PostCalls[0].Body.(map[string]any) - webhookParams := body["webhook"].(map[string]any) - actions := webhookParams["subscribed_actions"].([]string) + actions, ok := body["subscribed_actions"].([]any) + if !ok { + t.Fatalf("expected subscribed_actions []any, got %T", body["subscribed_actions"]) + } if len(actions) != 2 || actions[0] != "card_published" || actions[1] != "card_closed" { t.Errorf("expected actions [card_published, card_closed], got %v", actions) } @@ -296,9 +320,9 @@ func TestWebhookCreate(t *testing.T) { t.Run("requires name flag", func(t *testing.T) { mock := NewMockClient() - SetTestMode(mock) + SetTestModeWithSDK(mock) SetTestConfig("token", "account", "https://api.example.com") - defer ResetTestMode() + defer resetTest() webhookCreateBoard = "board-1" webhookCreateName = "" @@ -312,9 +336,9 @@ func TestWebhookCreate(t *testing.T) { t.Run("requires url flag", func(t *testing.T) { mock := NewMockClient() - SetTestMode(mock) + SetTestModeWithSDK(mock) SetTestConfig("token", "account", "https://api.example.com") - defer ResetTestMode() + defer resetTest() webhookCreateBoard = "board-1" webhookCreateName = "My Hook" @@ -338,9 +362,9 @@ func TestWebhookUpdate(t *testing.T) { }, } - SetTestMode(mock) + SetTestModeWithSDK(mock) SetTestConfig("token", "account", "https://api.example.com") - defer ResetTestMode() + defer resetTest() webhookUpdateBoard = "board-1" webhookUpdateName = "Updated Name" @@ -349,14 +373,13 @@ func TestWebhookUpdate(t *testing.T) { webhookUpdateName = "" assertExitCode(t, err, 0) - if mock.PatchCalls[0].Path != "/boards/board-1/webhooks/wh-1.json" { - t.Errorf("expected path '/boards/board-1/webhooks/wh-1.json', got '%s'", mock.PatchCalls[0].Path) + if mock.PatchCalls[0].Path != "/boards/board-1/webhooks/wh-1" { + t.Errorf("expected path '/boards/board-1/webhooks/wh-1', got '%s'", mock.PatchCalls[0].Path) } body := mock.PatchCalls[0].Body.(map[string]any) - webhookParams := body["webhook"].(map[string]any) - if webhookParams["name"] != "Updated Name" { - t.Errorf("expected name 'Updated Name', got '%v'", webhookParams["name"]) + if body["name"] != "Updated Name" { + t.Errorf("expected name 'Updated Name', got '%v'", body["name"]) } }) @@ -367,9 +390,9 @@ func TestWebhookUpdate(t *testing.T) { Data: map[string]any{"id": "wh-1"}, } - SetTestMode(mock) + SetTestModeWithSDK(mock) SetTestConfig("token", "account", "https://api.example.com") - defer ResetTestMode() + defer resetTest() webhookUpdateBoard = "board-1" webhookUpdateActions = []string{"card_closed"} @@ -380,8 +403,10 @@ func TestWebhookUpdate(t *testing.T) { assertExitCode(t, err, 0) body := mock.PatchCalls[0].Body.(map[string]any) - webhookParams := body["webhook"].(map[string]any) - actions := webhookParams["subscribed_actions"].([]string) + actions, ok := body["subscribed_actions"].([]any) + if !ok { + t.Fatalf("expected subscribed_actions []any, got %T", body["subscribed_actions"]) + } if len(actions) != 1 || actions[0] != "card_closed" { t.Errorf("expected actions [card_closed], got %v", actions) } @@ -391,9 +416,9 @@ func TestWebhookUpdate(t *testing.T) { mock := NewMockClient() mock.PatchError = errors.NewValidationError("Invalid webhook") - SetTestMode(mock) + SetTestModeWithSDK(mock) SetTestConfig("token", "account", "https://api.example.com") - defer ResetTestMode() + defer resetTest() webhookUpdateBoard = "board-1" webhookUpdateName = "Test" @@ -413,17 +438,17 @@ func TestWebhookDelete(t *testing.T) { Data: map[string]any{}, } - SetTestMode(mock) + SetTestModeWithSDK(mock) SetTestConfig("token", "account", "https://api.example.com") - defer ResetTestMode() + defer resetTest() webhookDeleteBoard = "board-1" err := webhookDeleteCmd.RunE(webhookDeleteCmd, []string{"wh-1"}) webhookDeleteBoard = "" assertExitCode(t, err, 0) - if mock.DeleteCalls[0].Path != "/boards/board-1/webhooks/wh-1.json" { - t.Errorf("expected path '/boards/board-1/webhooks/wh-1.json', got '%s'", mock.DeleteCalls[0].Path) + if mock.DeleteCalls[0].Path != "/boards/board-1/webhooks/wh-1" { + t.Errorf("expected path '/boards/board-1/webhooks/wh-1', got '%s'", mock.DeleteCalls[0].Path) } }) @@ -431,9 +456,9 @@ func TestWebhookDelete(t *testing.T) { mock := NewMockClient() mock.DeleteError = errors.NewNotFoundError("Webhook not found") - SetTestMode(mock) + SetTestModeWithSDK(mock) SetTestConfig("token", "account", "https://api.example.com") - defer ResetTestMode() + defer resetTest() webhookDeleteBoard = "board-1" err := webhookDeleteCmd.RunE(webhookDeleteCmd, []string{"bad-id"}) @@ -454,9 +479,9 @@ func TestWebhookReactivate(t *testing.T) { }, } - SetTestMode(mock) + result := SetTestModeWithSDK(mock) SetTestConfig("token", "account", "https://api.example.com") - defer ResetTestMode() + defer resetTest() webhookReactivateBoard = "board-1" err := webhookReactivateCmd.RunE(webhookReactivateCmd, []string{"wh-1"}) @@ -466,13 +491,20 @@ func TestWebhookReactivate(t *testing.T) { if mock.PostCalls[0].Path != "/boards/board-1/webhooks/wh-1/activation.json" { t.Errorf("expected path '/boards/board-1/webhooks/wh-1/activation.json', got '%s'", mock.PostCalls[0].Path) } + data, ok := result.Response.Data.(map[string]any) + if !ok { + t.Fatalf("expected response data map, got %T", result.Response.Data) + } + if data["id"] != "wh-1" || data["active"] != true { + t.Fatalf("expected activated webhook data, got %#v", data) + } }) t.Run("requires board", func(t *testing.T) { mock := NewMockClient() - SetTestMode(mock) + SetTestModeWithSDK(mock) SetTestConfig("token", "account", "https://api.example.com") - defer ResetTestMode() + defer resetTest() webhookReactivateBoard = "" err := webhookReactivateCmd.RunE(webhookReactivateCmd, []string{"wh-1"})