From 733df0d7ed45c2a9afeee3fd12012888c9d47ce7 Mon Sep 17 00:00:00 2001 From: Ramon Niebla Date: Tue, 17 Mar 2026 20:33:31 -0700 Subject: [PATCH 1/2] [feat] Agent friendly error handling --- internal/errors/suggestions.go | 24 +++ internal/errors/suggestions_test.go | 75 +++++++++ internal/output/output_test.go | 15 ++ internal/output/plaintext_fns.go | 16 +- .../output/plaintext_fns_internal_test.go | 26 +++ internal/output/resource_output_test.go | 34 ++++ internal/resources/client.go | 40 ++++- internal/resources/client_test.go | 156 +++++++++++++++++- 8 files changed, 364 insertions(+), 22 deletions(-) create mode 100644 internal/errors/suggestions.go create mode 100644 internal/errors/suggestions_test.go diff --git a/internal/errors/suggestions.go b/internal/errors/suggestions.go new file mode 100644 index 00000000..860dea6b --- /dev/null +++ b/internal/errors/suggestions.go @@ -0,0 +1,24 @@ +package errors + +import "strings" + +var suggestions = map[int]string{ + 401: "Your access token may be invalid or expired. Run `ldcli login` or set LD_ACCESS_TOKEN. " + + "Create a new token at {baseURI}/settings/authorization.", + 403: "You don't have permission for this action. Check your access token's role " + + "and custom role policies in LaunchDarkly settings.", + 404: "Resource not found. Verify the project key, flag key, or environment key. " + + "Use `ldcli projects list` or `ldcli flags list` to see available resources.", + 409: "Conflict: the resource may have been modified since you last read it. " + + "Fetch the latest version and retry.", + 429: "Rate limited. Wait a moment and retry. If this persists, check your account's " + + "rate limit allocation.", +} + +func SuggestionForStatus(statusCode int, baseURI string) string { + s, ok := suggestions[statusCode] + if !ok { + return "" + } + return strings.ReplaceAll(s, "{baseURI}", baseURI) +} diff --git a/internal/errors/suggestions_test.go b/internal/errors/suggestions_test.go new file mode 100644 index 00000000..5d2fbe1c --- /dev/null +++ b/internal/errors/suggestions_test.go @@ -0,0 +1,75 @@ +package errors_test + +import ( + "testing" + + "github.com/stretchr/testify/assert" + + "github.com/launchdarkly/ldcli/internal/errors" +) + +func TestSuggestionForStatus(t *testing.T) { + t.Run("401 returns access token suggestion with baseURI", func(t *testing.T) { + result := errors.SuggestionForStatus(401, "https://app.launchdarkly.com") + + assert.Contains(t, result, "ldcli login") + assert.Contains(t, result, "LD_ACCESS_TOKEN") + assert.Contains(t, result, "https://app.launchdarkly.com/settings/authorization") + assert.NotContains(t, result, "{baseURI}") + }) + + t.Run("403 returns permission suggestion", func(t *testing.T) { + result := errors.SuggestionForStatus(403, "https://app.launchdarkly.com") + + assert.Contains(t, result, "permission") + assert.Contains(t, result, "custom role") + }) + + t.Run("404 returns resource not found suggestion", func(t *testing.T) { + result := errors.SuggestionForStatus(404, "https://app.launchdarkly.com") + + assert.Contains(t, result, "not found") + assert.Contains(t, result, "ldcli projects list") + assert.Contains(t, result, "ldcli flags list") + }) + + t.Run("409 returns conflict suggestion", func(t *testing.T) { + result := errors.SuggestionForStatus(409, "https://app.launchdarkly.com") + + assert.Contains(t, result, "Conflict") + assert.Contains(t, result, "latest version") + }) + + t.Run("429 returns rate limit suggestion", func(t *testing.T) { + result := errors.SuggestionForStatus(429, "https://app.launchdarkly.com") + + assert.Contains(t, result, "Rate limited") + assert.Contains(t, result, "retry") + }) + + t.Run("unknown status code returns empty string", func(t *testing.T) { + result := errors.SuggestionForStatus(502, "https://app.launchdarkly.com") + + assert.Empty(t, result) + }) + + t.Run("200 returns empty string", func(t *testing.T) { + result := errors.SuggestionForStatus(200, "https://app.launchdarkly.com") + + assert.Empty(t, result) + }) + + t.Run("replaces baseURI placeholder with actual value", func(t *testing.T) { + result := errors.SuggestionForStatus(401, "https://custom.ld.example.com") + + assert.Contains(t, result, "https://custom.ld.example.com/settings/authorization") + assert.NotContains(t, result, "{baseURI}") + }) + + t.Run("empty baseURI does not panic", func(t *testing.T) { + result := errors.SuggestionForStatus(401, "") + + assert.Contains(t, result, "ldcli login") + assert.Contains(t, result, "/settings/authorization") + }) +} diff --git a/internal/output/output_test.go b/internal/output/output_test.go index df701f9d..a10e2003 100644 --- a/internal/output/output_test.go +++ b/internal/output/output_test.go @@ -71,6 +71,21 @@ func TestCmdOutputSingular(t *testing.T) { fn: output.ErrorPlaintextOutputFn, input: `{}`, }, + "with an error with a suggestion": { + expected: "Not Found (code: not_found)\nSuggestion: Check the resource key.", + fn: output.ErrorPlaintextOutputFn, + input: `{"code": "not_found", "message": "Not Found", "suggestion": "Check the resource key."}`, + }, + "with an error with an empty suggestion": { + expected: "Internal Server Error (code: internal_server_error)", + fn: output.ErrorPlaintextOutputFn, + input: `{"code": "internal_server_error", "message": "Internal Server Error", "suggestion": ""}`, + }, + "with an error with only a message and a suggestion": { + expected: "an error\nSuggestion: Try again.", + fn: output.ErrorPlaintextOutputFn, + input: `{"message": "an error", "suggestion": "Try again."}`, + }, "with a singular resource": { expected: "test-name (test-key)", fn: output.SingularPlaintextOutputFn, diff --git a/internal/output/plaintext_fns.go b/internal/output/plaintext_fns.go index 566ffcc0..43d8ac71 100644 --- a/internal/output/plaintext_fns.go +++ b/internal/output/plaintext_fns.go @@ -28,16 +28,24 @@ var ConfigPlaintextOutputFn = func(r resource) string { // An error response could have a code and message or just a message. It's also possible that // there isn't either property. var ErrorPlaintextOutputFn = func(r resource) string { + var parts []string + switch { case r["code"] == nil && (r["message"] == "" || r["message"] == nil): - return "unknown error occurred" + parts = append(parts, "unknown error occurred") case r["code"] == nil: - return r["message"].(string) + parts = append(parts, fmt.Sprint(r["message"])) case r["message"] == "": - return fmt.Sprintf("an error occurred (code: %s)", r["code"]) + parts = append(parts, fmt.Sprintf("an error occurred (code: %v)", r["code"])) default: - return fmt.Sprintf("%s (code: %s)", r["message"], r["code"]) + parts = append(parts, fmt.Sprintf("%v (code: %v)", r["message"], r["code"])) + } + + if suggestion, ok := r["suggestion"]; ok && suggestion != nil && suggestion != "" { + parts = append(parts, fmt.Sprintf("\nSuggestion: %s", suggestion)) } + + return strings.Join(parts, "") } // MultiplePlaintextOutputFn converts the resource to plain text. diff --git a/internal/output/plaintext_fns_internal_test.go b/internal/output/plaintext_fns_internal_test.go index 7b379346..e4c39bf0 100644 --- a/internal/output/plaintext_fns_internal_test.go +++ b/internal/output/plaintext_fns_internal_test.go @@ -6,6 +6,32 @@ import ( "github.com/stretchr/testify/assert" ) +func TestErrorPlaintextOutputFn(t *testing.T) { + t.Run("with a non-string message does not panic", func(t *testing.T) { + r := resource{"message": float64(404)} + + result := ErrorPlaintextOutputFn(r) + + assert.Equal(t, "404", result) + }) + + t.Run("with a non-string code renders via fmt formatting", func(t *testing.T) { + r := resource{"code": 123, "message": "an error"} + + result := ErrorPlaintextOutputFn(r) + + assert.Equal(t, "an error (code: 123)", result) + }) + + t.Run("with non-string message and code does not panic", func(t *testing.T) { + r := resource{"code": true, "message": 42} + + result := ErrorPlaintextOutputFn(r) + + assert.Equal(t, "42 (code: true)", result) + }) +} + func TestSingularPlaintextOutputFn(t *testing.T) { tests := map[string]struct { resource resource diff --git a/internal/output/resource_output_test.go b/internal/output/resource_output_test.go index 1d1ec434..8b482f84 100644 --- a/internal/output/resource_output_test.go +++ b/internal/output/resource_output_test.go @@ -360,4 +360,38 @@ func TestCmdOutputError(t *testing.T) { assert.Equal(t, "something went wrong", result) }) }) + + t.Run("with an error containing statusCode and suggestion", func(t *testing.T) { + errJSON := `{"code":"not_found","message":"Not Found","statusCode":404,"suggestion":"Resource not found. Use ldcli flags list."}` + + t.Run("with JSON output includes all fields", func(t *testing.T) { + err := errors.NewError(errJSON) + + result := output.CmdOutputError("json", err) + + assert.JSONEq(t, errJSON, result) + }) + + t.Run("with plaintext output includes suggestion line", func(t *testing.T) { + err := errors.NewError(errJSON) + + result := output.CmdOutputError("plaintext", err) + + assert.Contains(t, result, "Not Found (code: not_found)") + assert.Contains(t, result, "\nSuggestion: Resource not found. Use ldcli flags list.") + }) + }) + + t.Run("with an error with statusCode but no suggestion", func(t *testing.T) { + errJSON := `{"code":"internal_server_error","message":"Internal Server Error","statusCode":500}` + + t.Run("with plaintext output does not include suggestion line", func(t *testing.T) { + err := errors.NewError(errJSON) + + result := output.CmdOutputError("plaintext", err) + + assert.Equal(t, "Internal Server Error (code: internal_server_error)", result) + assert.NotContains(t, result, "Suggestion:") + }) + }) } diff --git a/internal/resources/client.go b/internal/resources/client.go index 01a06786..ba1d164d 100644 --- a/internal/resources/client.go +++ b/internal/resources/client.go @@ -7,6 +7,7 @@ import ( "io" "net/http" "net/url" + "strings" "github.com/launchdarkly/ldcli/internal/errors" ) @@ -73,18 +74,39 @@ func (c ResourcesClient) MakeRequest( return body, nil } + var baseURI string + if parsed, err := url.Parse(path); err == nil && parsed.Scheme != "" { + baseURI = parsed.Scheme + "://" + parsed.Host + } + if len(body) > 0 { + var errMap map[string]interface{} + if err := json.Unmarshal(body, &errMap); err != nil { + errMap = map[string]interface{}{ + "code": strings.ToLower(strings.ReplaceAll(http.StatusText(res.StatusCode), " ", "_")), + "message": string(body), + "statusCode": res.StatusCode, + } + } else { + if _, exists := errMap["statusCode"]; !exists { + errMap["statusCode"] = res.StatusCode + } + } + if suggestion := errors.SuggestionForStatus(res.StatusCode, baseURI); suggestion != "" { + errMap["suggestion"] = suggestion + } + body, _ = json.Marshal(errMap) return body, errors.NewError(string(body)) } - switch res.StatusCode { - case http.StatusMethodNotAllowed: - resp, _ := json.Marshal(map[string]string{ - "code": "method_not_allowed", - "message": "method not allowed", - }) - return body, errors.NewError(string(resp)) - default: - return body, errors.NewError(fmt.Sprintf("could not complete the request: %d", res.StatusCode)) + errMap := map[string]interface{}{ + "code": strings.ToLower(strings.ReplaceAll(http.StatusText(res.StatusCode), " ", "_")), + "message": http.StatusText(res.StatusCode), + "statusCode": res.StatusCode, + } + if suggestion := errors.SuggestionForStatus(res.StatusCode, baseURI); suggestion != "" { + errMap["suggestion"] = suggestion } + resp, _ := json.Marshal(errMap) + return body, errors.NewError(string(resp)) } diff --git a/internal/resources/client_test.go b/internal/resources/client_test.go index 7b341adb..acef338e 100644 --- a/internal/resources/client_test.go +++ b/internal/resources/client_test.go @@ -1,6 +1,7 @@ package resources_test import ( + "encoding/json" "net/http" "net/http/httptest" "testing" @@ -12,7 +13,7 @@ import ( ) func TestMakeUnauthenticatedRequest(t *testing.T) { - t.Run("with a successful response", func(t *testing.T) { + t.Run("with a successful response returns body and no error", func(t *testing.T) { server := makeServer(t, http.StatusOK, `{"message": "success"}`) defer server.Close() c := resources.NewClient("test-version") @@ -23,28 +24,165 @@ func TestMakeUnauthenticatedRequest(t *testing.T) { assert.JSONEq(t, `{"message": "success"}`, string(response)) }) - t.Run("with an error with a response body", func(t *testing.T) { - server := makeServer(t, http.StatusBadRequest, `{"message": "an error"}`) + t.Run("with 204 No Content returns no error", func(t *testing.T) { + server := makeServer(t, http.StatusNoContent, "") + defer server.Close() + c := resources.NewClient("test-version") + + _, err := c.MakeUnauthenticatedRequest("DELETE", server.URL, []byte(`{}`)) + + require.NoError(t, err) + }) + + t.Run("with an error with a response body injects statusCode", func(t *testing.T) { + server := makeServer(t, http.StatusBadRequest, `{"code":"invalid_request","message": "an error"}`) defer server.Close() c := resources.NewClient("test-version") _, err := c.MakeUnauthenticatedRequest("POST", server.URL, []byte(`{}`)) - assert.EqualError(t, err, `{"message": "an error"}`) + require.Error(t, err) + assert.JSONEq(t, `{"code":"invalid_request","message":"an error","statusCode":400}`, err.Error()) + }) + + t.Run("with a body that already has statusCode does not overwrite it", func(t *testing.T) { + server := makeServer(t, http.StatusBadRequest, `{"code":"invalid_request","message":"an error","statusCode":400}`) + defer server.Close() + c := resources.NewClient("test-version") + + _, err := c.MakeUnauthenticatedRequest("POST", server.URL, []byte(`{}`)) + + require.Error(t, err) + assert.JSONEq(t, `{"code":"invalid_request","message":"an error","statusCode":400}`, err.Error()) + }) + + t.Run("with a 401 error includes suggestion in body", func(t *testing.T) { + server := makeServer(t, http.StatusUnauthorized, `{"code":"unauthorized","message":"invalid token"}`) + defer server.Close() + c := resources.NewClient("test-version") + + _, err := c.MakeUnauthenticatedRequest("POST", server.URL, []byte(`{}`)) + + require.Error(t, err) + + var errMap map[string]interface{} + require.NoError(t, json.Unmarshal([]byte(err.Error()), &errMap)) + assert.Equal(t, float64(401), errMap["statusCode"]) + assert.Contains(t, errMap["suggestion"], "ldcli login") + assert.Contains(t, errMap["suggestion"], "settings/authorization") }) t.Run("with a method not allowed status code and no body", func(t *testing.T) { server := makeServer(t, http.StatusMethodNotAllowed, "") defer server.Close() c := resources.NewClient("test-version") - expectedResponse := `{ - "code": "method_not_allowed", - "message": "method not allowed" - }` _, err := c.MakeUnauthenticatedRequest("POST", server.URL, []byte(`{}`)) - assert.JSONEq(t, expectedResponse, err.Error()) + require.Error(t, err) + assert.JSONEq(t, `{ + "code": "method_not_allowed", + "message": "Method Not Allowed", + "statusCode": 405 + }`, err.Error()) + }) + + t.Run("with a 404 and no body produces valid JSON with statusCode", func(t *testing.T) { + server := makeServer(t, http.StatusNotFound, "") + defer server.Close() + c := resources.NewClient("test-version") + + _, err := c.MakeUnauthenticatedRequest("GET", server.URL, []byte(`{}`)) + + require.Error(t, err) + + var errMap map[string]interface{} + require.NoError(t, json.Unmarshal([]byte(err.Error()), &errMap)) + assert.Equal(t, "not_found", errMap["code"]) + assert.Equal(t, "Not Found", errMap["message"]) + assert.Equal(t, float64(404), errMap["statusCode"]) + assert.Contains(t, errMap["suggestion"], "ldcli projects list") + }) + + t.Run("with a 500 and no body produces valid JSON without suggestion", func(t *testing.T) { + server := makeServer(t, http.StatusInternalServerError, "") + defer server.Close() + c := resources.NewClient("test-version") + + _, err := c.MakeUnauthenticatedRequest("GET", server.URL, []byte(`{}`)) + + require.Error(t, err) + + var errMap map[string]interface{} + require.NoError(t, json.Unmarshal([]byte(err.Error()), &errMap)) + assert.Equal(t, "internal_server_error", errMap["code"]) + assert.Equal(t, "Internal Server Error", errMap["message"]) + assert.Equal(t, float64(500), errMap["statusCode"]) + assert.Nil(t, errMap["suggestion"]) + }) + + t.Run("with a 502 and no body produces valid JSON without suggestion", func(t *testing.T) { + server := makeServer(t, http.StatusBadGateway, "") + defer server.Close() + c := resources.NewClient("test-version") + + _, err := c.MakeUnauthenticatedRequest("GET", server.URL, []byte(`{}`)) + + require.Error(t, err) + + var errMap map[string]interface{} + require.NoError(t, json.Unmarshal([]byte(err.Error()), &errMap)) + assert.Equal(t, "bad_gateway", errMap["code"]) + assert.Equal(t, float64(502), errMap["statusCode"]) + assert.Nil(t, errMap["suggestion"]) + }) + + t.Run("with a 429 and body includes rate limit suggestion", func(t *testing.T) { + server := makeServer(t, http.StatusTooManyRequests, `{"code":"rate_limited","message":"too many requests"}`) + defer server.Close() + c := resources.NewClient("test-version") + + _, err := c.MakeUnauthenticatedRequest("GET", server.URL, []byte(`{}`)) + + require.Error(t, err) + + var errMap map[string]interface{} + require.NoError(t, json.Unmarshal([]byte(err.Error()), &errMap)) + assert.Equal(t, float64(429), errMap["statusCode"]) + assert.Contains(t, errMap["suggestion"], "Rate limited") + }) + + t.Run("with a non-JSON body produces synthetic JSON error", func(t *testing.T) { + server := makeServer(t, http.StatusBadGateway, "Bad Gateway") + defer server.Close() + c := resources.NewClient("test-version") + + _, err := c.MakeUnauthenticatedRequest("GET", server.URL, []byte(`{}`)) + + require.Error(t, err) + + var errMap map[string]interface{} + require.NoError(t, json.Unmarshal([]byte(err.Error()), &errMap)) + assert.Equal(t, "bad_gateway", errMap["code"]) + assert.Equal(t, "Bad Gateway", errMap["message"]) + assert.Equal(t, float64(502), errMap["statusCode"]) + }) + + t.Run("with a plain-text body produces synthetic JSON error with suggestion", func(t *testing.T) { + server := makeServer(t, http.StatusNotFound, "page not found") + defer server.Close() + c := resources.NewClient("test-version") + + _, err := c.MakeUnauthenticatedRequest("GET", server.URL, []byte(`{}`)) + + require.Error(t, err) + + var errMap map[string]interface{} + require.NoError(t, json.Unmarshal([]byte(err.Error()), &errMap)) + assert.Equal(t, "not_found", errMap["code"]) + assert.Equal(t, "page not found", errMap["message"]) + assert.Equal(t, float64(404), errMap["statusCode"]) + assert.Contains(t, errMap["suggestion"], "ldcli projects list") }) } From 6e027006885afbcb8ec16bd2ce04a2cf427cc596 Mon Sep 17 00:00:00 2001 From: Ramon Niebla Date: Mon, 13 Apr 2026 16:53:46 -0700 Subject: [PATCH 2/2] feat(REL-12755): adding --fields flag (#662) * [feat] adding --fields flag * feat(REL-15756): updating agent friendly and improved rich text output (#663) feat(REL-15756) updating agent friendly and improved rich text output --- cmd/cliflags/flags.go | 11 + cmd/config/config.go | 4 +- cmd/config/testdata/help.golden | 1 + cmd/flags/archive.go | 4 +- cmd/flags/archive_test.go | 124 +++- cmd/flags/toggle.go | 4 +- cmd/flags/toggle_test.go | 352 +++++++++- cmd/members/invite.go | 4 +- cmd/members/invite_test.go | 8 +- cmd/resources/resources.go | 4 +- cmd/root.go | 6 + cmd/root_test.go | 7 +- internal/output/plaintext_fns.go | 14 +- .../output/plaintext_fns_internal_test.go | 159 +++++ internal/output/resource_output.go | 98 ++- internal/output/resource_output_test.go | 599 +++++++++++++++++- internal/output/table.go | 198 ++++++ internal/output/table_test.go | 410 ++++++++++++ 18 files changed, 1946 insertions(+), 61 deletions(-) create mode 100644 internal/output/table.go create mode 100644 internal/output/table_test.go diff --git a/cmd/cliflags/flags.go b/cmd/cliflags/flags.go index 592894d0..4e89c415 100644 --- a/cmd/cliflags/flags.go +++ b/cmd/cliflags/flags.go @@ -13,6 +13,15 @@ func GetOutputKind(cmd *cobra.Command) string { return viper.GetString(OutputFlag) } +// GetFields returns the list of fields to include in JSON output, or nil if not specified. +func GetFields(cmd *cobra.Command) []string { + fields, err := cmd.Root().PersistentFlags().GetStringSlice(FieldsFlag) + if err != nil { + return nil + } + return fields +} + const ( BaseURIDefault = "https://app.launchdarkly.com" DevStreamURIDefault = "https://stream.launchdarkly.com" @@ -27,6 +36,7 @@ const ( DevStreamURIFlag = "dev-stream-uri" EmailsFlag = "emails" EnvironmentFlag = "environment" + FieldsFlag = "fields" FlagFlag = "flag" JSONFlag = "json" OutputFlag = "output" @@ -42,6 +52,7 @@ const ( CorsOriginFlagDescription = "Allowed CORS origin. Use '*' for all origins (default: '*')" DevStreamURIDescription = "Streaming service endpoint that the dev server uses to obtain authoritative flag data. This may be a LaunchDarkly or Relay Proxy endpoint" EnvironmentFlagDescription = "Default environment key" + FieldsFlagDescription = "Comma-separated list of top-level fields to include in JSON output (e.g., --fields key,name,kind)" FlagFlagDescription = "Default feature flag key" JSONFlagDescription = "Output JSON format (shorthand for --output json)" OutputFlagDescription = "Output format: json or plaintext (default: plaintext in a terminal, json otherwise)" diff --git a/cmd/config/config.go b/cmd/config/config.go index 186faf74..cd923e7b 100644 --- a/cmd/config/config.go +++ b/cmd/config/config.go @@ -287,7 +287,7 @@ func outputSetAction(newFields []string) (string, error) { Items: newFields, } fieldsJSON, _ := json.Marshal(fields) - output, err := output.CmdOutput("update", viper.GetString(cliflags.OutputFlag), fieldsJSON) + output, err := output.CmdOutput("update", viper.GetString(cliflags.OutputFlag), fieldsJSON, nil) if err != nil { return "", errs.NewError(err.Error()) } @@ -302,7 +302,7 @@ func outputUnsetAction(newField string) (string, error) { Key: newField, } fieldJSON, _ := json.Marshal(field) - output, err := output.CmdOutput("delete", viper.GetString(cliflags.OutputFlag), fieldJSON) + output, err := output.CmdOutput("delete", viper.GetString(cliflags.OutputFlag), fieldJSON, nil) if err != nil { return "", errs.NewError(err.Error()) } diff --git a/cmd/config/testdata/help.golden b/cmd/config/testdata/help.golden index a0aaeab2..9cfacf1a 100644 --- a/cmd/config/testdata/help.golden +++ b/cmd/config/testdata/help.golden @@ -27,5 +27,6 @@ Global Flags: --access-token string LaunchDarkly access token with write-level access --analytics-opt-out Opt out of analytics tracking --base-uri string LaunchDarkly base URI (default "https://app.launchdarkly.com") + --fields strings Comma-separated list of top-level fields to include in JSON output (e.g., --fields key,name,kind) --json Output JSON format (shorthand for --output json) -o, --output string Output format: json or plaintext (default: plaintext in a terminal, json otherwise) (default "plaintext") diff --git a/cmd/flags/archive.go b/cmd/flags/archive.go index adea772f..005ffee4 100644 --- a/cmd/flags/archive.go +++ b/cmd/flags/archive.go @@ -51,7 +51,9 @@ func makeArchiveRequest(client resources.Client) func(*cobra.Command, []string) return output.NewCmdOutputError(err, cliflags.GetOutputKind(cmd)) } - output, err := output.CmdOutput("update", cliflags.GetOutputKind(cmd), res) + output, err := output.CmdOutput("update", cliflags.GetOutputKind(cmd), res, cliflags.GetFields(cmd), output.CmdOutputOpts{ + ResourceName: "flags", + }) if err != nil { return errors.NewError(err.Error()) } diff --git a/cmd/flags/archive_test.go b/cmd/flags/archive_test.go index c8d1ac7c..eace8b5e 100644 --- a/cmd/flags/archive_test.go +++ b/cmd/flags/archive_test.go @@ -15,11 +15,13 @@ func TestArchive(t *testing.T) { mockClient := &resources.MockClient{ Response: []byte(`{ "key": "test-flag", - "name": "test flag" + "name": "test flag", + "kind": "boolean", + "archived": true }`), } - t.Run("succeeds with valid inputs", func(t *testing.T) { + t.Run("succeeds with plaintext output", func(t *testing.T) { args := []string{ "flags", "archive", "--access-token", "abcd1234", @@ -37,8 +39,122 @@ func TestArchive(t *testing.T) { require.NoError(t, err) assert.Equal(t, `[{"op": "replace", "path": "/archived", "value": true}]`, string(mockClient.Input)) - assert.Equal(t, "Successfully updated test flag (test-flag)\n", string(output)) + assert.Contains(t, string(output), "Successfully updated\n\nKey:") + assert.Contains(t, string(output), "test-flag") + assert.Contains(t, string(output), "Name:") + assert.Contains(t, string(output), "test flag") + assert.NotContains(t, string(output), "* ") }) + + t.Run("succeeds with JSON output", func(t *testing.T) { + args := []string{ + "flags", "archive", + "--access-token", "abcd1234", + "--flag", "test-flag", + "--project", "test-proj", + "--output", "json", + } + output, err := cmd.CallCmd( + t, + cmd.APIClients{ + ResourcesClient: mockClient, + }, + analytics.NoopClientFn{}.Tracker(), + args, + ) + + require.NoError(t, err) + assert.JSONEq(t, `{"key":"test-flag","name":"test flag","kind":"boolean","archived":true}`, string(output)) + }) + + t.Run("succeeds with --json shorthand", func(t *testing.T) { + args := []string{ + "flags", "archive", + "--access-token", "abcd1234", + "--flag", "test-flag", + "--project", "test-proj", + "--json", + } + output, err := cmd.CallCmd( + t, + cmd.APIClients{ + ResourcesClient: mockClient, + }, + analytics.NoopClientFn{}.Tracker(), + args, + ) + + require.NoError(t, err) + assert.JSONEq(t, `{"key":"test-flag","name":"test flag","kind":"boolean","archived":true}`, string(output)) + }) + + t.Run("filters JSON output with --fields", func(t *testing.T) { + args := []string{ + "flags", "archive", + "--access-token", "abcd1234", + "--flag", "test-flag", + "--project", "test-proj", + "--output", "json", + "--fields", "key,name", + } + output, err := cmd.CallCmd( + t, + cmd.APIClients{ + ResourcesClient: mockClient, + }, + analytics.NoopClientFn{}.Tracker(), + args, + ) + + require.NoError(t, err) + assert.JSONEq(t, `{"key":"test-flag","name":"test flag"}`, string(output)) + }) + + t.Run("filters JSON output with --json and --fields", func(t *testing.T) { + args := []string{ + "flags", "archive", + "--access-token", "abcd1234", + "--flag", "test-flag", + "--project", "test-proj", + "--json", + "--fields", "key,name", + } + output, err := cmd.CallCmd( + t, + cmd.APIClients{ + ResourcesClient: mockClient, + }, + analytics.NoopClientFn{}.Tracker(), + args, + ) + + require.NoError(t, err) + assert.JSONEq(t, `{"key":"test-flag","name":"test flag"}`, string(output)) + }) + + t.Run("ignores --fields with plaintext output", func(t *testing.T) { + args := []string{ + "flags", "archive", + "--access-token", "abcd1234", + "--flag", "test-flag", + "--project", "test-proj", + "--fields", "key", + } + output, err := cmd.CallCmd( + t, + cmd.APIClients{ + ResourcesClient: mockClient, + }, + analytics.NoopClientFn{}.Tracker(), + args, + ) + + require.NoError(t, err) + assert.Contains(t, string(output), "Successfully updated") + assert.Contains(t, string(output), "Key:") + assert.Contains(t, string(output), "test-flag") + }) + t.Run("returns error with missing flags", func(t *testing.T) { args := []string{ "flags", "archive", @@ -55,6 +171,6 @@ func TestArchive(t *testing.T) { ) assert.Error(t, err) - assert.Equal(t, "required flag(s) \"project\" not set. See `ldcli flags archive --help` for supported flags and usage.", err.Error()) + assert.Contains(t, err.Error(), `required flag(s) "project" not set`) }) } diff --git a/cmd/flags/toggle.go b/cmd/flags/toggle.go index c34fce2f..3a292eb3 100644 --- a/cmd/flags/toggle.go +++ b/cmd/flags/toggle.go @@ -73,7 +73,9 @@ func runE(client resources.Client) func(*cobra.Command, []string) error { return output.NewCmdOutputError(err, cliflags.GetOutputKind(cmd)) } - output, err := output.CmdOutput("update", cliflags.GetOutputKind(cmd), res) + output, err := output.CmdOutput("update", cliflags.GetOutputKind(cmd), res, cliflags.GetFields(cmd), output.CmdOutputOpts{ + ResourceName: "flags", + }) if err != nil { return errors.NewError(err.Error()) } diff --git a/cmd/flags/toggle_test.go b/cmd/flags/toggle_test.go index 58f142e9..429f08ef 100644 --- a/cmd/flags/toggle_test.go +++ b/cmd/flags/toggle_test.go @@ -15,26 +15,340 @@ func TestToggleOn(t *testing.T) { mockClient := &resources.MockClient{ Response: []byte(`{ "key": "test-flag", - "name": "test flag" + "name": "test flag", + "kind": "boolean", + "temporary": true }`), } - args := []string{ - "flags", "toggle-on", - "--access-token", "abcd1234", - "--environment", "test-env", - "--flag", "test-flag", - "--project", "test-proj", + + t.Run("succeeds with plaintext output", func(t *testing.T) { + args := []string{ + "flags", "toggle-on", + "--access-token", "abcd1234", + "--environment", "test-env", + "--flag", "test-flag", + "--project", "test-proj", + } + output, err := cmd.CallCmd( + t, + cmd.APIClients{ + ResourcesClient: mockClient, + }, + analytics.NoopClientFn{}.Tracker(), + args, + ) + + require.NoError(t, err) + assert.Equal(t, `[{"op": "replace", "path": "/environments/test-env/on", "value": true}]`, string(mockClient.Input)) + assert.Contains(t, string(output), "Successfully updated\n\nKey:") + assert.Contains(t, string(output), "test-flag") + assert.Contains(t, string(output), "Name:") + assert.Contains(t, string(output), "test flag") + assert.NotContains(t, string(output), "* ") + }) + + t.Run("succeeds with JSON output", func(t *testing.T) { + args := []string{ + "flags", "toggle-on", + "--access-token", "abcd1234", + "--environment", "test-env", + "--flag", "test-flag", + "--project", "test-proj", + "--output", "json", + } + output, err := cmd.CallCmd( + t, + cmd.APIClients{ + ResourcesClient: mockClient, + }, + analytics.NoopClientFn{}.Tracker(), + args, + ) + + require.NoError(t, err) + assert.JSONEq(t, `{"key":"test-flag","name":"test flag","kind":"boolean","temporary":true}`, string(output)) + }) + + t.Run("succeeds with --json shorthand", func(t *testing.T) { + args := []string{ + "flags", "toggle-on", + "--access-token", "abcd1234", + "--environment", "test-env", + "--flag", "test-flag", + "--project", "test-proj", + "--json", + } + output, err := cmd.CallCmd( + t, + cmd.APIClients{ + ResourcesClient: mockClient, + }, + analytics.NoopClientFn{}.Tracker(), + args, + ) + + require.NoError(t, err) + assert.JSONEq(t, `{"key":"test-flag","name":"test flag","kind":"boolean","temporary":true}`, string(output)) + }) + + t.Run("filters JSON output with --fields", func(t *testing.T) { + args := []string{ + "flags", "toggle-on", + "--access-token", "abcd1234", + "--environment", "test-env", + "--flag", "test-flag", + "--project", "test-proj", + "--output", "json", + "--fields", "key,name", + } + output, err := cmd.CallCmd( + t, + cmd.APIClients{ + ResourcesClient: mockClient, + }, + analytics.NoopClientFn{}.Tracker(), + args, + ) + + require.NoError(t, err) + assert.JSONEq(t, `{"key":"test-flag","name":"test flag"}`, string(output)) + }) + + t.Run("filters JSON output with --json and --fields", func(t *testing.T) { + args := []string{ + "flags", "toggle-on", + "--access-token", "abcd1234", + "--environment", "test-env", + "--flag", "test-flag", + "--project", "test-proj", + "--json", + "--fields", "key,name", + } + output, err := cmd.CallCmd( + t, + cmd.APIClients{ + ResourcesClient: mockClient, + }, + analytics.NoopClientFn{}.Tracker(), + args, + ) + + require.NoError(t, err) + assert.JSONEq(t, `{"key":"test-flag","name":"test flag"}`, string(output)) + }) + + t.Run("ignores --fields with plaintext output", func(t *testing.T) { + args := []string{ + "flags", "toggle-on", + "--access-token", "abcd1234", + "--environment", "test-env", + "--flag", "test-flag", + "--project", "test-proj", + "--fields", "key", + } + output, err := cmd.CallCmd( + t, + cmd.APIClients{ + ResourcesClient: mockClient, + }, + analytics.NoopClientFn{}.Tracker(), + args, + ) + + require.NoError(t, err) + assert.Contains(t, string(output), "Successfully updated") + assert.Contains(t, string(output), "Key:") + assert.Contains(t, string(output), "test-flag") + }) + + t.Run("returns error with missing required flags", func(t *testing.T) { + args := []string{ + "flags", "toggle-on", + "--access-token", "abcd1234", + "--environment", "test-env", + "--flag", "test-flag", + } + _, err := cmd.CallCmd( + t, + cmd.APIClients{ + ResourcesClient: mockClient, + }, + analytics.NoopClientFn{}.Tracker(), + args, + ) + + assert.Error(t, err) + assert.Contains(t, err.Error(), `required flag(s) "project" not set`) + }) +} + +func TestToggleOff(t *testing.T) { + mockClient := &resources.MockClient{ + Response: []byte(`{ + "key": "test-flag", + "name": "test flag", + "kind": "boolean", + "temporary": true + }`), } - output, err := cmd.CallCmd( - t, - cmd.APIClients{ - ResourcesClient: mockClient, - }, - analytics.NoopClientFn{}.Tracker(), - args, - ) - - require.NoError(t, err) - assert.Equal(t, `[{"op": "replace", "path": "/environments/test-env/on", "value": true}]`, string(mockClient.Input)) - assert.Equal(t, "Successfully updated test flag (test-flag)\n", string(output)) + + t.Run("succeeds with plaintext output", func(t *testing.T) { + args := []string{ + "flags", "toggle-off", + "--access-token", "abcd1234", + "--environment", "test-env", + "--flag", "test-flag", + "--project", "test-proj", + } + output, err := cmd.CallCmd( + t, + cmd.APIClients{ + ResourcesClient: mockClient, + }, + analytics.NoopClientFn{}.Tracker(), + args, + ) + + require.NoError(t, err) + assert.Equal(t, `[{"op": "replace", "path": "/environments/test-env/on", "value": false}]`, string(mockClient.Input)) + assert.Contains(t, string(output), "Successfully updated\n\nKey:") + assert.Contains(t, string(output), "test-flag") + assert.Contains(t, string(output), "Name:") + assert.Contains(t, string(output), "test flag") + assert.NotContains(t, string(output), "* ") + }) + + t.Run("succeeds with JSON output", func(t *testing.T) { + args := []string{ + "flags", "toggle-off", + "--access-token", "abcd1234", + "--environment", "test-env", + "--flag", "test-flag", + "--project", "test-proj", + "--output", "json", + } + output, err := cmd.CallCmd( + t, + cmd.APIClients{ + ResourcesClient: mockClient, + }, + analytics.NoopClientFn{}.Tracker(), + args, + ) + + require.NoError(t, err) + assert.JSONEq(t, `{"key":"test-flag","name":"test flag","kind":"boolean","temporary":true}`, string(output)) + }) + + t.Run("succeeds with --json shorthand", func(t *testing.T) { + args := []string{ + "flags", "toggle-off", + "--access-token", "abcd1234", + "--environment", "test-env", + "--flag", "test-flag", + "--project", "test-proj", + "--json", + } + output, err := cmd.CallCmd( + t, + cmd.APIClients{ + ResourcesClient: mockClient, + }, + analytics.NoopClientFn{}.Tracker(), + args, + ) + + require.NoError(t, err) + assert.JSONEq(t, `{"key":"test-flag","name":"test flag","kind":"boolean","temporary":true}`, string(output)) + }) + + t.Run("filters JSON output with --fields", func(t *testing.T) { + args := []string{ + "flags", "toggle-off", + "--access-token", "abcd1234", + "--environment", "test-env", + "--flag", "test-flag", + "--project", "test-proj", + "--output", "json", + "--fields", "key,name", + } + output, err := cmd.CallCmd( + t, + cmd.APIClients{ + ResourcesClient: mockClient, + }, + analytics.NoopClientFn{}.Tracker(), + args, + ) + + require.NoError(t, err) + assert.JSONEq(t, `{"key":"test-flag","name":"test flag"}`, string(output)) + }) + + t.Run("filters JSON output with --json and --fields", func(t *testing.T) { + args := []string{ + "flags", "toggle-off", + "--access-token", "abcd1234", + "--environment", "test-env", + "--flag", "test-flag", + "--project", "test-proj", + "--json", + "--fields", "key,name", + } + output, err := cmd.CallCmd( + t, + cmd.APIClients{ + ResourcesClient: mockClient, + }, + analytics.NoopClientFn{}.Tracker(), + args, + ) + + require.NoError(t, err) + assert.JSONEq(t, `{"key":"test-flag","name":"test flag"}`, string(output)) + }) + + t.Run("ignores --fields with plaintext output", func(t *testing.T) { + args := []string{ + "flags", "toggle-off", + "--access-token", "abcd1234", + "--environment", "test-env", + "--flag", "test-flag", + "--project", "test-proj", + "--fields", "key", + } + output, err := cmd.CallCmd( + t, + cmd.APIClients{ + ResourcesClient: mockClient, + }, + analytics.NoopClientFn{}.Tracker(), + args, + ) + + require.NoError(t, err) + assert.Contains(t, string(output), "Successfully updated") + assert.Contains(t, string(output), "Key:") + assert.Contains(t, string(output), "test-flag") + }) + + t.Run("returns error with missing required flags", func(t *testing.T) { + args := []string{ + "flags", "toggle-off", + "--access-token", "abcd1234", + "--environment", "test-env", + "--flag", "test-flag", + } + _, err := cmd.CallCmd( + t, + cmd.APIClients{ + ResourcesClient: mockClient, + }, + analytics.NoopClientFn{}.Tracker(), + args, + ) + + assert.Error(t, err) + assert.Contains(t, err.Error(), `required flag(s) "project" not set`) + }) } diff --git a/cmd/members/invite.go b/cmd/members/invite.go index cf7095be..6ad313d2 100644 --- a/cmd/members/invite.go +++ b/cmd/members/invite.go @@ -63,7 +63,9 @@ func runE(client resources.Client) func(*cobra.Command, []string) error { return output.NewCmdOutputError(err, cliflags.GetOutputKind(cmd)) } - output, err := output.CmdOutput("update", cliflags.GetOutputKind(cmd), res) + output, err := output.CmdOutput("update", cliflags.GetOutputKind(cmd), res, cliflags.GetFields(cmd), output.CmdOutputOpts{ + ResourceName: "members", + }) if err != nil { return errors.NewError(err.Error()) } diff --git a/cmd/members/invite_test.go b/cmd/members/invite_test.go index bb218b51..3a9db702 100644 --- a/cmd/members/invite_test.go +++ b/cmd/members/invite_test.go @@ -45,5 +45,11 @@ func TestInvite(t *testing.T) { require.NoError(t, err) assert.Equal(t, `[{"email":"test1@test.com","role":"writer"},{"email":"test2@test.com","role":"writer"}]`, string(mockClient.Input)) - assert.Equal(t, "Successfully updated\n* test1@test.com (000000000000000000000001)\n* test2@test.com (000000000000000000000002)\n", string(output)) + assert.Contains(t, string(output), "Successfully updated") + assert.Contains(t, string(output), "EMAIL") + assert.Contains(t, string(output), "ROLE") + assert.Contains(t, string(output), "test1@test.com") + assert.Contains(t, string(output), "test2@test.com") + assert.Contains(t, string(output), "writer") + assert.NotContains(t, string(output), "* ") } diff --git a/cmd/resources/resources.go b/cmd/resources/resources.go index 0c0bcabd..610461e2 100644 --- a/cmd/resources/resources.go +++ b/cmd/resources/resources.go @@ -349,7 +349,9 @@ func (op *OperationCmd) makeRequest(cmd *cobra.Command, args []string) error { res = []byte(fmt.Sprintf(`{"key": %q}`, urlParms[len(urlParms)-1])) } - output, err := output.CmdOutput(cmd.Use, cliflags.GetOutputKind(cmd), res) + output, err := output.CmdOutput(cmd.Use, cliflags.GetOutputKind(cmd), res, cliflags.GetFields(cmd), output.CmdOutputOpts{ + ResourceName: cmd.Parent().Name(), + }) if err != nil { return errors.NewError(err.Error()) } diff --git a/cmd/root.go b/cmd/root.go index 792f36ca..4d8e68cf 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -214,6 +214,12 @@ func NewRootCommand( cliflags.JSONFlagDescription, ) + cmd.PersistentFlags().StringSlice( + cliflags.FieldsFlag, + nil, + cliflags.FieldsFlagDescription, + ) + configCmd := configcmd.NewConfigCmd(configService, analyticsTrackerFn) cmd.AddCommand(configCmd.Cmd()) cmd.AddCommand(NewQuickStartCmd(analyticsTrackerFn, clients.EnvironmentsClient, clients.FlagsClient)) diff --git a/cmd/root_test.go b/cmd/root_test.go index 88646f13..69cb2c5e 100644 --- a/cmd/root_test.go +++ b/cmd/root_test.go @@ -101,8 +101,8 @@ func TestOutputFlags(t *testing.T) { ) require.NoError(t, err) - assert.Contains(t, string(output), "Successfully updated") - assert.Contains(t, string(output), "test-name (test-key)") + assert.Contains(t, string(output), "Successfully updated\n\nKey:") + assert.Contains(t, string(output), "test-key") }) } @@ -205,7 +205,8 @@ func TestTTYDefaultOutput(t *testing.T) { out := execNonTTYCmd(t, mockClient) assert.Contains(t, string(out), "Successfully updated") - assert.Contains(t, string(out), "test-name (test-key)") + assert.Contains(t, string(out), "Key:") + assert.Contains(t, string(out), "test-key") }) t.Run("FORCE_TTY overrides non-TTY detection", func(t *testing.T) { diff --git a/internal/output/plaintext_fns.go b/internal/output/plaintext_fns.go index 43d8ac71..9b15e19f 100644 --- a/internal/output/plaintext_fns.go +++ b/internal/output/plaintext_fns.go @@ -62,19 +62,19 @@ var SingularPlaintextOutputFn = func(r resource) string { switch { case name != nil && key != nil: - return fmt.Sprintf("%s (%s)", name.(string), key.(string)) + return fmt.Sprintf("%s (%s)", fmt.Sprint(name), fmt.Sprint(key)) case email != nil && id != nil: - return fmt.Sprintf("%s (%s)", email.(string), id.(string)) + return fmt.Sprintf("%s (%s)", fmt.Sprint(email), fmt.Sprint(id)) case name != nil && id != nil: - return fmt.Sprintf("%s (%s)", name.(string), id.(string)) + return fmt.Sprintf("%s (%s)", fmt.Sprint(name), fmt.Sprint(id)) case key != nil: - return key.(string) + return fmt.Sprint(key) case email != nil: - return email.(string) + return fmt.Sprint(email) case id != nil: - return id.(string) + return fmt.Sprint(id) case name != nil: - return name.(string) + return fmt.Sprint(name) default: return "cannot read resource" } diff --git a/internal/output/plaintext_fns_internal_test.go b/internal/output/plaintext_fns_internal_test.go index e4c39bf0..4f7aee97 100644 --- a/internal/output/plaintext_fns_internal_test.go +++ b/internal/output/plaintext_fns_internal_test.go @@ -7,6 +7,54 @@ import ( ) func TestErrorPlaintextOutputFn(t *testing.T) { + t.Run("with code and message", func(t *testing.T) { + r := resource{"code": "conflict", "message": "resource already exists"} + + result := ErrorPlaintextOutputFn(r) + + assert.Equal(t, "resource already exists (code: conflict)", result) + }) + + t.Run("with only a message", func(t *testing.T) { + r := resource{"message": "something went wrong"} + + result := ErrorPlaintextOutputFn(r) + + assert.Equal(t, "something went wrong", result) + }) + + t.Run("with only a code", func(t *testing.T) { + r := resource{"code": "not_found", "message": ""} + + result := ErrorPlaintextOutputFn(r) + + assert.Equal(t, "an error occurred (code: not_found)", result) + }) + + t.Run("with neither code nor message", func(t *testing.T) { + r := resource{} + + result := ErrorPlaintextOutputFn(r) + + assert.Equal(t, "unknown error occurred", result) + }) + + t.Run("with empty message and nil code", func(t *testing.T) { + r := resource{"message": ""} + + result := ErrorPlaintextOutputFn(r) + + assert.Equal(t, "unknown error occurred", result) + }) + + t.Run("with suggestion appends suggestion line", func(t *testing.T) { + r := resource{"code": "not_found", "message": "Not Found", "suggestion": "Try ldcli flags list."} + + result := ErrorPlaintextOutputFn(r) + + assert.Equal(t, "Not Found (code: not_found)\nSuggestion: Try ldcli flags list.", result) + }) + t.Run("with a non-string message does not panic", func(t *testing.T) { r := resource{"message": float64(404)} @@ -32,6 +80,91 @@ func TestErrorPlaintextOutputFn(t *testing.T) { }) } +func TestMultiplePlaintextOutputFn(t *testing.T) { + tests := map[string]struct { + resource resource + expected string + }{ + "with a name and key": { + resource: resource{ + "key": "test-key", + "name": "test-name", + }, + expected: "* test-name (test-key)", + }, + "with only a key": { + resource: resource{ + "key": "test-key", + }, + expected: "* test-key", + }, + "with an email and ID": { + resource: resource{ + "_id": "test-id", + "email": "test-email", + }, + expected: "* test-email (test-id)", + }, + "without any valid field": { + resource: resource{ + "other": "other-value", + }, + expected: "* cannot read resource", + }, + } + + for name, tt := range tests { + tt := tt + t.Run(name, func(t *testing.T) { + out := MultiplePlaintextOutputFn(tt.resource) + + assert.Equal(t, tt.expected, out) + }) + } +} + +func TestConfigPlaintextOutputFn(t *testing.T) { + tests := map[string]struct { + resource resource + expected string + }{ + "with multiple keys sorts alphabetically": { + resource: resource{ + "zeta": "last", + "alpha": "first", + "mid": "middle", + }, + expected: "alpha: first\nmid: middle\nzeta: last", + }, + "with single key": { + resource: resource{ + "key": "value", + }, + expected: "key: value", + }, + "with empty resource": { + resource: resource{}, + expected: "", + }, + "with non-string values": { + resource: resource{ + "count": float64(42), + "enabled": true, + }, + expected: "count: 42\nenabled: true", + }, + } + + for name, tt := range tests { + tt := tt + t.Run(name, func(t *testing.T) { + out := ConfigPlaintextOutputFn(tt.resource) + + assert.Equal(t, tt.expected, out) + }) + } +} + func TestSingularPlaintextOutputFn(t *testing.T) { tests := map[string]struct { resource resource @@ -77,6 +210,32 @@ func TestSingularPlaintextOutputFn(t *testing.T) { }, expected: "cannot read resource", }, + "with non-string name and key does not panic": { + resource: resource{ + "key": float64(123), + "name": true, + }, + expected: "true (123)", + }, + "with non-string email and id does not panic": { + resource: resource{ + "_id": float64(999), + "email": float64(42), + }, + expected: "42 (999)", + }, + "with non-string key only does not panic": { + resource: resource{ + "key": float64(456), + }, + expected: "456", + }, + "with non-string name only does not panic": { + resource: resource{ + "name": float64(789), + }, + expected: "789", + }, } for name, tt := range tests { diff --git a/internal/output/resource_output.go b/internal/output/resource_output.go index 6de5a7d1..1b5a9476 100644 --- a/internal/output/resource_output.go +++ b/internal/output/resource_output.go @@ -12,10 +12,33 @@ import ( errs "github.com/launchdarkly/ldcli/internal/errors" ) +// CmdOutputOpts configures optional behavior for CmdOutput. +type CmdOutputOpts struct { + Fields []string + ResourceName string +} + // CmdOutput returns a response from a resource action formatted based on the output flag along with -// an optional message based on the action. -func CmdOutput(action string, outputKind string, input []byte) (string, error) { +// an optional message based on the action. When fields is non-empty and outputKind is "json", +// only the specified top-level fields are included in the output. When resourceName matches a +// registered resource, list output uses table formatting and singular output uses key-value pairs. +func CmdOutput(action string, outputKind string, input []byte, fields []string, opts ...CmdOutputOpts) (string, error) { + var resourceName string + if len(opts) > 0 { + resourceName = opts[0].ResourceName + if len(fields) == 0 { + fields = opts[0].Fields + } + } + if outputKind == "json" { + if len(fields) > 0 { + filtered, err := filterFields(input, fields) + if err != nil { + return string(input), nil + } + return string(filtered), nil + } return string(input), nil } @@ -59,6 +82,13 @@ func CmdOutput(action string, outputKind string, input []byte) (string, error) { } if !isMultipleResponse { + if cols := GetSingularColumns(resourceName); cols != nil { + kv := KeyValueOutput(maybeResource, cols) + if strings.TrimSpace(successMessage) != "" { + return successMessage + "\n\n" + kv, nil + } + return kv, nil + } return plaintextOutput(SingularPlaintextOutputFn(maybeResource), successMessage+" "), nil } @@ -66,9 +96,15 @@ func CmdOutput(action string, outputKind string, input []byte) (string, error) { return "No items found", nil } - items := make([]string, 0, len(maybeResources.Items)) - for _, i := range maybeResources.Items { - items = append(items, MultiplePlaintextOutputFn(i)) + var body string + if cols := GetListColumns(resourceName); cols != nil { + body = TableOutput(maybeResources.Items, cols) + } else { + items := make([]string, 0, len(maybeResources.Items)) + for _, i := range maybeResources.Items { + items = append(items, MultiplePlaintextOutputFn(i)) + } + body = strings.Join(items, "\n") } var ( @@ -99,7 +135,7 @@ func CmdOutput(action string, outputKind string, input []byte) (string, error) { if successMessage != "" { successMessage += "\n" } - return plaintextOutput(strings.Join(items, "\n"), successMessage) + pagination, nil + return plaintextOutput(body, successMessage) + pagination, nil } func plaintextOutput(out string, successMessage string) string { @@ -141,6 +177,56 @@ func NewCmdOutputError(err error, outputKind string) error { return errs.NewError(CmdOutputError(outputKind, err)) } +func filterFields(input []byte, fields []string) ([]byte, error) { + fieldSet := make(map[string]bool, len(fields)) + for _, f := range fields { + if trimmed := strings.TrimSpace(f); trimmed != "" { + fieldSet[trimmed] = true + } + } + if len(fieldSet) == 0 { + return input, nil + } + + var raw map[string]interface{} + if err := json.Unmarshal(input, &raw); err != nil { + return nil, err + } + + if items, ok := raw["items"]; ok { + if itemList, ok := items.([]interface{}); ok { + filtered := make([]interface{}, 0, len(itemList)) + for _, item := range itemList { + if m, ok := item.(map[string]interface{}); ok { + filtered = append(filtered, filterMap(m, fieldSet)) + } else { + filtered = append(filtered, item) + } + } + result := map[string]interface{}{"items": filtered} + if tc, ok := raw["totalCount"]; ok { + result["totalCount"] = tc + } + if links, ok := raw["_links"]; ok { + result["_links"] = links + } + return json.MarshalIndent(result, "", " ") + } + } + + return json.MarshalIndent(filterMap(raw, fieldSet), "", " ") +} + +func filterMap(m map[string]interface{}, fields map[string]bool) map[string]interface{} { + result := make(map[string]interface{}, len(fields)) + for k, v := range m { + if fields[k] { + result[k] = v + } + } + return result +} + func errJSON(s string) string { return fmt.Sprintf( `{ diff --git a/internal/output/resource_output_test.go b/internal/output/resource_output_test.go index 8b482f84..18fb9db2 100644 --- a/internal/output/resource_output_test.go +++ b/internal/output/resource_output_test.go @@ -26,7 +26,7 @@ func TestCmdOutput(t *testing.T) { t.Run("returns a success message", func(t *testing.T) { expected := "* test-id" - result, err := output.CmdOutput("list", "plaintext", []byte(input)) + result, err := output.CmdOutput("list", "plaintext", []byte(input), nil) require.NoError(t, err) assert.Equal(t, expected, result) @@ -78,7 +78,7 @@ func TestCmdOutput(t *testing.T) { tt.offset, ) - result, err := output.CmdOutput("list", "plaintext", []byte(input)) + result, err := output.CmdOutput("list", "plaintext", []byte(input), nil) require.NoError(t, err) assert.Equal(t, tt.expected, result) @@ -100,7 +100,7 @@ func TestCmdOutput(t *testing.T) { t.Run("returns a list of resources", func(t *testing.T) { expected := "* test-name (test-id)" - result, err := output.CmdOutput("list", "plaintext", []byte(input)) + result, err := output.CmdOutput("list", "plaintext", []byte(input), nil) require.NoError(t, err) assert.Equal(t, expected, result) @@ -127,7 +127,7 @@ func TestCmdOutput(t *testing.T) { t.Run("returns the list", func(t *testing.T) { expected := "* tag1\n* tag2\nShowing results 1 - 2 of 2." - result, err := output.CmdOutput("list", "plaintext", []byte(input)) + result, err := output.CmdOutput("list", "plaintext", []byte(input), nil) require.NoError(t, err) assert.Equal(t, expected, result) @@ -144,7 +144,7 @@ func TestCmdOutput(t *testing.T) { t.Run("with JSON output", func(t *testing.T) { t.Run("returns the JSON", func(t *testing.T) { - result, err := output.CmdOutput("create", "json", []byte(input)) + result, err := output.CmdOutput("create", "json", []byte(input), nil) require.NoError(t, err) assert.JSONEq(t, input, result) @@ -155,7 +155,7 @@ func TestCmdOutput(t *testing.T) { t.Run("returns a success message", func(t *testing.T) { expected := "Successfully created test-name (test-key)" - result, err := output.CmdOutput("create", "plaintext", []byte(input)) + result, err := output.CmdOutput("create", "plaintext", []byte(input), nil) require.NoError(t, err) assert.Equal(t, expected, result) @@ -176,7 +176,7 @@ func TestCmdOutput(t *testing.T) { t.Run("with JSON output", func(t *testing.T) { t.Run("returns the JSON", func(t *testing.T) { - result, err := output.CmdOutput("create", "json", []byte(input)) + result, err := output.CmdOutput("create", "json", []byte(input), nil) require.NoError(t, err) assert.JSONEq(t, input, result) @@ -187,7 +187,7 @@ func TestCmdOutput(t *testing.T) { t.Run("returns a success message", func(t *testing.T) { expected := "Successfully created\n* test-name (test-key)" - result, err := output.CmdOutput("create", "plaintext", []byte(input)) + result, err := output.CmdOutput("create", "plaintext", []byte(input), nil) require.NoError(t, err) assert.Equal(t, expected, result) @@ -208,7 +208,7 @@ func TestCmdOutput(t *testing.T) { t.Run("with JSON output", func(t *testing.T) { t.Run("returns the JSON", func(t *testing.T) { - result, err := output.CmdOutput("create", "json", []byte(input)) + result, err := output.CmdOutput("create", "json", []byte(input), nil) require.NoError(t, err) assert.JSONEq(t, input, result) @@ -219,7 +219,7 @@ func TestCmdOutput(t *testing.T) { t.Run("returns a success message", func(t *testing.T) { expected := "Successfully created\n* test-email (test-id)" - result, err := output.CmdOutput("create", "plaintext", []byte(input)) + result, err := output.CmdOutput("create", "plaintext", []byte(input), nil) require.NoError(t, err) assert.Equal(t, expected, result) @@ -235,7 +235,7 @@ func TestCmdOutput(t *testing.T) { t.Run("with JSON output", func(t *testing.T) { t.Run("does not return anything", func(t *testing.T) { - result, err := output.CmdOutput("delete", "json", []byte("")) + result, err := output.CmdOutput("delete", "json", []byte(""), nil) require.NoError(t, err) assert.Equal(t, "", result) @@ -247,7 +247,7 @@ func TestCmdOutput(t *testing.T) { t.Run("returns a success message", func(t *testing.T) { expected := "Successfully deleted test-name (test-key)" - result, err := output.CmdOutput("delete", "plaintext", []byte(input)) + result, err := output.CmdOutput("delete", "plaintext", []byte(input), nil) require.NoError(t, err) assert.Equal(t, expected, result) @@ -261,7 +261,7 @@ func TestCmdOutput(t *testing.T) { "key": "test-key" }` - result, err := output.CmdOutput("delete", "plaintext", []byte(input)) + result, err := output.CmdOutput("delete", "plaintext", []byte(input), nil) require.NoError(t, err) assert.Equal(t, expected, result) @@ -279,7 +279,7 @@ func TestCmdOutput(t *testing.T) { t.Run("with JSON output", func(t *testing.T) { t.Run("returns the JSON", func(t *testing.T) { - result, err := output.CmdOutput("update", "json", []byte(input)) + result, err := output.CmdOutput("update", "json", []byte(input), nil) require.NoError(t, err) assert.JSONEq(t, input, result) @@ -290,7 +290,7 @@ func TestCmdOutput(t *testing.T) { t.Run("returns a success message", func(t *testing.T) { expected := "Successfully updated test-name (test-key)" - result, err := output.CmdOutput("update", "plaintext", []byte(input)) + result, err := output.CmdOutput("update", "plaintext", []byte(input), nil) require.NoError(t, err) assert.Equal(t, expected, result) @@ -395,3 +395,572 @@ func TestCmdOutputError(t *testing.T) { }) }) } + +func TestCmdOutputWithFields(t *testing.T) { + t.Run("filters singular resource to requested fields", func(t *testing.T) { + input := `{"key":"my-flag","name":"My Flag","kind":"boolean","temporary":true,"extra":"noise"}` + + result, err := output.CmdOutput("list", "json", []byte(input), []string{"key", "name"}) + + require.NoError(t, err) + assert.JSONEq(t, `{"key":"my-flag","name":"My Flag"}`, result) + }) + + t.Run("filters collection items and preserves totalCount and _links", func(t *testing.T) { + input := `{ + "items": [ + {"key":"flag-1","name":"Flag 1","kind":"boolean","extra":"noise"}, + {"key":"flag-2","name":"Flag 2","kind":"multivariate","extra":"more noise"} + ], + "totalCount": 2, + "_links": { + "self": {"href":"/api/v2/flags/proj?limit=20&offset=0","type":"application/json"} + } + }` + + result, err := output.CmdOutput("list", "json", []byte(input), []string{"key", "name"}) + + require.NoError(t, err) + assert.JSONEq(t, `{ + "items": [ + {"key":"flag-1","name":"Flag 1"}, + {"key":"flag-2","name":"Flag 2"} + ], + "totalCount": 2, + "_links": { + "self": {"href":"/api/v2/flags/proj?limit=20&offset=0","type":"application/json"} + } + }`, result) + }) + + t.Run("omits non-existent fields without error", func(t *testing.T) { + input := `{"key":"my-flag","name":"My Flag"}` + + result, err := output.CmdOutput("list", "json", []byte(input), []string{"key", "doesNotExist"}) + + require.NoError(t, err) + assert.JSONEq(t, `{"key":"my-flag"}`, result) + }) + + t.Run("returns full JSON when fields is empty slice", func(t *testing.T) { + input := `{"key":"my-flag","name":"My Flag","kind":"boolean"}` + + result, err := output.CmdOutput("list", "json", []byte(input), []string{}) + + require.NoError(t, err) + assert.JSONEq(t, input, result) + }) + + t.Run("returns full JSON when fields is nil", func(t *testing.T) { + input := `{"key":"my-flag","name":"My Flag","kind":"boolean"}` + + result, err := output.CmdOutput("list", "json", []byte(input), nil) + + require.NoError(t, err) + assert.JSONEq(t, input, result) + }) + + t.Run("returns original bytes when input is invalid JSON", func(t *testing.T) { + input := `not valid json at all` + + result, err := output.CmdOutput("list", "json", []byte(input), []string{"key"}) + + require.NoError(t, err) + assert.Equal(t, input, result) + }) + + t.Run("preserves nested objects intact when field is selected", func(t *testing.T) { + input := `{ + "key":"my-flag", + "name":"My Flag", + "environments":{ + "production":{"on":true,"rules":[]}, + "staging":{"on":false,"rules":[]} + }, + "variations":[{"value":true},{"value":false}] + }` + + result, err := output.CmdOutput("list", "json", []byte(input), []string{"key", "environments"}) + + require.NoError(t, err) + assert.JSONEq(t, `{ + "key":"my-flag", + "environments":{ + "production":{"on":true,"rules":[]}, + "staging":{"on":false,"rules":[]} + } + }`, result) + }) + + t.Run("trims whitespace from field names", func(t *testing.T) { + input := `{"key":"my-flag","name":"My Flag","kind":"boolean"}` + + result, err := output.CmdOutput("list", "json", []byte(input), []string{" key ", " name"}) + + require.NoError(t, err) + assert.JSONEq(t, `{"key":"my-flag","name":"My Flag"}`, result) + }) + + t.Run("is ignored when output is plaintext", func(t *testing.T) { + input := `{"key":"test-key","name":"test-name","extra":"extra-value"}` + + result, err := output.CmdOutput("create", "plaintext", []byte(input), []string{"key"}) + + require.NoError(t, err) + assert.Equal(t, "Successfully created test-name (test-key)", result) + }) + + t.Run("is ignored when output is plaintext for collections", func(t *testing.T) { + input := `{"items":[{"key":"test-key","name":"test-name","extra":"extra-value"}]}` + + result, err := output.CmdOutput("list", "plaintext", []byte(input), []string{"key"}) + + require.NoError(t, err) + assert.Equal(t, "* test-name (test-key)", result) + }) + + t.Run("handles empty JSON object without panic", func(t *testing.T) { + input := `{}` + + result, err := output.CmdOutput("list", "json", []byte(input), []string{"key"}) + + require.NoError(t, err) + assert.JSONEq(t, `{}`, result) + }) + + t.Run("handles empty collection with fields", func(t *testing.T) { + input := `{"items":[],"totalCount":0}` + + result, err := output.CmdOutput("list", "json", []byte(input), []string{"key"}) + + require.NoError(t, err) + assert.JSONEq(t, `{"items":[],"totalCount":0}`, result) + }) + + t.Run("preserves scalar items in collection", func(t *testing.T) { + input := `{"items":["tag1","tag2"],"totalCount":2}` + + result, err := output.CmdOutput("list", "json", []byte(input), []string{"key"}) + + require.NoError(t, err) + assert.JSONEq(t, `{"items":["tag1","tag2"],"totalCount":2}`, result) + }) + + t.Run("returns all fields when every field is requested", func(t *testing.T) { + input := `{"key":"my-flag","name":"My Flag"}` + + result, err := output.CmdOutput("list", "json", []byte(input), []string{"key", "name"}) + + require.NoError(t, err) + assert.JSONEq(t, input, result) + }) + + t.Run("returns original input for top-level JSON array", func(t *testing.T) { + input := `[{"key":"x"},{"key":"y"}]` + + result, err := output.CmdOutput("list", "json", []byte(input), []string{"key"}) + + require.NoError(t, err) + assert.Equal(t, input, result) + }) + + t.Run("deduplicates repeated field names", func(t *testing.T) { + input := `{"key":"my-flag","name":"My Flag","extra":"noise"}` + + result, err := output.CmdOutput("list", "json", []byte(input), []string{"key", "key", "name"}) + + require.NoError(t, err) + assert.JSONEq(t, `{"key":"my-flag","name":"My Flag"}`, result) + }) + + t.Run("skips empty field strings and filters normally", func(t *testing.T) { + input := `{"key":"my-flag","name":"My Flag","extra":"noise"}` + + result, err := output.CmdOutput("list", "json", []byte(input), []string{"", " ", "key"}) + + require.NoError(t, err) + assert.JSONEq(t, `{"key":"my-flag"}`, result) + }) + + t.Run("returns full JSON when all field strings are empty", func(t *testing.T) { + input := `{"key":"my-flag","name":"My Flag"}` + + result, err := output.CmdOutput("list", "json", []byte(input), []string{"", " "}) + + require.NoError(t, err) + assert.JSONEq(t, input, result) + }) +} + +func TestCmdOutputWithResourceName(t *testing.T) { + t.Run("flags list uses table format", func(t *testing.T) { + input := `{ + "items": [ + { + "key": "my-flag", + "name": "My Flag", + "kind": "boolean", + "temporary": true, + "tags": ["beta", "frontend"] + }, + { + "key": "dark-mode", + "name": "Dark Mode", + "kind": "boolean", + "temporary": false, + "tags": ["ui"] + } + ] + }` + + result, err := output.CmdOutput("list", "plaintext", []byte(input), nil, output.CmdOutputOpts{ + ResourceName: "flags", + }) + + require.NoError(t, err) + assert.Contains(t, result, "KEY") + assert.Contains(t, result, "NAME") + assert.Contains(t, result, "KIND") + assert.Contains(t, result, "TEMPORARY") + assert.Contains(t, result, "TAGS") + assert.Contains(t, result, "my-flag") + assert.Contains(t, result, "My Flag") + assert.Contains(t, result, "yes") + assert.Contains(t, result, "dark-mode") + assert.Contains(t, result, "no") + assert.NotContains(t, result, "* ") + }) + + t.Run("flags singular uses key-value format", func(t *testing.T) { + input := `{ + "key": "my-flag", + "name": "My Flag", + "kind": "boolean", + "temporary": true, + "creationDate": 1718438400000, + "tags": ["beta"] + }` + + result, err := output.CmdOutput("get", "plaintext", []byte(input), nil, output.CmdOutputOpts{ + ResourceName: "flags", + }) + + require.NoError(t, err) + assert.Contains(t, result, "Key:") + assert.Contains(t, result, "my-flag") + assert.Contains(t, result, "Name:") + assert.Contains(t, result, "My Flag") + assert.Contains(t, result, "Kind:") + assert.Contains(t, result, "boolean") + assert.Contains(t, result, "Temporary:") + assert.Contains(t, result, "yes") + }) + + t.Run("flags singular with success message", func(t *testing.T) { + input := `{ + "key": "my-flag", + "name": "My Flag", + "kind": "boolean", + "temporary": false + }` + + result, err := output.CmdOutput("update", "plaintext", []byte(input), nil, output.CmdOutputOpts{ + ResourceName: "flags", + }) + + require.NoError(t, err) + assert.Contains(t, result, "Successfully updated\n\nKey:") + assert.Contains(t, result, "my-flag") + }) + + t.Run("unknown resource falls back to bullet format for lists", func(t *testing.T) { + input := `{ + "items": [ + { + "key": "test-key", + "name": "test-name" + } + ] + }` + + result, err := output.CmdOutput("list", "plaintext", []byte(input), nil, output.CmdOutputOpts{ + ResourceName: "unknown-resource", + }) + + require.NoError(t, err) + assert.Equal(t, "* test-name (test-key)", result) + }) + + t.Run("unknown resource falls back to name (key) for singular", func(t *testing.T) { + input := `{ + "key": "test-key", + "name": "test-name" + }` + + result, err := output.CmdOutput("create", "plaintext", []byte(input), nil, output.CmdOutputOpts{ + ResourceName: "unknown-resource", + }) + + require.NoError(t, err) + assert.Equal(t, "Successfully created test-name (test-key)", result) + }) + + t.Run("empty resource name falls back to bullet format", func(t *testing.T) { + input := `{ + "items": [ + { + "key": "test-key", + "name": "test-name" + } + ] + }` + + result, err := output.CmdOutput("list", "plaintext", []byte(input), nil) + + require.NoError(t, err) + assert.Equal(t, "* test-name (test-key)", result) + }) + + t.Run("table output with pagination", func(t *testing.T) { + input := `{ + "_links": { + "self": { + "href": "/api/v2/flags/proj?limit=5&offset=0", + "type": "application/json" + } + }, + "items": [ + { + "key": "my-flag", + "name": "My Flag", + "kind": "boolean", + "temporary": true, + "tags": ["beta"] + } + ], + "totalCount": 100 + }` + + result, err := output.CmdOutput("list", "plaintext", []byte(input), nil, output.CmdOutputOpts{ + ResourceName: "flags", + }) + + require.NoError(t, err) + assert.Contains(t, result, "KEY") + assert.Contains(t, result, "my-flag") + assert.Contains(t, result, "Showing results 1 - 5 of 100.") + assert.Contains(t, result, "Use --offset 5 for additional results.") + }) + + t.Run("empty items with resource name returns no items found", func(t *testing.T) { + input := `{"items": []}` + + result, err := output.CmdOutput("list", "plaintext", []byte(input), nil, output.CmdOutputOpts{ + ResourceName: "flags", + }) + + require.NoError(t, err) + assert.Equal(t, "No items found", result) + }) + + t.Run("JSON output is unaffected by resource name", func(t *testing.T) { + input := `{ + "items": [ + {"key": "my-flag", "name": "My Flag"} + ] + }` + + result, err := output.CmdOutput("list", "json", []byte(input), nil, output.CmdOutputOpts{ + ResourceName: "flags", + }) + + require.NoError(t, err) + assert.JSONEq(t, input, result) + }) + + t.Run("members list uses table format", func(t *testing.T) { + input := `{ + "items": [ + { + "email": "alice@example.com", + "role": "admin", + "lastName": "Smith", + "firstName": "Alice" + } + ] + }` + + result, err := output.CmdOutput("list", "plaintext", []byte(input), nil, output.CmdOutputOpts{ + ResourceName: "members", + }) + + require.NoError(t, err) + assert.Contains(t, result, "EMAIL") + assert.Contains(t, result, "ROLE") + assert.Contains(t, result, "alice@example.com") + assert.Contains(t, result, "admin") + assert.NotContains(t, result, "* ") + }) + + t.Run("environments list uses table format", func(t *testing.T) { + input := `{ + "items": [ + { + "key": "production", + "name": "Production", + "color": "FF0000" + } + ] + }` + + result, err := output.CmdOutput("list", "plaintext", []byte(input), nil, output.CmdOutputOpts{ + ResourceName: "environments", + }) + + require.NoError(t, err) + assert.Contains(t, result, "KEY") + assert.Contains(t, result, "NAME") + assert.Contains(t, result, "COLOR") + assert.Contains(t, result, "production") + assert.Contains(t, result, "FF0000") + }) + + t.Run("projects list uses table format", func(t *testing.T) { + input := `{ + "items": [ + { + "key": "proj-1", + "name": "Project One", + "tags": ["tag1", "tag2"] + } + ] + }` + + result, err := output.CmdOutput("list", "plaintext", []byte(input), nil, output.CmdOutputOpts{ + ResourceName: "projects", + }) + + require.NoError(t, err) + assert.Contains(t, result, "KEY") + assert.Contains(t, result, "proj-1") + assert.Contains(t, result, "2") + }) + + t.Run("segments list uses table format", func(t *testing.T) { + input := `{ + "items": [ + { + "key": "beta-users", + "name": "Beta Users", + "creationDate": 1718438400000 + } + ] + }` + + result, err := output.CmdOutput("list", "plaintext", []byte(input), nil, output.CmdOutputOpts{ + ResourceName: "segments", + }) + + require.NoError(t, err) + assert.Contains(t, result, "KEY") + assert.Contains(t, result, "CREATED") + assert.Contains(t, result, "beta-users") + assert.Contains(t, result, "2024-06-15") + }) + + t.Run("create with list and resource name shows table with success message", func(t *testing.T) { + input := `{ + "items": [ + { + "key": "new-flag", + "name": "New Flag", + "kind": "boolean", + "temporary": false, + "tags": [] + } + ] + }` + + result, err := output.CmdOutput("create", "plaintext", []byte(input), nil, output.CmdOutputOpts{ + ResourceName: "flags", + }) + + require.NoError(t, err) + assert.Contains(t, result, "Successfully created") + assert.Contains(t, result, "KEY") + assert.Contains(t, result, "new-flag") + }) + + t.Run("segments singular uses key-value format", func(t *testing.T) { + input := `{ + "key": "beta-users", + "name": "Beta Users", + "creationDate": 1718438400000 + }` + + result, err := output.CmdOutput("get", "plaintext", []byte(input), nil, output.CmdOutputOpts{ + ResourceName: "segments", + }) + + require.NoError(t, err) + assert.Contains(t, result, "Key:") + assert.Contains(t, result, "beta-users") + assert.Contains(t, result, "Name:") + assert.Contains(t, result, "Beta Users") + assert.Contains(t, result, "Created:") + assert.Contains(t, result, "2024-06-15") + }) + + t.Run("Fields from opts used when fields param is nil", func(t *testing.T) { + input := `{"key":"my-flag","name":"My Flag","extra":"noise"}` + + result, err := output.CmdOutput("get", "json", []byte(input), nil, output.CmdOutputOpts{ + Fields: []string{"key", "name"}, + ResourceName: "flags", + }) + + require.NoError(t, err) + assert.JSONEq(t, `{"key":"my-flag","name":"My Flag"}`, result) + }) + + t.Run("explicit fields param takes precedence over opts Fields", func(t *testing.T) { + input := `{"key":"my-flag","name":"My Flag","extra":"noise"}` + + result, err := output.CmdOutput("get", "json", []byte(input), []string{"key"}, output.CmdOutputOpts{ + Fields: []string{"key", "name"}, + ResourceName: "flags", + }) + + require.NoError(t, err) + assert.JSONEq(t, `{"key":"my-flag"}`, result) + }) + + t.Run("empty items without resource name returns no items found", func(t *testing.T) { + input := `{"items": []}` + + result, err := output.CmdOutput("list", "plaintext", []byte(input), nil) + + require.NoError(t, err) + assert.Equal(t, "No items found", result) + }) +} + +func TestCmdOutputErrorNotAffectedByFields(t *testing.T) { + t.Run("JSON error response always returns full structure", func(t *testing.T) { + errJSON := `{"code":"not_found","message":"Not Found","statusCode":404,"suggestion":"Try ldcli flags list."}` + err := errors.NewError(errJSON) + + result := output.CmdOutputError("json", err) + + assert.JSONEq(t, errJSON, result) + }) + + t.Run("plaintext error response always returns full structure", func(t *testing.T) { + errJSON := `{"code":"conflict","message":"an error"}` + err := errors.NewError(errJSON) + + result := output.CmdOutputError("plaintext", err) + + assert.Equal(t, "an error (code: conflict)", result) + }) +} diff --git a/internal/output/table.go b/internal/output/table.go new file mode 100644 index 00000000..ddd4b004 --- /dev/null +++ b/internal/output/table.go @@ -0,0 +1,198 @@ +package output + +import ( + "bytes" + "fmt" + "strings" + "text/tabwriter" + "time" +) + +// ColumnDef describes a single column in table or key-value output. +type ColumnDef struct { + Header string + Field string + Format func(interface{}) string +} + +func defaultFormat(v interface{}) string { + if v == nil { + return "" + } + return fmt.Sprint(v) +} + +func boolYesNo(v interface{}) string { + switch b := v.(type) { + case bool: + if b { + return "yes" + } + return "no" + default: + return defaultFormat(v) + } +} + +func truncatedList(max int) func(interface{}) string { + return func(v interface{}) string { + items, ok := v.([]interface{}) + if !ok { + return defaultFormat(v) + } + strs := make([]string, 0, len(items)) + for _, item := range items { + strs = append(strs, fmt.Sprint(item)) + } + if len(strs) <= max { + return strings.Join(strs, ", ") + } + return strings.Join(strs[:max], ", ") + ", ..." + } +} + +func countList(v interface{}) string { + items, ok := v.([]interface{}) + if !ok { + return defaultFormat(v) + } + return fmt.Sprintf("%d", len(items)) +} + +func formatTimestamp(v interface{}) string { + switch n := v.(type) { + case float64: + return time.UnixMilli(int64(n)).UTC().Format(time.RFC3339) + default: + return defaultFormat(v) + } +} + +// listColumnRegistry maps resource names to their list-view column definitions. +var listColumnRegistry = map[string][]ColumnDef{ + "flags": { + {Header: "KEY", Field: "key"}, + {Header: "NAME", Field: "name"}, + {Header: "KIND", Field: "kind"}, + {Header: "TEMPORARY", Field: "temporary", Format: boolYesNo}, + {Header: "TAGS", Field: "tags", Format: truncatedList(3)}, + }, + "projects": { + {Header: "KEY", Field: "key"}, + {Header: "NAME", Field: "name"}, + {Header: "TAG COUNT", Field: "tags", Format: countList}, + }, + "environments": { + {Header: "KEY", Field: "key"}, + {Header: "NAME", Field: "name"}, + {Header: "COLOR", Field: "color"}, + }, + "members": { + {Header: "EMAIL", Field: "email"}, + {Header: "ROLE", Field: "role"}, + {Header: "LAST NAME", Field: "lastName"}, + {Header: "FIRST NAME", Field: "firstName"}, + }, + "segments": { + {Header: "KEY", Field: "key"}, + {Header: "NAME", Field: "name"}, + {Header: "CREATED", Field: "creationDate", Format: formatTimestamp}, + }, +} + +// singularColumnRegistry maps resource names to their singular-view column definitions. +var singularColumnRegistry = map[string][]ColumnDef{ + "flags": { + {Header: "Key", Field: "key"}, + {Header: "Name", Field: "name"}, + {Header: "Kind", Field: "kind"}, + {Header: "Temporary", Field: "temporary", Format: boolYesNo}, + {Header: "Created", Field: "creationDate", Format: formatTimestamp}, + {Header: "Tags", Field: "tags", Format: truncatedList(10)}, + }, + "projects": { + {Header: "Key", Field: "key"}, + {Header: "Name", Field: "name"}, + {Header: "Tag Count", Field: "tags", Format: countList}, + }, + "environments": { + {Header: "Key", Field: "key"}, + {Header: "Name", Field: "name"}, + {Header: "Color", Field: "color"}, + }, + "members": { + {Header: "Email", Field: "email"}, + {Header: "Role", Field: "role"}, + {Header: "Last Name", Field: "lastName"}, + {Header: "First Name", Field: "firstName"}, + }, + "segments": { + {Header: "Key", Field: "key"}, + {Header: "Name", Field: "name"}, + {Header: "Created", Field: "creationDate", Format: formatTimestamp}, + }, +} + +// GetListColumns returns the column definitions for a resource's list view, or nil if none are registered. +func GetListColumns(resourceName string) []ColumnDef { + return listColumnRegistry[resourceName] +} + +// GetSingularColumns returns the column definitions for a resource's singular view, or nil if none are registered. +func GetSingularColumns(resourceName string) []ColumnDef { + return singularColumnRegistry[resourceName] +} + +func colValue(r resource, col ColumnDef) string { + if r == nil { + return "" + } + format := col.Format + if format == nil { + format = defaultFormat + } + return strings.ReplaceAll(format(r[col.Field]), "\t", " ") +} + +// TableOutput formats a slice of resources as an aligned table using tabwriter. +func TableOutput(items []resource, cols []ColumnDef) string { + var buf bytes.Buffer + w := tabwriter.NewWriter(&buf, 0, 0, 3, ' ', 0) + + headers := make([]string, len(cols)) + for i, col := range cols { + headers[i] = col.Header + } + fmt.Fprintln(w, strings.Join(headers, "\t")) + + for _, item := range items { + vals := make([]string, len(cols)) + for i, col := range cols { + vals[i] = colValue(item, col) + } + fmt.Fprintln(w, strings.Join(vals, "\t")) + } + + w.Flush() + + return strings.TrimRight(buf.String(), "\n") +} + +// KeyValueOutput formats a single resource as key-value pairs. +func KeyValueOutput(r resource, cols []ColumnDef) string { + maxLen := 0 + for _, col := range cols { + if len(col.Header) > maxLen { + maxLen = len(col.Header) + } + } + + lines := make([]string, 0, len(cols)) + for _, col := range cols { + val := colValue(r, col) + padding := strings.Repeat(" ", maxLen-len(col.Header)) + lines = append(lines, fmt.Sprintf("%s:%s %s", col.Header, padding, val)) + } + + return strings.Join(lines, "\n") +} diff --git a/internal/output/table_test.go b/internal/output/table_test.go new file mode 100644 index 00000000..b2d8fc0f --- /dev/null +++ b/internal/output/table_test.go @@ -0,0 +1,410 @@ +package output + +import ( + "strings" + "testing" + "time" + + "github.com/stretchr/testify/assert" +) + +func TestTableOutput(t *testing.T) { + t.Run("flags list shows key, name, kind, temporary, tags", func(t *testing.T) { + cols := GetListColumns("flags") + items := []resource{ + { + "key": "my-flag", + "name": "My Feature Flag", + "kind": "boolean", + "temporary": true, + "tags": []interface{}{"beta", "frontend"}, + }, + { + "key": "dark-mode", + "name": "Dark Mode Toggle", + "kind": "boolean", + "temporary": false, + "tags": []interface{}{"ui"}, + }, + } + + result := TableOutput(items, cols) + lines := strings.Split(result, "\n") + + assert.Len(t, lines, 3) + assert.Contains(t, lines[0], "KEY") + assert.Contains(t, lines[0], "NAME") + assert.Contains(t, lines[0], "KIND") + assert.Contains(t, lines[0], "TEMPORARY") + assert.Contains(t, lines[0], "TAGS") + assert.Contains(t, lines[1], "my-flag") + assert.Contains(t, lines[1], "My Feature Flag") + assert.Contains(t, lines[1], "boolean") + assert.Contains(t, lines[1], "yes") + assert.Contains(t, lines[1], "beta, frontend") + assert.Contains(t, lines[2], "dark-mode") + assert.Contains(t, lines[2], "no") + }) + + t.Run("flags list truncates tags beyond 3", func(t *testing.T) { + cols := GetListColumns("flags") + items := []resource{ + { + "key": "many-tags", + "name": "Many Tags", + "kind": "boolean", + "temporary": false, + "tags": []interface{}{"a", "b", "c", "d"}, + }, + } + + result := TableOutput(items, cols) + + assert.Contains(t, result, "a, b, c, ...") + }) + + t.Run("projects list shows key, name, tag count", func(t *testing.T) { + cols := GetListColumns("projects") + items := []resource{ + { + "key": "proj-1", + "name": "Project One", + "tags": []interface{}{"tag1", "tag2", "tag3"}, + }, + } + + result := TableOutput(items, cols) + lines := strings.Split(result, "\n") + + assert.Len(t, lines, 2) + assert.Contains(t, lines[0], "KEY") + assert.Contains(t, lines[0], "NAME") + assert.Contains(t, lines[0], "TAG COUNT") + assert.Contains(t, lines[1], "proj-1") + assert.Contains(t, lines[1], "Project One") + assert.Contains(t, lines[1], "3") + }) + + t.Run("environments list shows key, name, color", func(t *testing.T) { + cols := GetListColumns("environments") + items := []resource{ + { + "key": "production", + "name": "Production", + "color": "FF0000", + }, + } + + result := TableOutput(items, cols) + lines := strings.Split(result, "\n") + + assert.Len(t, lines, 2) + assert.Contains(t, lines[0], "KEY") + assert.Contains(t, lines[0], "NAME") + assert.Contains(t, lines[0], "COLOR") + assert.Contains(t, lines[1], "production") + assert.Contains(t, lines[1], "Production") + assert.Contains(t, lines[1], "FF0000") + }) + + t.Run("members list shows email, role, lastName, firstName", func(t *testing.T) { + cols := GetListColumns("members") + items := []resource{ + { + "email": "alice@example.com", + "role": "admin", + "lastName": "Smith", + "firstName": "Alice", + }, + } + + result := TableOutput(items, cols) + lines := strings.Split(result, "\n") + + assert.Len(t, lines, 2) + assert.Contains(t, lines[0], "EMAIL") + assert.Contains(t, lines[0], "ROLE") + assert.Contains(t, lines[0], "LAST NAME") + assert.Contains(t, lines[0], "FIRST NAME") + assert.Contains(t, lines[1], "alice@example.com") + assert.Contains(t, lines[1], "admin") + assert.Contains(t, lines[1], "Smith") + assert.Contains(t, lines[1], "Alice") + }) + + t.Run("segments list shows key, name, creationDate", func(t *testing.T) { + cols := GetListColumns("segments") + items := []resource{ + { + "key": "beta-users", + "name": "Beta Users", + "creationDate": float64(1718438400000), + }, + } + + result := TableOutput(items, cols) + lines := strings.Split(result, "\n") + + assert.Len(t, lines, 2) + expected := time.UnixMilli(1718438400000).UTC().Format(time.RFC3339) + assert.Contains(t, lines[0], "KEY") + assert.Contains(t, lines[0], "NAME") + assert.Contains(t, lines[0], "CREATED") + assert.Contains(t, lines[1], "beta-users") + assert.Contains(t, lines[1], "Beta Users") + assert.Contains(t, lines[1], expected) + }) + + t.Run("handles nil field values gracefully", func(t *testing.T) { + cols := GetListColumns("flags") + items := []resource{ + { + "key": "no-extras", + "name": "No Extras", + }, + } + + result := TableOutput(items, cols) + + assert.Contains(t, result, "no-extras") + assert.Contains(t, result, "No Extras") + }) + + t.Run("multiple rows are aligned", func(t *testing.T) { + cols := GetListColumns("environments") + items := []resource{ + {"key": "short", "name": "S", "color": "000"}, + {"key": "a-very-long-key", "name": "A Very Long Name", "color": "FFF"}, + } + + result := TableOutput(items, cols) + lines := strings.Split(result, "\n") + + assert.Len(t, lines, 3) + headerKeyEnd := strings.Index(lines[0], "NAME") + row1KeyEnd := strings.Index(lines[1], "S") + row2KeyEnd := strings.Index(lines[2], "A Very Long Name") + assert.Equal(t, headerKeyEnd, row1KeyEnd) + assert.Equal(t, headerKeyEnd, row2KeyEnd) + }) +} + +func TestKeyValueOutput(t *testing.T) { + t.Run("flags singular shows key-value pairs", func(t *testing.T) { + cols := GetSingularColumns("flags") + r := resource{ + "key": "my-flag", + "name": "My Feature Flag", + "kind": "boolean", + "temporary": true, + "creationDate": float64(1718438400000), + "tags": []interface{}{"beta", "frontend"}, + } + + result := KeyValueOutput(r, cols) + + expected := time.UnixMilli(1718438400000).UTC().Format(time.RFC3339) + assert.Contains(t, result, "Key:") + assert.Contains(t, result, "my-flag") + assert.Contains(t, result, "Name:") + assert.Contains(t, result, "My Feature Flag") + assert.Contains(t, result, "Kind:") + assert.Contains(t, result, "boolean") + assert.Contains(t, result, "Temporary:") + assert.Contains(t, result, "yes") + assert.Contains(t, result, "Created:") + assert.Contains(t, result, expected) + assert.Contains(t, result, "Tags:") + assert.Contains(t, result, "beta, frontend") + }) + + t.Run("members singular shows email, role, names", func(t *testing.T) { + cols := GetSingularColumns("members") + r := resource{ + "email": "alice@example.com", + "role": "admin", + "lastName": "Smith", + "firstName": "Alice", + } + + result := KeyValueOutput(r, cols) + + assert.Contains(t, result, "Email:") + assert.Contains(t, result, "alice@example.com") + assert.Contains(t, result, "Role:") + assert.Contains(t, result, "admin") + }) + + t.Run("handles nil values", func(t *testing.T) { + cols := GetSingularColumns("flags") + r := resource{ + "key": "minimal", + "name": "Minimal Flag", + } + + result := KeyValueOutput(r, cols) + + assert.Contains(t, result, "Key: minimal") + assert.Contains(t, result, "Name: Minimal Flag") + assert.Contains(t, result, "Kind:") + }) +} + +func TestGetListColumns(t *testing.T) { + t.Run("returns nil for unknown resource", func(t *testing.T) { + cols := GetListColumns("unknown-resource") + + assert.Nil(t, cols) + }) + + t.Run("returns columns for flags", func(t *testing.T) { + cols := GetListColumns("flags") + + assert.NotNil(t, cols) + assert.Equal(t, "KEY", cols[0].Header) + }) +} + +func TestGetSingularColumns(t *testing.T) { + t.Run("returns nil for unknown resource", func(t *testing.T) { + cols := GetSingularColumns("unknown-resource") + + assert.Nil(t, cols) + }) + + t.Run("returns columns for flags", func(t *testing.T) { + cols := GetSingularColumns("flags") + + assert.NotNil(t, cols) + assert.Equal(t, "Key", cols[0].Header) + }) +} + +func TestBoolYesNo(t *testing.T) { + assert.Equal(t, "yes", boolYesNo(true)) + assert.Equal(t, "no", boolYesNo(false)) + assert.Equal(t, "something", boolYesNo("something")) + assert.Equal(t, "", boolYesNo(nil)) +} + +func TestTruncatedList(t *testing.T) { + fn := truncatedList(3) + + t.Run("within limit", func(t *testing.T) { + result := fn([]interface{}{"a", "b"}) + assert.Equal(t, "a, b", result) + }) + + t.Run("at limit", func(t *testing.T) { + result := fn([]interface{}{"a", "b", "c"}) + assert.Equal(t, "a, b, c", result) + }) + + t.Run("over limit", func(t *testing.T) { + result := fn([]interface{}{"a", "b", "c", "d"}) + assert.Equal(t, "a, b, c, ...", result) + }) + + t.Run("empty list", func(t *testing.T) { + result := fn([]interface{}{}) + assert.Equal(t, "", result) + }) + + t.Run("non-slice value", func(t *testing.T) { + result := fn("not-a-list") + assert.Equal(t, "not-a-list", result) + }) + + t.Run("nil value", func(t *testing.T) { + result := fn(nil) + assert.Equal(t, "", result) + }) +} + +func TestCountList(t *testing.T) { + t.Run("counts items", func(t *testing.T) { + result := countList([]interface{}{"a", "b", "c"}) + assert.Equal(t, "3", result) + }) + + t.Run("empty list", func(t *testing.T) { + result := countList([]interface{}{}) + assert.Equal(t, "0", result) + }) + + t.Run("non-slice", func(t *testing.T) { + result := countList("not-a-list") + assert.Equal(t, "not-a-list", result) + }) + + t.Run("nil value", func(t *testing.T) { + result := countList(nil) + assert.Equal(t, "", result) + }) +} + +func TestFormatTimestamp(t *testing.T) { + t.Run("formats float64 unix millis to RFC3339", func(t *testing.T) { + result := formatTimestamp(float64(1718438400000)) + expected := time.UnixMilli(1718438400000).UTC().Format(time.RFC3339) + assert.Equal(t, expected, result) + }) + + t.Run("nil returns empty string", func(t *testing.T) { + result := formatTimestamp(nil) + assert.Equal(t, "", result) + }) + + t.Run("string passes through to defaultFormat", func(t *testing.T) { + result := formatTimestamp("2024-06-15T00:00:00Z") + assert.Equal(t, "2024-06-15T00:00:00Z", result) + }) +} + +func TestTableOutputEdgeCases(t *testing.T) { + t.Run("empty items produces header only", func(t *testing.T) { + cols := GetListColumns("flags") + result := TableOutput([]resource{}, cols) + lines := strings.Split(result, "\n") + assert.Len(t, lines, 1) + assert.Contains(t, lines[0], "KEY") + }) + + t.Run("nil resource in items does not panic", func(t *testing.T) { + cols := GetListColumns("environments") + items := []resource{ + {"key": "good", "name": "Good", "color": "FFF"}, + nil, + } + result := TableOutput(items, cols) + assert.Contains(t, result, "good") + }) + + t.Run("tab in value does not break alignment", func(t *testing.T) { + cols := []ColumnDef{ + {Header: "KEY", Field: "key"}, + {Header: "NAME", Field: "name"}, + } + items := []resource{ + {"key": "has\ttab", "name": "normal"}, + } + result := TableOutput(items, cols) + assert.NotContains(t, result, "\t") + assert.Contains(t, result, "has tab") + }) +} + +func TestKeyValueOutputEdgeCases(t *testing.T) { + t.Run("empty cols returns empty string", func(t *testing.T) { + r := resource{"key": "val"} + result := KeyValueOutput(r, []ColumnDef{}) + assert.Equal(t, "", result) + }) +} + +func TestBoolYesNoEdgeCases(t *testing.T) { + t.Run("numeric float64 passes through", func(t *testing.T) { + result := boolYesNo(float64(1)) + assert.Equal(t, "1", result) + }) +}