From d6219ca86d9f8cd0343374f225a17c5e779989b5 Mon Sep 17 00:00:00 2001 From: Jos Huiting Date: Thu, 12 Mar 2026 22:00:47 +0100 Subject: [PATCH] Improve CLI UX: help text, empty results, update notification - Add concrete format examples to --after/--before flag descriptions and move timestamp examples to cobra Example field for better visibility - Show "No results." on stderr for empty list responses (interactive only); raw mode passes through the JSON unchanged for piping - Throttle update notification to once per 24h by tracking NotifiedAt in the update cache Co-Authored-By: Claude Opus 4.6 --- internal/cmd/resource.go | 23 +++++++++++------ internal/cmd/resource_test.go | 47 +++++++++++++++++++++++++++++++++++ internal/cmd/root.go | 3 ++- internal/update/check.go | 37 +++++++++++++++++++++++---- internal/update/check_test.go | 29 +++++++++++++++++++++ 5 files changed, 126 insertions(+), 13 deletions(-) diff --git a/internal/cmd/resource.go b/internal/cmd/resource.go index 80f424d..32257f1 100644 --- a/internal/cmd/resource.go +++ b/internal/cmd/resource.go @@ -1,6 +1,7 @@ package cmd import ( + "encoding/json" "fmt" "os" "strings" @@ -9,6 +10,7 @@ import ( "github.com/spf13/cobra" "github.com/jhuiting/chargebee-cli/internal/api" + "github.com/jhuiting/chargebee-cli/internal/output" "github.com/jhuiting/chargebee-cli/internal/timeutil" ) @@ -112,8 +114,9 @@ func buildOpCmd(res ResourceDef, op OpDef, factory ClientFactory) *cobra.Command cmd.Flags().String(f.Flag, "", f.Help) } if res.TimestampField != "" { - cmd.Flags().String("after", "", "only return results after this time (ISO date, unix, or relative like 7d, 24h)") - cmd.Flags().String("before", "", "only return results before this time (ISO date, unix, or relative like 7d, 24h)") + cmd.Flags().String("after", "", "filter results after this time (e.g. 2024-01-01, 7d, 24h, 1700000000)") + cmd.Flags().String("before", "", "filter results before this time (e.g. 2024-01-01, 7d, 24h, 1700000000)") + cmd.Example = fmt.Sprintf(" cb %s list --after 7d\n cb %s list --before 2024-01-01\n cb %s list -d \"%s[after]=2024-01-01\" --raw", res.Name, res.Name, res.Name, res.TimestampField) } } @@ -142,11 +145,7 @@ func opLongHelp(res ResourceDef, op OpDef) string { fmt.Fprintf(&b, " cb %s list --%s %s\n", res.Name, res.Filters[0].Flag, filterExampleValue(res.Filters[0])) } fmt.Fprintf(&b, " cb %s list -l 10\n", res.Name) - if res.TimestampField != "" { - fmt.Fprintf(&b, " cb %s list --after 2024-01-01\n", res.Name) - fmt.Fprintf(&b, " cb %s list --after 7d\n", res.Name) - fmt.Fprintf(&b, " cb %s list -d \"%s[after]=2024-01-01\" --raw", res.Name, res.TimestampField) - } else { + if res.TimestampField == "" { fmt.Fprintf(&b, " cb %s list -d \"created_at[after]=1700000000\" --raw", res.Name) } @@ -222,6 +221,16 @@ func makeRunOp(res ResourceDef, op OpDef, factory ClientFactory) func(*cobra.Com printResponseHeaders(os.Stderr, result) } + if op.Name == "list" && !raw { + var envelope struct { + List []json.RawMessage `json:"list"` + } + if json.Unmarshal(result.JSON(), &envelope) == nil && envelope.List != nil && len(envelope.List) == 0 { + output.Default.Dim("No results.") + return nil + } + } + return printJSON(os.Stdout, result.JSON(), raw) } } diff --git a/internal/cmd/resource_test.go b/internal/cmd/resource_test.go index 7dd4106..c98e41b 100644 --- a/internal/cmd/resource_test.go +++ b/internal/cmd/resource_test.go @@ -1,7 +1,9 @@ package cmd_test import ( + "bytes" "encoding/json" + "io" "net/http" "net/http/httptest" "testing" @@ -12,6 +14,7 @@ import ( "github.com/jhuiting/chargebee-cli/internal/api" "github.com/jhuiting/chargebee-cli/internal/cmd" + "github.com/jhuiting/chargebee-cli/internal/output" ) func newResourceTestCmd(baseURL string) *cobra.Command { @@ -200,3 +203,47 @@ func TestResourceRetrieveHasNoFilterFlags(t *testing.T) { assert.Nil(t, customersCmd.Flags().Lookup("company"), "retrieve should not have filter flags") assert.Nil(t, customersCmd.Flags().Lookup("email"), "retrieve should not have filter flags") } + +func TestResourceListEmptyShowsNoResults(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusOK) + _ = json.NewEncoder(w).Encode(map[string]any{"list": []any{}}) + })) + defer srv.Close() + + var stderrBuf bytes.Buffer + out := output.New(&stderrBuf, io.Discard) + + origDefault := output.Default + output.Default = out + defer func() { output.Default = origDefault }() + + root := newResourceTestCmd(srv.URL) + root.SetArgs([]string{"customers", "list"}) + err := root.Execute() + + require.NoError(t, err) + assert.Contains(t, stderrBuf.String(), "No results.") +} + +func TestResourceListEmptyRawPassesThrough(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusOK) + _, _ = w.Write([]byte(`{"list":[]}`)) + })) + defer srv.Close() + + var stderrBuf bytes.Buffer + out := output.New(&stderrBuf, io.Discard) + + origDefault := output.Default + output.Default = out + defer func() { output.Default = origDefault }() + + root := newResourceTestCmd(srv.URL) + root.SetArgs([]string{"customers", "list", "--raw"}) + err := root.Execute() + + require.NoError(t, err) + assert.NotContains(t, stderrBuf.String(), "No results.") +} diff --git a/internal/cmd/root.go b/internal/cmd/root.go index d08cb6c..2e14733 100644 --- a/internal/cmd/root.go +++ b/internal/cmd/root.go @@ -88,10 +88,11 @@ func Execute() { select { case info := <-updateCh: - if info != nil && info.UpdateAvailable { + if info != nil && info.UpdateAvailable && !info.NotifiedRecently { _, _ = fmt.Fprintln(os.Stderr) output.Default.Warning("A new version of cb is available: %s → %s", info.CurrentVersion, info.LatestVersion) output.Default.Dim("Update with: brew upgrade chargebee-cli") + update.MarkNotified() } default: } diff --git a/internal/update/check.go b/internal/update/check.go index 040e059..ab689b1 100644 --- a/internal/update/check.go +++ b/internal/update/check.go @@ -20,14 +20,16 @@ var releaseURL = "https://api.github.com/repos/jhuiting/chargebee-cli/releases/l // Info holds the result of an update check. type Info struct { - CurrentVersion string - LatestVersion string - UpdateAvailable bool + CurrentVersion string + LatestVersion string + UpdateAvailable bool + NotifiedRecently bool } type cache struct { CheckedAt time.Time `json:"checked_at"` LatestVersion string `json:"latest_version"` + NotifiedAt time.Time `json:"notified_at,omitempty"` } // CheckForUpdate checks if a newer CLI version is available. @@ -48,7 +50,9 @@ func CheckForUpdate(ctx context.Context, currentVersion string) *Info { cached, err := readCache(cachePath) if err == nil && time.Since(cached.CheckedAt) < checkInterval { - return buildInfo(currentVersion, cached.LatestVersion) + info := buildInfo(currentVersion, cached.LatestVersion) + info.NotifiedRecently = !cached.NotifiedAt.IsZero() && time.Since(cached.NotifiedAt) < checkInterval + return info } latest, err := fetchLatestVersion(ctx) @@ -56,12 +60,35 @@ func CheckForUpdate(ctx context.Context, currentVersion string) *Info { return nil } + var notifiedAt time.Time + if cached != nil { + notifiedAt = cached.NotifiedAt + } + _ = writeCache(cachePath, &cache{ CheckedAt: time.Now(), LatestVersion: latest, + NotifiedAt: notifiedAt, }) - return buildInfo(currentVersion, latest) + info := buildInfo(currentVersion, latest) + info.NotifiedRecently = !notifiedAt.IsZero() && time.Since(notifiedAt) < checkInterval + return info +} + +// MarkNotified records that the update notification was shown, so it can be +// suppressed for the next 24 hours. +func MarkNotified() { + path := cacheFilePath() + if path == "" { + return + } + cached, err := readCache(path) + if err != nil { + return + } + cached.NotifiedAt = time.Now() + _ = writeCache(path, cached) } func cacheFilePath() string { diff --git a/internal/update/check_test.go b/internal/update/check_test.go index e404f7a..0c9ed0e 100644 --- a/internal/update/check_test.go +++ b/internal/update/check_test.go @@ -119,3 +119,32 @@ func TestCheckForUpdate_UsesCache(t *testing.T) { assert.True(t, info.UpdateAvailable) assert.Equal(t, 1, callCount) } + +func TestCheckForUpdate_NotifiedRecently(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + _ = json.NewEncoder(w).Encode(map[string]string{"tag_name": "v2.0.0"}) + })) + defer srv.Close() + + origURL := releaseURL + releaseURL = srv.URL + defer func() { releaseURL = origURL }() + + t.Setenv("CB_CONFIG_DIR", t.TempDir()) + t.Setenv("CB_NO_UPDATE_CHECK", "") + + // First check: not recently notified. + info := CheckForUpdate(context.Background(), "v1.0.0") + require.NotNil(t, info) + assert.True(t, info.UpdateAvailable) + assert.False(t, info.NotifiedRecently) + + // Mark as notified. + MarkNotified() + + // Second check: recently notified. + info = CheckForUpdate(context.Background(), "v1.0.0") + require.NotNil(t, info) + assert.True(t, info.UpdateAvailable) + assert.True(t, info.NotifiedRecently) +}