From 1899b5dae0c661fa30bc28cf8fe95ea96c35ac7a Mon Sep 17 00:00:00 2001 From: Qasim Date: Sat, 28 Mar 2026 02:39:30 -0400 Subject: [PATCH 1/2] [TW-4734] refactor(cli): reduce code duplication across commands and adapters Centralize repeated patterns into shared helpers to improve maintainability: - Add common.StatusColor/StatusIcon/ColorSprint for status-to-color mapping, replacing duplicate functions in scheduler, webhook, and notetaker packages - Add common.SetupPagination with explicit PaginationMode (SinglePage/WithCap/All) to eliminate duplicate limit/maxItems logic in list commands - Add common.NormalizePageSize and FetchCursorPages to reduce pagination boilerplate - Extract dashboard setDPoPProof() from duplicate blocks in doPostRaw/doGetRaw - Add testutil.WriteJSONResponse() with buffer-first encoding (goroutine-safe, returns proper 500 on encode failure) - Consolidate email list command into shared fetchListMessages/applyListFolderFilter Includes unit tests for all new helpers and updated consumer tests. --- internal/adapters/dashboard/http.go | 20 +- internal/adapters/dashboard/http_test.go | 248 ++++++++++++++++ internal/cli/calendar/events_list.go | 66 ++-- internal/cli/calendar/events_list_test.go | 84 ++++++ internal/cli/common/pagination.go | 72 +++++ internal/cli/common/pagination_test.go | 132 ++++++++ internal/cli/common/status.go | 47 +++ internal/cli/common/status_test.go | 100 +++++++ internal/cli/contacts/list.go | 17 +- internal/cli/contacts/list_test.go | 84 ++++++ internal/cli/email/list.go | 230 +++++++------- internal/cli/email/list_test.go | 281 ++++++++++++++++++ internal/cli/email/search.go | 11 +- internal/cli/notetaker/list.go | 21 +- internal/cli/scheduler/bookings.go | 30 +- internal/cli/webhook/list.go | 73 ++--- internal/cli/webhook/render_test.go | 155 ++++++++++ internal/cli/webhook/show.go | 60 ++-- internal/cli/webhook/update.go | 2 +- internal/cli/webhook/webhook_advanced_test.go | 19 +- internal/testutil/helpers.go | 35 +++ internal/testutil/helpers_test.go | 97 ++++++ 22 files changed, 1558 insertions(+), 326 deletions(-) create mode 100644 internal/adapters/dashboard/http_test.go create mode 100644 internal/cli/calendar/events_list_test.go create mode 100644 internal/cli/common/status.go create mode 100644 internal/cli/common/status_test.go create mode 100644 internal/cli/contacts/list_test.go create mode 100644 internal/cli/email/list_test.go create mode 100644 internal/cli/webhook/render_test.go diff --git a/internal/adapters/dashboard/http.go b/internal/adapters/dashboard/http.go index e386b89..010e136 100644 --- a/internal/adapters/dashboard/http.go +++ b/internal/adapters/dashboard/http.go @@ -46,6 +46,16 @@ func (c *AccountClient) doPost(ctx context.Context, path string, body any, extra return nil } +// setDPoPProof generates and sets the DPoP proof header on the request. +func (c *AccountClient) setDPoPProof(req *http.Request, method, fullURL, accessToken string) error { + proof, err := c.dpop.GenerateProof(method, fullURL, accessToken) + if err != nil { + return err + } + req.Header.Set("DPoP", proof) + return nil +} + // doPostRaw sends a JSON POST request and returns the raw response body. func (c *AccountClient) doPostRaw(ctx context.Context, path string, body any, extraHeaders map[string]string, accessToken string) ([]byte, error) { fullURL := c.baseURL + path @@ -68,12 +78,9 @@ func (c *AccountClient) doPostRaw(ctx context.Context, path string, body any, ex req.Header.Set("Content-Type", "application/json") } - // Add DPoP proof - proof, err := c.dpop.GenerateProof(http.MethodPost, fullURL, accessToken) - if err != nil { + if err := c.setDPoPProof(req, http.MethodPost, fullURL, accessToken); err != nil { return nil, err } - req.Header.Set("DPoP", proof) // Add extra headers (Authorization, X-Nylas-Org) for k, v := range extraHeaders { @@ -135,12 +142,9 @@ func (c *AccountClient) doGetRaw(ctx context.Context, path string, extraHeaders return nil, fmt.Errorf("failed to create request: %w", err) } - // Add DPoP proof - proof, err := c.dpop.GenerateProof(http.MethodGet, fullURL, accessToken) - if err != nil { + if err := c.setDPoPProof(req, http.MethodGet, fullURL, accessToken); err != nil { return nil, err } - req.Header.Set("DPoP", proof) for k, v := range extraHeaders { req.Header.Set(k, v) diff --git a/internal/adapters/dashboard/http_test.go b/internal/adapters/dashboard/http_test.go new file mode 100644 index 0000000..2316ca7 --- /dev/null +++ b/internal/adapters/dashboard/http_test.go @@ -0,0 +1,248 @@ +package dashboard + +import ( + "context" + "encoding/json" + "net/http" + "net/http/httptest" + "testing" +) + +// mockDPoP implements ports.DPoP for testing. +type mockDPoP struct { + proof string + err error +} + +func (m *mockDPoP) GenerateProof(method, url, accessToken string) (string, error) { + return m.proof, m.err +} + +func (m *mockDPoP) Thumbprint() string { + return "test-thumbprint" +} + +func TestSetDPoPProof(t *testing.T) { + tests := []struct { + name string + proof string + proofErr error + wantHeader string + wantErr bool + }{ + { + name: "sets DPoP header on success", + proof: "test-proof-jwt", + wantHeader: "test-proof-jwt", + }, + { + name: "returns error on failure", + proofErr: errTestDPoP, + wantErr: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + client := &AccountClient{ + dpop: &mockDPoP{proof: tt.proof, err: tt.proofErr}, + } + + req := httptest.NewRequest(http.MethodPost, "https://example.com/test", nil) + err := client.setDPoPProof(req, http.MethodPost, "https://example.com/test", "token") + + if tt.wantErr { + if err == nil { + t.Fatal("expected error, got nil") + } + return + } + + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + if got := req.Header.Get("DPoP"); got != tt.wantHeader { + t.Errorf("DPoP header = %q, want %q", got, tt.wantHeader) + } + }) + } +} + +var errTestDPoP = &testError{msg: "dpop generation failed"} + +type testError struct{ msg string } + +func (e *testError) Error() string { return e.msg } + +func TestParseErrorResponse(t *testing.T) { + tests := []struct { + name string + statusCode int + body string + wantMsg string + }{ + { + name: "parses error with code and message", + statusCode: 400, + body: `{"error":{"code":"invalid_request","message":"bad input"}}`, + wantMsg: "invalid_request: bad input", + }, + { + name: "parses error with message only", + statusCode: 500, + body: `{"error":{"message":"internal error"}}`, + wantMsg: "internal error", + }, + { + name: "falls back to raw body", + statusCode: 502, + body: "Bad Gateway", + wantMsg: "Bad Gateway", + }, + { + name: "truncates long body", + statusCode: 500, + body: string(make([]byte, 300)), + wantMsg: "", // truncated to 200 chars + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := parseErrorResponse(tt.statusCode, []byte(tt.body)) + dashErr, ok := err.(*DashboardAPIError) + if !ok { + t.Fatalf("expected *DashboardAPIError, got %T", err) + } + if dashErr.StatusCode != tt.statusCode { + t.Errorf("StatusCode = %d, want %d", dashErr.StatusCode, tt.statusCode) + } + if tt.wantMsg != "" && dashErr.ServerMsg != tt.wantMsg { + t.Errorf("ServerMsg = %q, want %q", dashErr.ServerMsg, tt.wantMsg) + } + }) + } +} + +func TestUnwrapEnvelope(t *testing.T) { + tests := []struct { + name string + body string + wantKey string + wantErr bool + }{ + { + name: "unwraps data field", + body: `{"request_id":"abc","success":true,"data":{"name":"test"}}`, + wantKey: "name", + }, + { + name: "returns body as-is when no data field", + body: `{"name":"test"}`, + wantKey: "name", + }, + { + name: "returns error on invalid JSON", + body: "not json", + wantErr: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result, err := unwrapEnvelope([]byte(tt.body)) + if tt.wantErr { + if err == nil { + t.Fatal("expected error, got nil") + } + return + } + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + var parsed map[string]any + if jsonErr := json.Unmarshal(result, &parsed); jsonErr != nil { + t.Fatalf("result is not valid JSON: %v", jsonErr) + } + if _, ok := parsed[tt.wantKey]; !ok { + t.Errorf("result missing key %q: %s", tt.wantKey, string(result)) + } + }) + } +} + +func TestDashboardAPIError_Error(t *testing.T) { + tests := []struct { + name string + err DashboardAPIError + wantStr string + }{ + { + name: "with message", + err: DashboardAPIError{StatusCode: 400, ServerMsg: "bad request"}, + wantStr: "dashboard API error (HTTP 400): bad request", + }, + { + name: "without message", + err: DashboardAPIError{StatusCode: 500}, + wantStr: "dashboard API error (HTTP 500)", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := tt.err.Error(); got != tt.wantStr { + t.Errorf("Error() = %q, want %q", got, tt.wantStr) + } + }) + } +} + +func TestDoPostAndGet_Integration(t *testing.T) { + // Set up a test server that returns a valid envelope response + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + // Verify DPoP header was set + if dpop := r.Header.Get("DPoP"); dpop == "" { + t.Error("DPoP header not set") + } + + w.Header().Set("Content-Type", "application/json") + resp := map[string]any{ + "request_id": "test-123", + "success": true, + "data": map[string]string{"id": "app-1", "name": "Test App"}, + } + _ = json.NewEncoder(w).Encode(resp) + })) + defer server.Close() + + client := &AccountClient{ + baseURL: server.URL, + httpClient: server.Client(), + dpop: &mockDPoP{proof: "test-proof"}, + } + + t.Run("doPost decodes response", func(t *testing.T) { + var result map[string]string + err := client.doPost(context.Background(), "/test", nil, nil, "token", &result) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if result["id"] != "app-1" { + t.Errorf("result[id] = %q, want %q", result["id"], "app-1") + } + }) + + t.Run("doGet decodes response", func(t *testing.T) { + var result map[string]string + err := client.doGet(context.Background(), "/test", nil, "token", &result) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if result["name"] != "Test App" { + t.Errorf("result[name] = %q, want %q", result["name"], "Test App") + } + }) +} diff --git a/internal/cli/calendar/events_list.go b/internal/cli/calendar/events_list.go index b235358..4a79702 100644 --- a/internal/cli/calendar/events_list.go +++ b/internal/cli/calendar/events_list.go @@ -54,11 +54,11 @@ Examples: } } - // Auto-paginate when limit exceeds API maximum + pag := common.SetupPagination(limit, false, 0) + limit = pag.Limit maxItems := 0 - if limit > common.MaxAPILimit { - maxItems = limit - limit = common.MaxAPILimit + if pag.Mode == common.PaginateWithCap { + maxItems = pag.MaxItems } _, err := common.WithClient(args, func(ctx context.Context, client ports.NylasClient, grantID string) (struct{}, error) { @@ -84,38 +84,9 @@ Examples: params.ShowCancelled = true } - var events []domain.Event - if maxItems > 0 { - // Paginated fetch for large limits - pageSize := min(limit, common.MaxAPILimit) - params.Limit = pageSize - - fetcher := func(ctx context.Context, cursor string) (common.PageResult[domain.Event], error) { - params.PageToken = cursor - resp, err := client.GetEventsWithCursor(ctx, grantID, calID, params) - if err != nil { - return common.PageResult[domain.Event]{}, err - } - return common.PageResult[domain.Event]{ - Data: resp.Data, - NextCursor: resp.Pagination.NextCursor, - }, nil - } - - config := common.DefaultPaginationConfig() - config.PageSize = pageSize - config.MaxItems = maxItems - - events, err = common.FetchAllPages(ctx, config, fetcher) - if err != nil { - return struct{}{}, common.WrapListError("events", err) - } - } else { - // Standard single-page fetch - events, err = client.GetEvents(ctx, grantID, calID, params) - if err != nil { - return struct{}{}, common.WrapListError("events", err) - } + events, err := fetchEvents(ctx, client, grantID, calID, params, maxItems) + if err != nil { + return struct{}{}, common.WrapListError("events", err) } // JSON output (including empty array) @@ -223,3 +194,26 @@ Examples: return cmd } + +func fetchEvents(ctx context.Context, client ports.NylasClient, grantID, calendarID string, params *domain.EventQueryParams, maxItems int) ([]domain.Event, error) { + if maxItems <= 0 { + return client.GetEvents(ctx, grantID, calendarID, params) + } + + pageSize := common.NormalizePageSize(params.Limit) + params.Limit = pageSize + + fetcher := func(ctx context.Context, cursor string) (common.PageResult[domain.Event], error) { + params.PageToken = cursor + resp, err := client.GetEventsWithCursor(ctx, grantID, calendarID, params) + if err != nil { + return common.PageResult[domain.Event]{}, err + } + return common.PageResult[domain.Event]{ + Data: resp.Data, + NextCursor: resp.Pagination.NextCursor, + }, nil + } + + return common.FetchCursorPages(ctx, pageSize, maxItems, fetcher) +} diff --git a/internal/cli/calendar/events_list_test.go b/internal/cli/calendar/events_list_test.go new file mode 100644 index 0000000..b0370a4 --- /dev/null +++ b/internal/cli/calendar/events_list_test.go @@ -0,0 +1,84 @@ +package calendar + +import ( + "context" + "errors" + "testing" + + "github.com/nylas/cli/internal/adapters/nylas" + "github.com/nylas/cli/internal/domain" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +type testCalendarClient struct { + *nylas.MockClient + getEventsWithCursorFunc func(ctx context.Context, grantID, calendarID string, params *domain.EventQueryParams) (*domain.EventListResponse, error) +} + +func (c *testCalendarClient) GetEventsWithCursor(ctx context.Context, grantID, calendarID string, params *domain.EventQueryParams) (*domain.EventListResponse, error) { + if c.getEventsWithCursorFunc != nil { + return c.getEventsWithCursorFunc(ctx, grantID, calendarID, params) + } + return c.MockClient.GetEventsWithCursor(ctx, grantID, calendarID, params) +} + +func TestFetchEvents(t *testing.T) { + t.Run("uses direct fetch when maxItems is zero", func(t *testing.T) { + client := &testCalendarClient{MockClient: nylas.NewMockClient()} + client.GetEventsFunc = func(ctx context.Context, grantID, calendarID string, params *domain.EventQueryParams) ([]domain.Event, error) { + assert.Equal(t, 25, params.Limit) + return []domain.Event{{ID: "event-1"}}, nil + } + + events, err := fetchEvents(context.Background(), client, "grant-123", "cal-123", &domain.EventQueryParams{Limit: 25}, 0) + + require.NoError(t, err) + assert.Len(t, events, 1) + assert.Equal(t, "event-1", events[0].ID) + }) + + t.Run("uses cursor pagination when maxItems is positive", func(t *testing.T) { + client := &testCalendarClient{ + MockClient: nylas.NewMockClient(), + getEventsWithCursorFunc: func(ctx context.Context, grantID, calendarID string, params *domain.EventQueryParams) (*domain.EventListResponse, error) { + switch params.PageToken { + case "": + return &domain.EventListResponse{ + Data: []domain.Event{{ID: "event-1"}, {ID: "event-2"}}, + Pagination: domain.Pagination{ + NextCursor: "next", + }, + }, nil + case "next": + return &domain.EventListResponse{ + Data: []domain.Event{{ID: "event-3"}}, + }, nil + default: + return nil, errors.New("unexpected cursor") + } + }, + } + + events, err := fetchEvents(context.Background(), client, "grant-123", "cal-123", &domain.EventQueryParams{Limit: 500}, 3) + + require.NoError(t, err) + assert.Len(t, events, 3) + assert.Equal(t, []string{"event-1", "event-2", "event-3"}, []string{events[0].ID, events[1].ID, events[2].ID}) + }) + + t.Run("returns pagination errors", func(t *testing.T) { + client := &testCalendarClient{ + MockClient: nylas.NewMockClient(), + getEventsWithCursorFunc: func(ctx context.Context, grantID, calendarID string, params *domain.EventQueryParams) (*domain.EventListResponse, error) { + return nil, errors.New("boom") + }, + } + + events, err := fetchEvents(context.Background(), client, "grant-123", "cal-123", &domain.EventQueryParams{Limit: 500}, 3) + + require.Error(t, err) + assert.Nil(t, events) + assert.Contains(t, err.Error(), "failed to fetch page 1") + }) +} diff --git a/internal/cli/common/pagination.go b/internal/cli/common/pagination.go index 5325abb..ac8cc12 100644 --- a/internal/cli/common/pagination.go +++ b/internal/cli/common/pagination.go @@ -34,6 +34,16 @@ type PaginationConfig struct { Writer io.Writer // Output writer for progress } +// NormalizePageSize clamps a requested page size to the API maximum. +// Zero or negative values fall back to the maximum API page size. +func NormalizePageSize(limit int) int { + pageSize := min(limit, MaxAPILimit) + if pageSize <= 0 { + return MaxAPILimit + } + return pageSize +} + // DefaultPaginationConfig returns default pagination settings. func DefaultPaginationConfig() PaginationConfig { return PaginationConfig{ @@ -109,6 +119,68 @@ func FetchAllPages[T any](ctx context.Context, config PaginationConfig, fetcher return results, nil } +// FetchCursorPages fetches one or more cursor-based pages using the shared +// pagination config. maxItems uses the same semantics as FetchAllPages: +// 0 means unlimited, values >0 cap the total returned items. +func FetchCursorPages[T any](ctx context.Context, limit int, maxItems int, fetcher PageFetcher[T]) ([]T, error) { + config := DefaultPaginationConfig() + config.PageSize = NormalizePageSize(limit) + config.MaxItems = maxItems + return FetchAllPages(ctx, config, fetcher) +} + +// PaginationMode distinguishes between the three pagination behaviors. +type PaginationMode int + +const ( + // PaginateSinglePage fetches one page only. + PaginateSinglePage PaginationMode = iota + // PaginateWithCap fetches multiple pages up to MaxItems. + PaginateWithCap + // PaginateAll fetches every page with no cap. + PaginateAll +) + +// PaginationLimits holds the resolved pagination parameters after applying +// auto-pagination logic. Use SetupPagination to compute these from user flags. +type PaginationLimits struct { + Limit int // Per-page limit to pass to the API + MaxItems int // Total items to fetch (only meaningful when Mode == PaginateWithCap) + Mode PaginationMode // Which pagination behavior to use +} + +// SetupPagination resolves pagination parameters from user-provided flags. +// When limit exceeds MaxAPILimit, it enables auto-pagination. +// When fetchAll is true, it fetches all items up to maxItems (0 = unlimited). +// +// This eliminates duplicate logic across list commands (contacts, email, etc.). +func SetupPagination(limit int, fetchAll bool, maxItems int) PaginationLimits { + if fetchAll { + if maxItems > 0 { + return PaginationLimits{ + Limit: MaxAPILimit, + MaxItems: maxItems, + Mode: PaginateWithCap, + } + } + return PaginationLimits{ + Limit: MaxAPILimit, + Mode: PaginateAll, + } + } + if limit > MaxAPILimit { + return PaginationLimits{ + Limit: MaxAPILimit, + MaxItems: limit, + Mode: PaginateWithCap, + } + } + return PaginationLimits{ + Limit: limit, + Mode: PaginateSinglePage, + } +} + // FetchAllWithProgress fetches all pages and shows a progress indicator. func FetchAllWithProgress[T any](ctx context.Context, fetcher PageFetcher[T], maxItems int) ([]T, error) { config := DefaultPaginationConfig() diff --git a/internal/cli/common/pagination_test.go b/internal/cli/common/pagination_test.go index bc5209a..f86d27e 100644 --- a/internal/cli/common/pagination_test.go +++ b/internal/cli/common/pagination_test.go @@ -45,6 +45,25 @@ func TestDefaultPaginationConfig(t *testing.T) { assert.NotNil(t, config.Writer) } +func TestNormalizePageSize(t *testing.T) { + tests := []struct { + name string + limit int + expected int + }{ + {"keeps small values", 50, 50}, + {"clamps to API max", 500, MaxAPILimit}, + {"zero falls back to API max", 0, MaxAPILimit}, + {"negative falls back to API max", -5, MaxAPILimit}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assert.Equal(t, tt.expected, NormalizePageSize(tt.limit)) + }) + } +} + func TestFetchAllPages_SinglePage(t *testing.T) { ResetLogger() InitLogger(false, true) // Quiet mode to avoid progress output @@ -263,6 +282,34 @@ func TestFetchAllWithProgress_WithMaxItems(t *testing.T) { assert.Equal(t, 2, len(results)) } +func TestFetchCursorPages(t *testing.T) { + ResetLogger() + InitLogger(false, true) + + t.Run("caps results across multiple pages", func(t *testing.T) { + fetcher := func(ctx context.Context, cursor string) (PageResult[string], error) { + switch cursor { + case "": + return PageResult[string]{ + Data: []string{"a", "b"}, + NextCursor: "next", + }, nil + case "next": + return PageResult[string]{ + Data: []string{"c", "d"}, + }, nil + default: + return PageResult[string]{}, errors.New("unexpected cursor") + } + } + + results, err := FetchCursorPages(context.Background(), 500, 3, fetcher) + + require.NoError(t, err) + assert.Equal(t, []string{"a", "b", "c"}, results) + }) +} + func TestPaginatedDisplay_Operations(t *testing.T) { ResetLogger() InitLogger(false, false) // Not quiet for display output @@ -387,3 +434,88 @@ func TestFetchAllPages_ContextDeadline(t *testing.T) { assert.ErrorIs(t, err, context.DeadlineExceeded) assert.Empty(t, results) } + +func TestSetupPagination(t *testing.T) { + tests := []struct { + name string + limit int + fetchAll bool + maxItems int + wantLimit int + wantMaxItems int + wantMode PaginationMode + }{ + { + name: "small limit, single page", + limit: 50, + fetchAll: false, + maxItems: 0, + wantLimit: 50, + wantMode: PaginateSinglePage, + }, + { + name: "limit at API max, single page", + limit: MaxAPILimit, + fetchAll: false, + maxItems: 0, + wantLimit: MaxAPILimit, + wantMode: PaginateSinglePage, + }, + { + name: "limit exceeds API max, auto-paginate with cap", + limit: 500, + fetchAll: false, + maxItems: 0, + wantLimit: MaxAPILimit, + wantMaxItems: 500, + wantMode: PaginateWithCap, + }, + { + name: "fetchAll with no max fetches unlimited", + limit: 50, + fetchAll: true, + maxItems: 0, + wantLimit: MaxAPILimit, + wantMode: PaginateAll, + }, + { + name: "fetchAll with max items caps", + limit: 50, + fetchAll: true, + maxItems: 1000, + wantLimit: MaxAPILimit, + wantMaxItems: 1000, + wantMode: PaginateWithCap, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := SetupPagination(tt.limit, tt.fetchAll, tt.maxItems) + assert.Equal(t, tt.wantLimit, result.Limit, "Limit mismatch") + assert.Equal(t, tt.wantMaxItems, result.MaxItems, "MaxItems mismatch") + assert.Equal(t, tt.wantMode, result.Mode, "Mode mismatch") + }) + } +} + +func TestPaginationMode(t *testing.T) { + tests := []struct { + name string + mode PaginationMode + desc string + }{ + {"single page is 0", PaginateSinglePage, "default zero value"}, + {"with cap is 1", PaginateWithCap, "explicit cap"}, + {"all is 2", PaginateAll, "unlimited"}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // Verify the constants are distinct + assert.NotEqual(t, PaginateSinglePage, PaginateWithCap) + assert.NotEqual(t, PaginateWithCap, PaginateAll) + assert.NotEqual(t, PaginateSinglePage, PaginateAll) + }) + } +} diff --git a/internal/cli/common/status.go b/internal/cli/common/status.go new file mode 100644 index 0000000..46eae28 --- /dev/null +++ b/internal/cli/common/status.go @@ -0,0 +1,47 @@ +package common + +import "github.com/fatih/color" + +// StatusColor returns the appropriate color for a status string. +// This centralizes the status-to-color mapping used across CLI commands +// (scheduler bookings, webhook list, notetaker list, etc.). +// +// Common mappings: +// - Green: active, confirmed, complete, attending +// - Yellow: pending, scheduled, inactive +// - Red: failed, failing, error +// - Cyan: connecting, waiting, processing, waiting_for_entry, media_processing +// - Dim: cancelled, deleted, archived +func StatusColor(status string) *color.Color { + switch status { + case "active", "confirmed", "complete", "attending": + return Green + case "pending", "scheduled", "inactive": + return Yellow + case "failed", "failing", "error": + return Red + case "connecting", "waiting", "processing", "rescheduled", + "waiting_for_entry", "media_processing": + return Cyan + case "cancelled", "deleted", "archived": + return Dim + default: + return Reset + } +} + +// StatusIcon returns a colored status indicator dot for a status string. +// Returns a colored "●" for known statuses, or "○" for unknown. +func StatusIcon(status string) string { + c := StatusColor(status) + if c == Reset { + return "○" + } + return c.Sprint("●") +} + +// ColorSprint applies the status color to the given status string. +// Convenience method combining StatusColor lookup with Sprint. +func ColorSprint(status string) string { + return StatusColor(status).Sprint(status) +} diff --git a/internal/cli/common/status_test.go b/internal/cli/common/status_test.go new file mode 100644 index 0000000..4aaf237 --- /dev/null +++ b/internal/cli/common/status_test.go @@ -0,0 +1,100 @@ +package common + +import ( + "testing" + + "github.com/fatih/color" +) + +func TestStatusColor(t *testing.T) { + // Disable colors for consistent test output + color.NoColor = true + defer func() { color.NoColor = false }() + + tests := []struct { + name string + status string + wantColor *color.Color + }{ + {"active is green", "active", Green}, + {"confirmed is green", "confirmed", Green}, + {"complete is green", "complete", Green}, + {"attending is green", "attending", Green}, + {"pending is yellow", "pending", Yellow}, + {"scheduled is yellow", "scheduled", Yellow}, + {"inactive is yellow", "inactive", Yellow}, + {"failed is red", "failed", Red}, + {"failing is red", "failing", Red}, + {"error is red", "error", Red}, + {"connecting is cyan", "connecting", Cyan}, + {"waiting is cyan", "waiting", Cyan}, + {"processing is cyan", "processing", Cyan}, + {"rescheduled is cyan", "rescheduled", Cyan}, + {"waiting_for_entry is cyan", "waiting_for_entry", Cyan}, + {"media_processing is cyan", "media_processing", Cyan}, + {"cancelled is dim", "cancelled", Dim}, + {"deleted is dim", "deleted", Dim}, + {"archived is dim", "archived", Dim}, + {"unknown is reset", "unknown_status", Reset}, + {"empty is reset", "", Reset}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := StatusColor(tt.status) + if got != tt.wantColor { + t.Errorf("StatusColor(%q) returned wrong color", tt.status) + } + }) + } +} + +func TestStatusIcon(t *testing.T) { + color.NoColor = true + defer func() { color.NoColor = false }() + + tests := []struct { + name string + status string + want string + }{ + {"active shows filled dot", "active", "●"}, + {"pending shows filled dot", "pending", "●"}, + {"failed shows filled dot", "failed", "●"}, + {"unknown shows empty dot", "unknown", "○"}, + {"empty shows empty dot", "", "○"}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := StatusIcon(tt.status) + if got != tt.want { + t.Errorf("StatusIcon(%q) = %q, want %q", tt.status, got, tt.want) + } + }) + } +} + +func TestColorSprint(t *testing.T) { + color.NoColor = true + defer func() { color.NoColor = false }() + + tests := []struct { + name string + status string + want string + }{ + {"renders status text", "active", "active"}, + {"renders pending text", "pending", "pending"}, + {"renders unknown text", "unknown", "unknown"}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := ColorSprint(tt.status) + if got != tt.want { + t.Errorf("ColorSprint(%q) = %q, want %q", tt.status, got, tt.want) + } + }) + } +} diff --git a/internal/cli/contacts/list.go b/internal/cli/contacts/list.go index ab36cde..7421251 100644 --- a/internal/cli/contacts/list.go +++ b/internal/cli/contacts/list.go @@ -25,12 +25,9 @@ func newListCmd() *cobra.Command { Long: "List all contacts for the specified grant or default account.", Args: cobra.MaximumNArgs(1), RunE: func(cmd *cobra.Command, args []string) error { - // Auto-paginate when limit exceeds API maximum - maxItems := 0 - if limit > common.MaxAPILimit { - maxItems = limit - limit = common.MaxAPILimit - } + pag := common.SetupPagination(limit, false, 0) + limit = pag.Limit + maxItems := pag.MaxItems // Check if we should use structured output (JSON/YAML/quiet) if common.IsStructuredOutput(cmd) { @@ -125,7 +122,7 @@ func newListCmd() *cobra.Command { // fetchContacts retrieves contacts, using pagination when maxItems > 0. func fetchContacts(ctx context.Context, client ports.NylasClient, grantID string, params *domain.ContactQueryParams, maxItems int) ([]domain.Contact, error) { if maxItems > 0 { - pageSize := min(params.Limit, common.MaxAPILimit) + pageSize := common.NormalizePageSize(params.Limit) params.Limit = pageSize fetcher := func(ctx context.Context, cursor string) (common.PageResult[domain.Contact], error) { @@ -140,11 +137,7 @@ func fetchContacts(ctx context.Context, client ports.NylasClient, grantID string }, nil } - config := common.DefaultPaginationConfig() - config.PageSize = pageSize - config.MaxItems = maxItems - - return common.FetchAllPages(ctx, config, fetcher) + return common.FetchCursorPages(ctx, pageSize, maxItems, fetcher) } return client.GetContacts(ctx, grantID, params) diff --git a/internal/cli/contacts/list_test.go b/internal/cli/contacts/list_test.go new file mode 100644 index 0000000..fb060f1 --- /dev/null +++ b/internal/cli/contacts/list_test.go @@ -0,0 +1,84 @@ +package contacts + +import ( + "context" + "errors" + "testing" + + "github.com/nylas/cli/internal/adapters/nylas" + "github.com/nylas/cli/internal/domain" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +type testContactsClient struct { + *nylas.MockClient + getContactsWithCursorFunc func(ctx context.Context, grantID string, params *domain.ContactQueryParams) (*domain.ContactListResponse, error) +} + +func (c *testContactsClient) GetContactsWithCursor(ctx context.Context, grantID string, params *domain.ContactQueryParams) (*domain.ContactListResponse, error) { + if c.getContactsWithCursorFunc != nil { + return c.getContactsWithCursorFunc(ctx, grantID, params) + } + return c.MockClient.GetContactsWithCursor(ctx, grantID, params) +} + +func TestFetchContacts(t *testing.T) { + t.Run("uses direct fetch when maxItems is zero", func(t *testing.T) { + client := &testContactsClient{MockClient: nylas.NewMockClient()} + client.GetContactsFunc = func(ctx context.Context, grantID string, params *domain.ContactQueryParams) ([]domain.Contact, error) { + assert.Equal(t, 25, params.Limit) + return []domain.Contact{{ID: "contact-1"}}, nil + } + + contacts, err := fetchContacts(context.Background(), client, "grant-123", &domain.ContactQueryParams{Limit: 25}, 0) + + require.NoError(t, err) + assert.Len(t, contacts, 1) + assert.Equal(t, "contact-1", contacts[0].ID) + }) + + t.Run("uses cursor pagination when maxItems is positive", func(t *testing.T) { + client := &testContactsClient{ + MockClient: nylas.NewMockClient(), + getContactsWithCursorFunc: func(ctx context.Context, grantID string, params *domain.ContactQueryParams) (*domain.ContactListResponse, error) { + switch params.PageToken { + case "": + return &domain.ContactListResponse{ + Data: []domain.Contact{{ID: "contact-1"}, {ID: "contact-2"}}, + Pagination: domain.Pagination{ + NextCursor: "next", + }, + }, nil + case "next": + return &domain.ContactListResponse{ + Data: []domain.Contact{{ID: "contact-3"}}, + }, nil + default: + return nil, errors.New("unexpected cursor") + } + }, + } + + contacts, err := fetchContacts(context.Background(), client, "grant-123", &domain.ContactQueryParams{Limit: 500}, 3) + + require.NoError(t, err) + assert.Len(t, contacts, 3) + assert.Equal(t, []string{"contact-1", "contact-2", "contact-3"}, []string{contacts[0].ID, contacts[1].ID, contacts[2].ID}) + }) + + t.Run("returns pagination errors", func(t *testing.T) { + client := &testContactsClient{ + MockClient: nylas.NewMockClient(), + getContactsWithCursorFunc: func(ctx context.Context, grantID string, params *domain.ContactQueryParams) (*domain.ContactListResponse, error) { + return nil, errors.New("boom") + }, + } + + contacts, err := fetchContacts(context.Background(), client, "grant-123", &domain.ContactQueryParams{Limit: 500}, 3) + + require.Error(t, err) + assert.Nil(t, contacts) + assert.Contains(t, err.Error(), "failed to fetch page 1") + }) +} diff --git a/internal/cli/email/list.go b/internal/cli/email/list.go index 938eaba..8d6ef4e 100644 --- a/internal/cli/email/list.go +++ b/internal/cli/email/list.go @@ -3,6 +3,7 @@ package email import ( "context" "fmt" + "io" "slices" "strings" @@ -50,68 +51,26 @@ Use --max to limit total messages when using --all.`, nylas email list --all --max 500`, Args: cobra.MaximumNArgs(1), RunE: func(cmd *cobra.Command, args []string) error { - // Check if we should use structured output (JSON/YAML/quiet) - if common.IsStructuredOutput(cmd) { - return runListStructured(cmd, args, limit, unread, starred, from, folder, allFolders, all, maxItems, metadataPair) + opts := listOptions{ + limit: limit, + unread: unread, + starred: starred, + from: from, + folder: folder, + allFolders: allFolders, + all: all, + maxItems: maxItems, + metadataPair: metadataPair, } - // Auto-paginate when limit exceeds API maximum - if limit > common.MaxAPILimit && !all { - maxItems = limit - limit = common.MaxAPILimit - } else if !all { - maxItems = -1 // single-page fetch + // Check if we should use structured output (JSON/YAML/quiet) + if common.IsStructuredOutput(cmd) { + return runListStructured(cmd, args, opts) } // Traditional formatted output _, err := common.WithClient(args, func(ctx context.Context, client ports.NylasClient, grantID string) (struct{}, error) { - params := &domain.MessageQueryParams{ - Limit: limit, - } - - if cmd.Flags().Changed("unread") { - params.Unread = &unread - } - if cmd.Flags().Changed("starred") { - params.Starred = &starred - } - if from != "" { - params.From = from - } - if metadataPair != "" { - params.MetadataPair = metadataPair - } - - // Default to INBOX unless --all-folders is set or specific folder is provided - if folder != "" { - // Resolve folder name to ID if needed (for Microsoft accounts) - resolvedFolder, err := resolveFolderName(ctx, client, grantID, folder) - if err != nil { - // API error - warn user but continue with literal name - _, _ = fmt.Fprintf(cmd.ErrOrStderr(), "Warning: could not resolve folder '%s': %v\n", folder, err) - params.In = []string{folder} - } else if resolvedFolder != "" { - params.In = []string{resolvedFolder} - } else { - // Folder not found by name, use literal - params.In = []string{folder} - } - } else if !allFolders { - // Try to find inbox folder ID (works for both Google and Microsoft) - inboxID, err := resolveFolderName(ctx, client, grantID, "INBOX") - if err != nil { - // API error - warn but fallback to literal INBOX - _, _ = fmt.Fprintf(cmd.ErrOrStderr(), "Warning: could not resolve INBOX folder: %v\n", err) - params.In = []string{"INBOX"} - } else if inboxID != "" { - params.In = []string{inboxID} - } else { - // Fallback to INBOX (works for Google) - params.In = []string{"INBOX"} - } - } - - messages, err := fetchMessages(ctx, client, grantID, params, maxItems) + messages, err := fetchListMessages(ctx, cmd, client, grantID, opts) if err != nil { return struct{}{}, common.WrapFetchError("messages", err) } @@ -146,6 +105,59 @@ Use --max to limit total messages when using --all.`, return cmd } +type listOptions struct { + limit int + unread bool + starred bool + from string + folder string + allFolders bool + all bool + maxItems int + metadataPair string +} + +func resolveListPagination(limit int, all bool, maxItems int) (int, int) { + pag := common.SetupPagination(limit, all, maxItems) + limit = pag.Limit + + switch pag.Mode { + case common.PaginateSinglePage: + return limit, -1 // fetchMessages uses -1 for single-page + case common.PaginateAll: + return limit, 0 // fetchMessages uses 0 for unlimited + case common.PaginateWithCap: + return limit, pag.MaxItems + default: + return limit, -1 + } +} + +func fetchListMessages(ctx context.Context, cmd *cobra.Command, client ports.NylasClient, grantID string, opts listOptions) ([]domain.Message, error) { + limit, maxItems := resolveListPagination(opts.limit, opts.all, opts.maxItems) + + params := &domain.MessageQueryParams{ + Limit: limit, + } + + if cmd.Flags().Changed("unread") { + params.Unread = &opts.unread + } + if cmd.Flags().Changed("starred") { + params.Starred = &opts.starred + } + if opts.from != "" { + params.From = opts.from + } + if opts.metadataPair != "" { + params.MetadataPair = opts.metadataPair + } + + applyListFolderFilter(ctx, cmd.ErrOrStderr(), client, grantID, params, opts.folder, opts.allFolders) + + return fetchMessages(ctx, client, grantID, params, maxItems) +} + // resolveFolderName looks up a folder by name and returns its ID. // This is needed for Microsoft accounts which use folder IDs, not names like "INBOX". // For Google accounts, this will just return the original name if no match is found. @@ -196,67 +208,61 @@ func resolveFolderName(ctx context.Context, client ports.NylasClient, grantID, f return "", nil } -// runListStructured handles structured output (JSON/YAML/quiet) for the list command. -func runListStructured(cmd *cobra.Command, args []string, limit int, unread, starred bool, - from, folder string, allFolders, all bool, maxItems int, metadataPair string) error { - - // Auto-paginate when limit exceeds API maximum - if limit > common.MaxAPILimit && !all { - maxItems = limit - limit = common.MaxAPILimit - } else if !all { - maxItems = -1 // single-page fetch - } - - _, err := common.WithClient(args, func(ctx context.Context, client ports.NylasClient, grantID string) (struct{}, error) { - params := &domain.MessageQueryParams{ - Limit: limit, - } - - if cmd.Flags().Changed("unread") { - params.Unread = &unread - } - if cmd.Flags().Changed("starred") { - params.Starred = &starred - } - if from != "" { - params.From = from +func applyListFolderFilter(ctx context.Context, stderr io.Writer, client ports.NylasClient, grantID string, params *domain.MessageQueryParams, folder string, allFolders bool) { + if folder != "" { + // Resolve folder name to ID if needed (for Microsoft accounts) + resolvedFolder, err := resolveFolderName(ctx, client, grantID, folder) + if err != nil { + // API error - warn user but continue with literal name + _, _ = fmt.Fprintf(stderr, "Warning: could not resolve folder '%s': %v\n", folder, err) + params.In = []string{folder} + return } - if metadataPair != "" { - params.MetadataPair = metadataPair + if resolvedFolder != "" { + params.In = []string{resolvedFolder} + return } - // Default to INBOX unless --all-folders is set or specific folder is provided - if folder != "" { - resolvedFolder, err := resolveFolderName(ctx, client, grantID, folder) - if err != nil { - _, _ = fmt.Fprintf(cmd.ErrOrStderr(), "Warning: could not resolve folder '%s': %v\n", folder, err) - params.In = []string{folder} - } else if resolvedFolder != "" { - params.In = []string{resolvedFolder} - } else { - params.In = []string{folder} - } - } else if !allFolders { - inboxID, err := resolveFolderName(ctx, client, grantID, "INBOX") - if err != nil { - _, _ = fmt.Fprintf(cmd.ErrOrStderr(), "Warning: could not resolve INBOX folder: %v\n", err) - params.In = []string{"INBOX"} - } else if inboxID != "" { - params.In = []string{inboxID} - } else { - params.In = []string{"INBOX"} - } - } + // Folder not found by name, use literal + params.In = []string{folder} + return + } - messages, err := fetchMessages(ctx, client, grantID, params, maxItems) - if err != nil { - return struct{}{}, common.WrapFetchError("messages", err) - } + if allFolders { + return + } + + // Try to find inbox folder ID (works for both Google and Microsoft) + inboxID, err := resolveFolderName(ctx, client, grantID, "INBOX") + if err != nil { + // API error - warn but fallback to literal INBOX + _, _ = fmt.Fprintf(stderr, "Warning: could not resolve INBOX folder: %v\n", err) + params.In = []string{"INBOX"} + return + } + if inboxID != "" { + params.In = []string{inboxID} + return + } + + // Fallback to INBOX (works for Google) + params.In = []string{"INBOX"} +} - // Output structured data - out := common.GetOutputWriter(cmd) - return struct{}{}, out.Write(messages) +// runListStructured handles structured output (JSON/YAML/quiet) for the list command. +func runListStructured(cmd *cobra.Command, args []string, opts listOptions) error { + _, err := common.WithClient(args, func(ctx context.Context, client ports.NylasClient, grantID string) (struct{}, error) { + return struct{}{}, writeListStructured(ctx, cmd, client, grantID, opts) }) return err } + +func writeListStructured(ctx context.Context, cmd *cobra.Command, client ports.NylasClient, grantID string, opts listOptions) error { + messages, err := fetchListMessages(ctx, cmd, client, grantID, opts) + if err != nil { + return common.WrapFetchError("messages", err) + } + + out := common.GetOutputWriter(cmd) + return out.Write(messages) +} diff --git a/internal/cli/email/list_test.go b/internal/cli/email/list_test.go new file mode 100644 index 0000000..78c5ba2 --- /dev/null +++ b/internal/cli/email/list_test.go @@ -0,0 +1,281 @@ +package email + +import ( + "bytes" + "context" + "encoding/json" + "errors" + "testing" + + "github.com/nylas/cli/internal/adapters/nylas" + "github.com/nylas/cli/internal/domain" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestResolveListPagination(t *testing.T) { + tests := []struct { + name string + limit int + all bool + maxItems int + wantLimit int + wantMaxItems int + }{ + { + name: "single page keeps limit and uses direct fetch sentinel", + limit: 25, + wantLimit: 25, + wantMaxItems: -1, + }, + { + name: "all fetches unlimited pages", + limit: 25, + all: true, + wantLimit: 200, + wantMaxItems: 0, + }, + { + name: "all with cap preserves cap", + limit: 25, + all: true, + maxItems: 500, + wantLimit: 200, + wantMaxItems: 500, + }, + { + name: "large limit auto paginates with cap", + limit: 350, + wantLimit: 200, + wantMaxItems: 350, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + gotLimit, gotMaxItems := resolveListPagination(tt.limit, tt.all, tt.maxItems) + assert.Equal(t, tt.wantLimit, gotLimit) + assert.Equal(t, tt.wantMaxItems, gotMaxItems) + }) + } +} + +type testListClient struct { + *nylas.MockClient + getMessagesWithCursorFunc func(ctx context.Context, grantID string, params *domain.MessageQueryParams) (*domain.MessageListResponse, error) +} + +func (c *testListClient) GetMessagesWithCursor(ctx context.Context, grantID string, params *domain.MessageQueryParams) (*domain.MessageListResponse, error) { + if c.getMessagesWithCursorFunc != nil { + return c.getMessagesWithCursorFunc(ctx, grantID, params) + } + return c.MockClient.GetMessagesWithCursor(ctx, grantID, params) +} + +func TestFetchListMessages(t *testing.T) { + t.Run("applies explicit folder resolution and filters", func(t *testing.T) { + cmd := newListCmd() + err := cmd.Flags().Set("unread", "true") + require.NoError(t, err) + err = cmd.Flags().Set("starred", "true") + require.NoError(t, err) + + client := &testListClient{ + MockClient: nylas.NewMockClient(), + } + client.GetFoldersFunc = func(ctx context.Context, grantID string) ([]domain.Folder, error) { + return []domain.Folder{ + {ID: "folder-123", Name: "Sent Items"}, + }, nil + } + client.GetMessagesWithParamsFunc = func(ctx context.Context, grantID string, params *domain.MessageQueryParams) ([]domain.Message, error) { + require.NotNil(t, params.Unread) + require.NotNil(t, params.Starred) + assert.True(t, *params.Unread) + assert.True(t, *params.Starred) + assert.Equal(t, []string{"folder-123"}, params.In) + assert.Equal(t, "boss@example.com", params.From) + assert.Equal(t, "key:value", params.MetadataPair) + return []domain.Message{{ID: "msg-1"}}, nil + } + + messages, err := fetchListMessages(context.Background(), cmd, client, "grant-123", listOptions{ + limit: 25, + unread: true, + starred: true, + from: "boss@example.com", + folder: "Sent Items", + metadataPair: "key:value", + }) + + require.NoError(t, err) + assert.Len(t, messages, 1) + assert.Equal(t, "msg-1", messages[0].ID) + }) + + t.Run("warns and falls back to INBOX on folder resolution error", func(t *testing.T) { + cmd := newListCmd() + var stderr bytes.Buffer + cmd.SetErr(&stderr) + + client := &testListClient{ + MockClient: nylas.NewMockClient(), + } + client.GetFoldersFunc = func(ctx context.Context, grantID string) ([]domain.Folder, error) { + return nil, errors.New("folder lookup failed") + } + client.GetMessagesWithParamsFunc = func(ctx context.Context, grantID string, params *domain.MessageQueryParams) ([]domain.Message, error) { + assert.Equal(t, []string{"INBOX"}, params.In) + return []domain.Message{{ID: "msg-2"}}, nil + } + + messages, err := fetchListMessages(context.Background(), cmd, client, "grant-123", listOptions{ + limit: 15, + }) + + require.NoError(t, err) + assert.Len(t, messages, 1) + assert.Contains(t, stderr.String(), "could not resolve INBOX folder") + }) + + t.Run("falls back to literal folder when explicit folder resolution fails", func(t *testing.T) { + cmd := newListCmd() + var stderr bytes.Buffer + cmd.SetErr(&stderr) + + client := &testListClient{ + MockClient: nylas.NewMockClient(), + } + client.GetFoldersFunc = func(ctx context.Context, grantID string) ([]domain.Folder, error) { + return nil, errors.New("folder lookup failed") + } + client.GetMessagesWithParamsFunc = func(ctx context.Context, grantID string, params *domain.MessageQueryParams) ([]domain.Message, error) { + assert.Equal(t, []string{"SENT"}, params.In) + return []domain.Message{{ID: "msg-3"}}, nil + } + + messages, err := fetchListMessages(context.Background(), cmd, client, "grant-123", listOptions{ + limit: 15, + folder: "SENT", + }) + + require.NoError(t, err) + assert.Len(t, messages, 1) + assert.Contains(t, stderr.String(), "could not resolve folder 'SENT'") + }) + + t.Run("leaves folder filter unset when all folders requested", func(t *testing.T) { + cmd := newListCmd() + client := &testListClient{ + MockClient: nylas.NewMockClient(), + } + client.GetMessagesWithParamsFunc = func(ctx context.Context, grantID string, params *domain.MessageQueryParams) ([]domain.Message, error) { + assert.Nil(t, params.In) + return []domain.Message{{ID: "msg-4"}}, nil + } + + messages, err := fetchListMessages(context.Background(), cmd, client, "grant-123", listOptions{ + limit: 15, + allFolders: true, + }) + + require.NoError(t, err) + assert.Len(t, messages, 1) + }) + + t.Run("uses literal folder when no match found", func(t *testing.T) { + cmd := newListCmd() + client := &testListClient{ + MockClient: nylas.NewMockClient(), + } + client.GetFoldersFunc = func(ctx context.Context, grantID string) ([]domain.Folder, error) { + return []domain.Folder{ + {ID: "folder-1", Name: "Archive"}, + }, nil + } + client.GetMessagesWithParamsFunc = func(ctx context.Context, grantID string, params *domain.MessageQueryParams) ([]domain.Message, error) { + assert.Equal(t, []string{"CustomFolder"}, params.In) + return []domain.Message{{ID: "msg-5"}}, nil + } + + messages, err := fetchListMessages(context.Background(), cmd, client, "grant-123", listOptions{ + limit: 15, + folder: "CustomFolder", + }) + + require.NoError(t, err) + assert.Len(t, messages, 1) + }) + + t.Run("uses cursor pagination for all mode", func(t *testing.T) { + cmd := newListCmd() + client := &testListClient{ + MockClient: nylas.NewMockClient(), + getMessagesWithCursorFunc: func(ctx context.Context, grantID string, params *domain.MessageQueryParams) (*domain.MessageListResponse, error) { + switch params.PageToken { + case "": + return &domain.MessageListResponse{ + Data: []domain.Message{{ID: "msg-1"}}, + Pagination: domain.Pagination{ + NextCursor: "next", + }, + }, nil + case "next": + return &domain.MessageListResponse{ + Data: []domain.Message{{ID: "msg-2"}}, + }, nil + default: + return nil, errors.New("unexpected token") + } + }, + } + client.GetFoldersFunc = func(ctx context.Context, grantID string) ([]domain.Folder, error) { + return []domain.Folder{ + {ID: "inbox-id", Name: "Inbox"}, + }, nil + } + + messages, err := fetchListMessages(context.Background(), cmd, client, "grant-123", listOptions{ + limit: 10, + all: true, + }) + + require.NoError(t, err) + assert.Len(t, messages, 2) + assert.Equal(t, []string{"msg-1", "msg-2"}, []string{messages[0].ID, messages[1].ID}) + }) +} + +func TestRunListStructured(t *testing.T) { + cmd := newListCmd() + cmd.Flags().Bool("json", false, "Output in JSON format") + var stdout bytes.Buffer + cmd.SetOut(&stdout) + cmd.SetErr(&bytes.Buffer{}) + err := cmd.Flags().Set("json", "true") + require.NoError(t, err) + err = cmd.Flags().Set("unread", "true") + require.NoError(t, err) + + client := &testListClient{ + MockClient: nylas.NewMockClient(), + } + client.GetFoldersFunc = func(ctx context.Context, grantID string) ([]domain.Folder, error) { + return []domain.Folder{{ID: "inbox-id", Name: "Inbox"}}, nil + } + client.GetMessagesWithParamsFunc = func(ctx context.Context, grantID string, params *domain.MessageQueryParams) ([]domain.Message, error) { + return []domain.Message{{ID: "msg-structured"}}, nil + } + + resultErr := writeListStructured(context.Background(), cmd, client, "grant-123", listOptions{ + limit: 10, + unread: true, + }) + require.NoError(t, resultErr) + + var decoded []domain.Message + err = json.Unmarshal(stdout.Bytes(), &decoded) + require.NoError(t, err) + require.Len(t, decoded, 1) + assert.Equal(t, "msg-structured", decoded[0].ID) +} diff --git a/internal/cli/email/search.go b/internal/cli/email/search.go index 4ac2e6f..90efade 100644 --- a/internal/cli/email/search.go +++ b/internal/cli/email/search.go @@ -161,10 +161,7 @@ func fetchMessages(ctx context.Context, client messagesClient, grantID string, p return client.GetMessagesWithParams(ctx, grantID, params) } - pageSize := min(params.Limit, common.MaxAPILimit) - if pageSize <= 0 { - pageSize = common.MaxAPILimit - } + pageSize := common.NormalizePageSize(params.Limit) params.Limit = pageSize fetcher := func(ctx context.Context, cursor string) (common.PageResult[domain.Message], error) { @@ -179,11 +176,7 @@ func fetchMessages(ctx context.Context, client messagesClient, grantID string, p }, nil } - config := common.DefaultPaginationConfig() - config.PageSize = pageSize - config.MaxItems = maxItems - - return common.FetchAllPages(ctx, config, fetcher) + return common.FetchCursorPages(ctx, pageSize, maxItems, fetcher) } // parseDate parses a date string in YYYY-MM-DD format using local timezone. diff --git a/internal/cli/notetaker/list.go b/internal/cli/notetaker/list.go index 2ea2332..7a4504e 100644 --- a/internal/cli/notetaker/list.go +++ b/internal/cli/notetaker/list.go @@ -95,24 +95,13 @@ func newListCmd() *cobra.Command { } func formatState(state string) string { + // Map API state names to display-friendly labels + displayName := state switch state { - case domain.NotetakerStateScheduled: - return common.Yellow.Sprint("scheduled") - case domain.NotetakerStateConnecting: - return common.Cyan.Sprint("connecting") case domain.NotetakerStateWaitingForEntry: - return common.Cyan.Sprint("waiting") - case domain.NotetakerStateAttending: - return common.Green.Sprint("attending") + displayName = "waiting" case domain.NotetakerStateMediaProcessing: - return common.Cyan.Sprint("processing") - case domain.NotetakerStateComplete: - return common.Green.Sprint("complete") - case domain.NotetakerStateCancelled: - return common.Dim.Sprint("cancelled") - case domain.NotetakerStateFailed: - return common.Red.Sprint("failed") - default: - return state + displayName = "processing" } + return common.StatusColor(state).Sprint(displayName) } diff --git a/internal/cli/scheduler/bookings.go b/internal/cli/scheduler/bookings.go index dae5526..1fe85fa 100644 --- a/internal/cli/scheduler/bookings.go +++ b/internal/cli/scheduler/bookings.go @@ -6,15 +6,12 @@ import ( "fmt" "time" - "github.com/fatih/color" "github.com/nylas/cli/internal/cli/common" "github.com/nylas/cli/internal/domain" "github.com/nylas/cli/internal/ports" "github.com/spf13/cobra" ) -// Note: fatih/color import needed for getStatusColor return type (*color.Color) - func newBookingsCmd() *cobra.Command { cmd := &cobra.Command{ Use: "bookings", @@ -63,18 +60,8 @@ func newBookingListCmd() *cobra.Command { table := common.NewTable("TITLE", "ID", "START TIME", "STATUS") for _, b := range bookings { - status := b.Status - switch b.Status { - case "confirmed": - status = common.Green.Sprint(status) - case "pending": - status = common.Yellow.Sprint(status) - case "cancelled": - status = common.Dim.Sprint(status) - } - startTime := b.StartTime.Format("2006-01-02 15:04") - table.AddRow(common.Cyan.Sprint(b.Title), b.BookingID, startTime, status) + table.AddRow(common.Cyan.Sprint(b.Title), b.BookingID, startTime, common.ColorSprint(b.Status)) } table.Render() @@ -112,7 +99,7 @@ func newBookingShowCmd() *cobra.Command { _, _ = common.Bold.Printf("Booking: %s\n", booking.Title) fmt.Printf(" ID: %s\n", common.Cyan.Sprint(booking.BookingID)) - fmt.Printf(" Status: %s\n", getStatusColor(booking.Status).Sprint(booking.Status)) + fmt.Printf(" Status: %s\n", common.ColorSprint(booking.Status)) fmt.Printf(" Start: %s\n", booking.StartTime.Format(time.RFC1123)) fmt.Printf(" End: %s\n", booking.EndTime.Format(time.RFC1123)) @@ -301,16 +288,3 @@ func newBookingCancelCmd() *cobra.Command { return cmd } - -func getStatusColor(status string) *color.Color { - switch status { - case "confirmed": - return common.Green - case "pending": - return common.Yellow - case "cancelled": - return common.Dim - default: - return common.Reset - } -} diff --git a/internal/cli/webhook/list.go b/internal/cli/webhook/list.go index 23e9872..7185260 100644 --- a/internal/cli/webhook/list.go +++ b/internal/cli/webhook/list.go @@ -3,7 +3,6 @@ package webhook import ( "context" "encoding/csv" - "encoding/json" "fmt" "os" "strings" @@ -89,50 +88,35 @@ Shows webhook ID, description, URL, status, and trigger types.`, return cmd } -func outputJSON(webhooks any) error { +func outputJSON(webhooks []domain.Webhook) error { return common.PrintJSON(webhooks) } -func outputYAML(webhooks any) error { +func outputYAML(webhooks []domain.Webhook) error { return yaml.NewEncoder(os.Stdout).Encode(webhooks) } -func outputCSV(webhooks any) error { +func outputCSV(webhooks []domain.Webhook) error { w := csv.NewWriter(os.Stdout) defer w.Flush() // Write header _ = w.Write([]string{"ID", "Description", "URL", "Status", "Triggers"}) - // Get webhooks as slice - data, _ := json.Marshal(webhooks) - var items []map[string]any - _ = json.Unmarshal(data, &items) - - for _, item := range items { - id, _ := item["id"].(string) - desc, _ := item["description"].(string) - url, _ := item["webhook_url"].(string) - status, _ := item["status"].(string) - - var triggers []string - if triggerList, ok := item["trigger_types"].([]any); ok { - for _, t := range triggerList { - triggers = append(triggers, fmt.Sprintf("%v", t)) - } - } - - _ = w.Write([]string{id, desc, url, status, strings.Join(triggers, ";")}) + for _, webhook := range webhooks { + _ = w.Write([]string{ + webhook.ID, + webhook.Description, + webhook.WebhookURL, + webhook.Status, + strings.Join(webhook.TriggerTypes, ";"), + }) } return nil } -func outputTable(webhooks any, fullIDs bool) error { - data, _ := json.Marshal(webhooks) - var items []map[string]any - _ = json.Unmarshal(data, &items) - +func outputTable(webhooks []domain.Webhook, fullIDs bool) error { // Calculate column widths headers := []string{"ID", "Description", "URL", "Status", "Triggers"} widths := make([]int, len(headers)) @@ -145,25 +129,19 @@ func outputTable(webhooks any, fullIDs bool) error { } var rows []row - for _, item := range items { - id := fmt.Sprintf("%v", item["id"]) + for _, webhook := range webhooks { + id := webhook.ID if !fullIDs { id = common.Truncate(id, 20) } r := row{ id: id, - desc: common.Truncate(fmt.Sprintf("%v", item["description"]), 25), - url: common.Truncate(fmt.Sprintf("%v", item["webhook_url"]), 35), - status: fmt.Sprintf("%v", item["status"]), + desc: common.Truncate(webhook.Description, 25), + url: common.Truncate(webhook.WebhookURL, 35), + status: webhook.Status, } - var triggers []string - if triggerList, ok := item["trigger_types"].([]any); ok { - for _, t := range triggerList { - triggers = append(triggers, fmt.Sprintf("%v", t)) - } - } - r.triggers = common.Truncate(strings.Join(triggers, ", "), 30) + r.triggers = common.Truncate(strings.Join(webhook.TriggerTypes, ", "), 30) rows = append(rows, r) @@ -203,7 +181,7 @@ func outputTable(webhooks any, fullIDs bool) error { // Print rows for _, r := range rows { - statusIcon := getStatusIcon(r.status) + statusIcon := common.StatusIcon(r.status) fmt.Printf("%-*s %-*s %-*s %s %-*s %s\n", widths[0], r.id, widths[1], r.desc, @@ -215,16 +193,3 @@ func outputTable(webhooks any, fullIDs bool) error { fmt.Printf("\nTotal: %d webhooks\n", len(rows)) return nil } - -func getStatusIcon(status string) string { - switch status { - case "active": - return common.Green.Sprint("●") - case "inactive": - return common.Yellow.Sprint("●") - case "failing": - return common.Red.Sprint("●") - default: - return "○" - } -} diff --git a/internal/cli/webhook/render_test.go b/internal/cli/webhook/render_test.go new file mode 100644 index 0000000..e8fd676 --- /dev/null +++ b/internal/cli/webhook/render_test.go @@ -0,0 +1,155 @@ +package webhook + +import ( + "encoding/json" + "io" + "os" + "testing" + "time" + + "github.com/fatih/color" + "github.com/nylas/cli/internal/domain" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestDisplayWebhookDetails_TypedRendering(t *testing.T) { + webhook := &domain.Webhook{ + ID: "webhook-1234567890", + Description: "Test webhook", + WebhookURL: "https://example.com/webhook", + WebhookSecret: "secret-12345678", + Status: "active", + TriggerTypes: []string{"message.created", "message.updated"}, + NotificationEmailAddresses: []string{"alerts@example.com"}, + CreatedAt: time.Date(2024, time.January, 2, 3, 4, 5, 0, time.UTC), + UpdatedAt: time.Date(2024, time.January, 3, 4, 5, 6, 0, time.UTC), + StatusUpdatedAt: time.Date(2024, time.January, 4, 5, 6, 7, 0, time.UTC), + } + + output := captureStdout(t, func() { + err := displayWebhookDetails(webhook) + require.NoError(t, err) + }) + + assert.Contains(t, output, "Webhook: webhook-1234567890") + assert.Contains(t, output, "Description: Test webhook") + assert.Contains(t, output, "URL: https://example.com/webhook") + assert.Contains(t, output, "message.created") + assert.Contains(t, output, "alerts@example.com") + assert.Contains(t, output, "2024-01-02T03:04:05Z") + assert.NotContains(t, output, "secret-12345678") +} + +func TestOutputTable_TypedRendering(t *testing.T) { + color.NoColor = true + defer func() { color.NoColor = false }() + + webhooks := []domain.Webhook{ + { + ID: "webhook-1234567890", + Description: "Test webhook", + WebhookURL: "https://example.com/webhook", + Status: "active", + TriggerTypes: []string{"message.created", "message.updated"}, + }, + } + + output := captureStdout(t, func() { + err := outputTable(webhooks, false) + require.NoError(t, err) + }) + + assert.Contains(t, output, "Description") + assert.Contains(t, output, "https://example.com/webhook") + assert.Contains(t, output, "message.created") + assert.Contains(t, output, "Total: 1 webhooks") +} + +func TestOutputTable_Branches(t *testing.T) { + color.NoColor = true + defer func() { color.NoColor = false }() + + webhooks := []domain.Webhook{ + { + ID: "webhook-1234567890", + WebhookURL: "https://example.com/webhook", + Status: "unknown", + TriggerTypes: nil, + }, + } + + output := captureStdout(t, func() { + err := outputTable(webhooks, true) + require.NoError(t, err) + }) + + assert.Contains(t, output, "webhook-1234567890") + assert.Contains(t, output, "○ unknown") +} + +func TestOutputHelpers(t *testing.T) { + webhooks := []domain.Webhook{ + { + ID: "webhook-123", + Description: "Test webhook", + WebhookURL: "https://example.com/webhook", + Status: "active", + TriggerTypes: []string{"message.created", "message.updated"}, + }, + } + + t.Run("outputJSON", func(t *testing.T) { + output := captureStdout(t, func() { + err := outputJSON(webhooks) + require.NoError(t, err) + }) + + var decoded []domain.Webhook + err := json.Unmarshal([]byte(output), &decoded) + require.NoError(t, err) + require.Len(t, decoded, 1) + assert.Equal(t, "webhook-123", decoded[0].ID) + }) + + t.Run("outputYAML", func(t *testing.T) { + output := captureStdout(t, func() { + err := outputYAML(webhooks) + require.NoError(t, err) + }) + + assert.Contains(t, output, "id: webhook-123") + assert.Contains(t, output, "status: active") + }) + + t.Run("outputCSV", func(t *testing.T) { + output := captureStdout(t, func() { + err := outputCSV(webhooks) + require.NoError(t, err) + }) + + assert.Contains(t, output, "ID,Description,URL,Status,Triggers") + assert.Contains(t, output, "webhook-123,Test webhook,https://example.com/webhook,active,message.created;message.updated") + }) +} + +func captureStdout(t *testing.T, fn func()) string { + t.Helper() + + oldStdout := os.Stdout + r, w, err := os.Pipe() + require.NoError(t, err) + os.Stdout = w + + defer func() { + os.Stdout = oldStdout + }() + + fn() + + require.NoError(t, w.Close()) + output, err := io.ReadAll(r) + require.NoError(t, err) + require.NoError(t, r.Close()) + return string(output) +} diff --git a/internal/cli/webhook/show.go b/internal/cli/webhook/show.go index 17624fe..c331f3e 100644 --- a/internal/cli/webhook/show.go +++ b/internal/cli/webhook/show.go @@ -5,6 +5,7 @@ import ( "encoding/json" "fmt" "strings" + "time" "github.com/nylas/cli/internal/cli/common" "github.com/nylas/cli/internal/domain" @@ -60,70 +61,51 @@ status, and notification settings.`, return cmd } -func displayWebhookDetails(webhook any) error { - data, _ := json.Marshal(webhook) - var w map[string]any - _ = json.Unmarshal(data, &w) +func displayWebhookDetails(webhook *domain.Webhook) error { + statusIcon := common.StatusIcon(webhook.Status) - id := getString(w, "id") - desc := getString(w, "description") - url := getString(w, "webhook_url") - secret := getString(w, "webhook_secret") - status := getString(w, "status") - - statusIcon := getStatusIcon(status) - - fmt.Printf("Webhook: %s\n", id) + fmt.Printf("Webhook: %s\n", webhook.ID) fmt.Println(strings.Repeat("─", 60)) - if desc != "" { - fmt.Printf("Description: %s\n", desc) + if webhook.Description != "" { + fmt.Printf("Description: %s\n", webhook.Description) } - fmt.Printf("URL: %s\n", url) - fmt.Printf("Status: %s %s\n", statusIcon, status) + fmt.Printf("URL: %s\n", webhook.WebhookURL) + fmt.Printf("Status: %s %s\n", statusIcon, webhook.Status) - if secret != "" { - fmt.Printf("Secret: %s\n", maskSecret(secret)) + if webhook.WebhookSecret != "" { + fmt.Printf("Secret: %s\n", maskSecret(webhook.WebhookSecret)) } // Trigger types fmt.Println("\nTrigger Types:") - if triggers, ok := w["trigger_types"].([]any); ok { - for _, t := range triggers { - fmt.Printf(" • %s\n", t) - } + for _, trigger := range webhook.TriggerTypes { + fmt.Printf(" • %s\n", trigger) } // Notification emails - if emails, ok := w["notification_email_addresses"].([]any); ok && len(emails) > 0 { + if len(webhook.NotificationEmailAddresses) > 0 { fmt.Println("\nNotification Emails:") - for _, e := range emails { - fmt.Printf(" • %s\n", e) + for _, email := range webhook.NotificationEmailAddresses { + fmt.Printf(" • %s\n", email) } } // Timestamps fmt.Println("\nTimestamps:") - if created := getString(w, "created_at"); created != "" { - fmt.Printf(" Created: %s\n", created) + if !webhook.CreatedAt.IsZero() { + fmt.Printf(" Created: %s\n", webhook.CreatedAt.Format(time.RFC3339)) } - if updated := getString(w, "updated_at"); updated != "" { - fmt.Printf(" Updated: %s\n", updated) + if !webhook.UpdatedAt.IsZero() { + fmt.Printf(" Updated: %s\n", webhook.UpdatedAt.Format(time.RFC3339)) } - if statusUpdated := getString(w, "status_updated_at"); statusUpdated != "" { - fmt.Printf(" Status Updated: %s\n", statusUpdated) + if !webhook.StatusUpdatedAt.IsZero() { + fmt.Printf(" Status Updated: %s\n", webhook.StatusUpdatedAt.Format(time.RFC3339)) } return nil } -func getString(m map[string]any, key string) string { - if v, ok := m[key]; ok { - return fmt.Sprintf("%v", v) - } - return "" -} - // maskSecret masks a secret for display, showing first 4 and last 4 characters. // Handles edge cases for short secrets to prevent panics. func maskSecret(secret string) string { diff --git a/internal/cli/webhook/update.go b/internal/cli/webhook/update.go index 7008e71..b2362f6 100644 --- a/internal/cli/webhook/update.go +++ b/internal/cli/webhook/update.go @@ -125,7 +125,7 @@ You can update the URL, triggers, description, notification emails, or status.`, fmt.Println() fmt.Printf(" ID: %s\n", webhook.ID) fmt.Printf(" URL: %s\n", webhook.WebhookURL) - fmt.Printf(" Status: %s %s\n", getStatusIcon(webhook.Status), webhook.Status) + fmt.Printf(" Status: %s %s\n", common.StatusIcon(webhook.Status), webhook.Status) if len(webhook.TriggerTypes) > 0 { fmt.Println("\nTriggers:") diff --git a/internal/cli/webhook/webhook_advanced_test.go b/internal/cli/webhook/webhook_advanced_test.go index 76cce43..4d50e8c 100644 --- a/internal/cli/webhook/webhook_advanced_test.go +++ b/internal/cli/webhook/webhook_advanced_test.go @@ -58,26 +58,23 @@ func TestHelperFunctions(t *testing.T) { assert.Equal(t, "hello", result) }) - t.Run("getStatusIcon_active", func(t *testing.T) { - result := getStatusIcon("active") + t.Run("StatusIcon_active", func(t *testing.T) { + result := common.StatusIcon("active") assert.Contains(t, result, "●") - // Color codes are only present in TTY environments }) - t.Run("getStatusIcon_inactive", func(t *testing.T) { - result := getStatusIcon("inactive") + t.Run("StatusIcon_inactive", func(t *testing.T) { + result := common.StatusIcon("inactive") assert.Contains(t, result, "●") - // Color codes are only present in TTY environments }) - t.Run("getStatusIcon_failing", func(t *testing.T) { - result := getStatusIcon("failing") + t.Run("StatusIcon_failing", func(t *testing.T) { + result := common.StatusIcon("failing") assert.Contains(t, result, "●") - // Color codes are only present in TTY environments }) - t.Run("getStatusIcon_unknown", func(t *testing.T) { - result := getStatusIcon("unknown") + t.Run("StatusIcon_unknown", func(t *testing.T) { + result := common.StatusIcon("unknown") assert.Equal(t, "○", result) }) diff --git a/internal/testutil/helpers.go b/internal/testutil/helpers.go index a54c7cc..63cf1e5 100644 --- a/internal/testutil/helpers.go +++ b/internal/testutil/helpers.go @@ -3,6 +3,9 @@ package testutil import ( + "bytes" + "encoding/json" + "net/http" "os" "path/filepath" "testing" @@ -254,6 +257,38 @@ func AssertNotEmpty[T any](t *testing.T, slice []T, msg string) { } } +// WriteJSONResponse writes a JSON response to an http.ResponseWriter in tests. +// This eliminates the duplicate pattern of setting Content-Type, writing status, +// and encoding JSON that appears 180+ times across test files. +// +// Safe to call from httptest.NewServer handler goroutines — on encode failure it +// writes a 500 error response instead of calling t.Fatalf (which is unsafe from +// non-test goroutines). +// +// Example: +// +// server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { +// testutil.WriteJSONResponse(t, w, http.StatusOK, map[string]string{"id": "123"}) +// })) +func WriteJSONResponse(t *testing.T, w http.ResponseWriter, statusCode int, data any) { + t.Helper() + + // Encode into a buffer first so that on failure we can return 500 + // instead of a success status with a broken body. Writing headers + // is deferred until we know encoding succeeded. + var buf bytes.Buffer + if err := json.NewEncoder(&buf).Encode(data); err != nil { + // t.Errorf is safe from any goroutine (unlike t.Fatalf). + t.Errorf("WriteJSONResponse: failed to encode JSON: %v", err) + http.Error(w, "WriteJSONResponse: encode failed", http.StatusInternalServerError) + return + } + + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(statusCode) + _, _ = w.Write(buf.Bytes()) +} + // Helper function to check if string contains substring func contains(s, substr string) bool { if len(substr) == 0 { diff --git a/internal/testutil/helpers_test.go b/internal/testutil/helpers_test.go index bab9c91..d6a098c 100644 --- a/internal/testutil/helpers_test.go +++ b/internal/testutil/helpers_test.go @@ -1,6 +1,9 @@ package testutil import ( + "encoding/json" + "net/http" + "net/http/httptest" "os" "testing" ) @@ -68,3 +71,97 @@ func TestContainsHelper(t *testing.T) { }) } } + +func TestWriteJSONResponse(t *testing.T) { + t.Run("writes status and JSON body", func(t *testing.T) { + data := map[string]string{"id": "123", "name": "test"} + + rec := httptest.NewRecorder() + WriteJSONResponse(t, rec, http.StatusOK, data) + + resp := rec.Result() + defer func() { _ = resp.Body.Close() }() + + if resp.StatusCode != http.StatusOK { + t.Errorf("status = %d, want %d", resp.StatusCode, http.StatusOK) + } + + if ct := resp.Header.Get("Content-Type"); ct != "application/json" { + t.Errorf("Content-Type = %q, want %q", ct, "application/json") + } + + var got map[string]string + if err := json.NewDecoder(resp.Body).Decode(&got); err != nil { + t.Fatalf("failed to decode response: %v", err) + } + if got["id"] != "123" { + t.Errorf("id = %q, want %q", got["id"], "123") + } + if got["name"] != "test" { + t.Errorf("name = %q, want %q", got["name"], "test") + } + }) + + t.Run("writes custom status codes", func(t *testing.T) { + rec := httptest.NewRecorder() + WriteJSONResponse(t, rec, http.StatusCreated, map[string]bool{"ok": true}) + + if rec.Code != http.StatusCreated { + t.Errorf("status = %d, want %d", rec.Code, http.StatusCreated) + } + }) + + t.Run("writes array data", func(t *testing.T) { + items := []string{"a", "b", "c"} + rec := httptest.NewRecorder() + WriteJSONResponse(t, rec, http.StatusOK, items) + + var got []string + if err := json.NewDecoder(rec.Body).Decode(&got); err != nil { + t.Fatalf("failed to decode response: %v", err) + } + if len(got) != 3 { + t.Errorf("len = %d, want 3", len(got)) + } + }) + + t.Run("integrates with httptest server", func(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + WriteJSONResponse(t, w, http.StatusOK, map[string]string{"status": "ok"}) + })) + defer server.Close() + + resp, err := http.Get(server.URL) //nolint:gosec // test URL + if err != nil { + t.Fatalf("request failed: %v", err) + } + defer func() { _ = resp.Body.Close() }() + + var result map[string]string + if err := json.NewDecoder(resp.Body).Decode(&result); err != nil { + t.Fatalf("decode failed: %v", err) + } + if result["status"] != "ok" { + t.Errorf("status = %q, want %q", result["status"], "ok") + } + }) + + t.Run("returns 500 on encode failure", func(t *testing.T) { + // json.Marshal cannot encode channels — use this to trigger failure. + unencodable := make(chan int) + + // Use a fake *testing.T so we can observe the Errorf call without + // failing the real test. + fakeT := &testing.T{} + + rec := httptest.NewRecorder() + WriteJSONResponse(fakeT, rec, http.StatusOK, unencodable) + + if rec.Code != http.StatusInternalServerError { + t.Errorf("status = %d, want %d", rec.Code, http.StatusInternalServerError) + } + if !fakeT.Failed() { + t.Error("expected fakeT to be marked as failed") + } + }) +} From 59130ba991da554c398f155d4d9da6e7c13791f9 Mon Sep 17 00:00:00 2001 From: Qasim Date: Sat, 28 Mar 2026 02:41:07 -0400 Subject: [PATCH 2/2] [TW-4734] docs(CLAUDE.md): remove git push rule, add shared helper learnings - Remove NEVER git commit/push rules (handled by global CLAUDE.md) - Add learnings for new shared helpers: StatusColor, SetupPagination, NormalizePageSize, FetchCursorPages, WriteJSONResponse - Consolidate Testing/Hooks/Context sections to reduce redundancy --- CLAUDE.md | 28 ++++++++-------------------- 1 file changed, 8 insertions(+), 20 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 394b8d8..d4e1768 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -7,8 +7,6 @@ Quick reference for AI assistants working on this codebase. ## ⛔ CRITICAL RULES - YOU MUST FOLLOW THESE ### NEVER DO (IMPORTANT - violations will break the workflow): -- **NEVER run `git commit`** - User commits manually -- **NEVER run `git push`** - User pushes manually - **NEVER commit secrets** - No API keys, tokens, passwords, .env files - **NEVER skip tests** - All changes require passing tests - **NEVER skip security scans** - Run `make security` before commits @@ -139,31 +137,17 @@ Credentials stored in system keyring (service: `"nylas"`) via `nylas auth config ## Testing -**Command:** `make ci-full` (complete CI: quality + tests + cleanup) - -**Quick checks:** `make ci` (no integration tests) - **Details:** `.claude/rules/testing.md` --- -## Hooks & Commands - -**Hooks:** Auto-enforce quality (blocks bad code, auto-formats). See `.claude/HOOKS-CONFIG.md` - -**Skills:** `/session-start`, `/run-tests`, `/add-command`, `/generate-tests`, `/security-scan` - -**Agents:** See `.claude/agents/README.md` for parallelization guide - ---- - -## Context & Session +## Hooks, Skills & Agents -**Token tips:** Use `/compact` mid-session, `/clear` for new tasks, `/mcp` to disable unused servers +**Hooks:** Auto-enforce quality. See `.claude/HOOKS-CONFIG.md` -**On-demand docs:** `docs/COMMANDS.md`, `docs/ARCHITECTURE.md`, `.claude/shared/patterns/*.md` +**Skills:** `/run-tests`, `/add-command`, `/generate-tests`, `/security-scan` -**Session handoff:** Use `/diary` skill to record progress after major tasks +**Agents:** See `.claude/agents/README.md` --- @@ -206,6 +190,10 @@ This section captures lessons learned from mistakes. Claude updates this section - AI clients: Use shared helpers in `adapters/ai/base_client.go` (ConvertMessagesToMaps, ConvertToolsOpenAIFormat, FallbackStreamChat) - Output formatting: Use `common.GetOutputWriter(cmd)` for JSON/YAML/quiet support, NEVER create custom --format flags - Client helpers: Use `common.WithClient()` and `WithClientNoGrant()` to reduce boilerplate, NEVER duplicate setup code +- Status colors: Use `common.StatusColor()`/`StatusIcon()`/`ColorSprint()`, NEVER create package-local status color functions +- Pagination: Use `common.SetupPagination()` for limit/maxItems resolution, NEVER duplicate auto-pagination logic in list commands +- Pagination helpers: Use `common.NormalizePageSize()` and `FetchCursorPages()` for cursor-based pagination boilerplate +- Test JSON responses: Use `testutil.WriteJSONResponse()` in httptest handlers, NEVER repeat the Content-Type/WriteHeader/Encode triplet ### Non-Obvious Workflows - Progressive disclosure: Keep main skill files under 100 lines, use references/ for details