From c0aac64aa82ef91156a632c4ef3a328b238ccc97 Mon Sep 17 00:00:00 2001 From: Danny Olson Date: Fri, 10 May 2024 10:24:45 -0700 Subject: [PATCH 1/3] Fix plaintext output for a resource with only and ID --- cmd/resources/resources.go | 5 +- internal/output/plaintext_fns.go | 38 +++++++++++---- internal/output/plaintext_fns_test.go | 64 +++++++++++++++++++++++++ internal/output/resource_output.go | 4 +- internal/output/resource_output_test.go | 21 ++++++++ 5 files changed, 118 insertions(+), 14 deletions(-) create mode 100644 internal/output/plaintext_fns_test.go diff --git a/cmd/resources/resources.go b/cmd/resources/resources.go index 59292b6a..d609020c 100644 --- a/cmd/resources/resources.go +++ b/cmd/resources/resources.go @@ -25,12 +25,11 @@ import ( func getResourcesHelpTemplate() string { // This template uses `.Parent` to access subcommands on the root command. - return fmt.Sprintf(`Available commands:{{range $index, $cmd := .Parent.Commands}}{{if (or (eq (index $.Parent.Annotations $cmd.Name) "resource"))}} + return `Available commands:{{range $index, $cmd := .Parent.Commands}}{{if (or (eq (index $.Parent.Annotations $cmd.Name) "resource"))}} {{rpad $cmd.Name $cmd.NamePadding }} {{$cmd.Short}}{{end}}{{end}} Use "ldcli [command] --help" for more information about a command. -`, - ) +` } func NewResourcesCmd() *cobra.Command { diff --git a/internal/output/plaintext_fns.go b/internal/output/plaintext_fns.go index c2e7679c..fbdc861b 100644 --- a/internal/output/plaintext_fns.go +++ b/internal/output/plaintext_fns.go @@ -42,27 +42,45 @@ var ErrorPlaintextOutputFn = func(r resource) string { // MultipleEmailPlaintextOutputFn converts the resource to plain text specifically for member data. var MultipleEmailPlaintextOutputFn = func(r resource) string { - return fmt.Sprintf("* %s (%s)", r["email"], r["_id"]) + return fmt.Sprintf("* %s", SingularPlaintextOutputFn(r)) } // MultipleIDPlaintextOutputFn converts the resource to plain text for data without a key. var MultipleIDPlaintextOutputFn = func(r resource) string { - return fmt.Sprintf("* %s (%s)", r["name"], r["_id"]) + return fmt.Sprintf("* %s", SingularPlaintextOutputFn(r)) +} + +// MultipleNameIDPlaintextOutputFn converts the resource to plain text for data without a key. +var MultipleNameIDPlaintextOutputFn = func(r resource) string { + return fmt.Sprintf("* %s", SingularPlaintextOutputFn(r)) } // MultiplePlaintextOutputFn converts the resource to plain text based on its name and key in a list. var MultiplePlaintextOutputFn = func(r resource) string { - return fmt.Sprintf("* %s (%s)", r["name"], r["key"]) + return fmt.Sprintf("* %s", SingularPlaintextOutputFn(r)) } // SingularPlaintextOutputFn converts the resource to plain text based on its name and key. var SingularPlaintextOutputFn = func(r resource) string { - if r["name"] == nil { - return r["key"].(string) - } - if r["key"] == nil { - return r["name"].(string) - } + email := r["email"] + id := r["_id"] + key := r["key"] + name := r["name"] - return fmt.Sprintf("%s (%s)", r["name"], r["key"]) + switch { + case name != nil && key != nil: + return fmt.Sprintf("%s (%s)", name.(string), key.(string)) + case email != nil && id != nil: + return fmt.Sprintf("%s (%s)", email.(string), id.(string)) + case name != nil && id != nil: + return fmt.Sprintf("%s (%s)", name.(string), id.(string)) + case key != nil: + return key.(string) + case email != nil: + return email.(string) + case id != nil: + return id.(string) + default: + return "cannot read resource" + } } diff --git a/internal/output/plaintext_fns_test.go b/internal/output/plaintext_fns_test.go new file mode 100644 index 00000000..7b379346 --- /dev/null +++ b/internal/output/plaintext_fns_test.go @@ -0,0 +1,64 @@ +package output + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestSingularPlaintextOutputFn(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 ID and email": { + resource: resource{ + "_id": "test-id", + "email": "test-email", + "name": "test-name", + }, + expected: "test-email (test-id)", + }, + "with a name and ID": { + resource: resource{ + "_id": "test-id", + "name": "test-name", + }, + expected: "test-name (test-id)", + }, + "with only an ID": { + resource: resource{ + "_id": "test-id", + }, + expected: "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 := SingularPlaintextOutputFn(tt.resource) + + assert.Equal(t, tt.expected, out) + }) + } +} diff --git a/internal/output/resource_output.go b/internal/output/resource_output.go index 2cfbb6da..4fee1906 100644 --- a/internal/output/resource_output.go +++ b/internal/output/resource_output.go @@ -56,8 +56,10 @@ func CmdOutput(action string, outputKind string, input []byte) (string, error) { switch { case keyExists("email"): outputFn = MultipleEmailPlaintextOutputFn - case keyExists("_id"): + case keyExists("_id") && !keyExists("name"): outputFn = MultipleIDPlaintextOutputFn + case keyExists("_id"): + outputFn = MultipleNameIDPlaintextOutputFn } items := make([]string, 0, len(maybeResources.Items)) diff --git a/internal/output/resource_output_test.go b/internal/output/resource_output_test.go index ae1639a8..61344367 100644 --- a/internal/output/resource_output_test.go +++ b/internal/output/resource_output_test.go @@ -12,6 +12,27 @@ import ( ) func TestCmdOutput(t *testing.T) { + t.Run("with multiple resources with only an ID", func(t *testing.T) { + input := `{ + "items": [ + { + "_id": "test-id" + } + ] + }` + + t.Run("with plaintext output", func(t *testing.T) { + t.Run("returns a success message", func(t *testing.T) { + expected := "\n* test-id" + + result, err := output.CmdOutput("list", "plaintext", []byte(input)) + + require.NoError(t, err) + assert.Equal(t, expected, result) + }) + }) + }) + t.Run("with multiple resources with an ID and name", func(t *testing.T) { input := `{ "items": [ From f7df45cf525625760952e7ff524a50da754a88af Mon Sep 17 00:00:00 2001 From: Danny Olson Date: Fri, 10 May 2024 10:28:29 -0700 Subject: [PATCH 2/3] Remove unused functions --- internal/output/plaintext_fns.go | 17 +---------------- internal/output/resource_output.go | 14 +------------- 2 files changed, 2 insertions(+), 29 deletions(-) diff --git a/internal/output/plaintext_fns.go b/internal/output/plaintext_fns.go index fbdc861b..f5bdb712 100644 --- a/internal/output/plaintext_fns.go +++ b/internal/output/plaintext_fns.go @@ -40,22 +40,7 @@ var ErrorPlaintextOutputFn = func(r resource) string { } } -// MultipleEmailPlaintextOutputFn converts the resource to plain text specifically for member data. -var MultipleEmailPlaintextOutputFn = func(r resource) string { - return fmt.Sprintf("* %s", SingularPlaintextOutputFn(r)) -} - -// MultipleIDPlaintextOutputFn converts the resource to plain text for data without a key. -var MultipleIDPlaintextOutputFn = func(r resource) string { - return fmt.Sprintf("* %s", SingularPlaintextOutputFn(r)) -} - -// MultipleNameIDPlaintextOutputFn converts the resource to plain text for data without a key. -var MultipleNameIDPlaintextOutputFn = func(r resource) string { - return fmt.Sprintf("* %s", SingularPlaintextOutputFn(r)) -} - -// MultiplePlaintextOutputFn converts the resource to plain text based on its name and key in a list. +// MultiplePlaintextOutputFn converts the resource to plain text. var MultiplePlaintextOutputFn = func(r resource) string { return fmt.Sprintf("* %s", SingularPlaintextOutputFn(r)) } diff --git a/internal/output/resource_output.go b/internal/output/resource_output.go index 4fee1906..c09b1b91 100644 --- a/internal/output/resource_output.go +++ b/internal/output/resource_output.go @@ -50,21 +50,9 @@ func CmdOutput(action string, outputKind string, input []byte) (string, error) { return "No items found", nil } - // the response could have various properties we want to show - keyExists := func(key string) bool { _, ok := maybeResources.Items[0][key]; return ok } - outputFn := MultiplePlaintextOutputFn - switch { - case keyExists("email"): - outputFn = MultipleEmailPlaintextOutputFn - case keyExists("_id") && !keyExists("name"): - outputFn = MultipleIDPlaintextOutputFn - case keyExists("_id"): - outputFn = MultipleNameIDPlaintextOutputFn - } - items := make([]string, 0, len(maybeResources.Items)) for _, i := range maybeResources.Items { - items = append(items, outputFn(i)) + items = append(items, MultiplePlaintextOutputFn(i)) } return plaintextOutput("\n"+strings.Join(items, "\n"), successMessage), nil From 38abc41eecfb6286cb116c310412a9d260294f37 Mon Sep 17 00:00:00 2001 From: Danny Olson Date: Fri, 10 May 2024 10:31:22 -0700 Subject: [PATCH 3/3] Rename test --- .../{plaintext_fns_test.go => plaintext_fns_internal_test.go} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename internal/output/{plaintext_fns_test.go => plaintext_fns_internal_test.go} (100%) diff --git a/internal/output/plaintext_fns_test.go b/internal/output/plaintext_fns_internal_test.go similarity index 100% rename from internal/output/plaintext_fns_test.go rename to internal/output/plaintext_fns_internal_test.go