From 793a4b33688ed93280e3d548960495c17b31d8d0 Mon Sep 17 00:00:00 2001 From: zingzy Date: Sun, 16 Aug 2026 16:38:33 +0530 Subject: [PATCH 1/2] feat(stats): migrate off the deprecated scope param Stats and export now target the account, per-link, and public endpoints directly: aliases resolve to url ids via GET /urls, unresolved codes fall back to public per-link stats, and export is login-only. The TUI tracks an account/owned/public target instead of a scope string. --- internal/api/client.go | 7 ++ internal/api/export.go | 16 +++- internal/api/stats.go | 57 ++++++++++++--- internal/api/stats_test.go | 92 ++++++++++++++++++++++- internal/api/urls.go | 18 +++++ internal/api/urls_test.go | 37 ++++++++++ internal/cmd/export.go | 30 +++++--- internal/cmd/export_test.go | 96 ++++++++++++++++++++++++ internal/cmd/stats.go | 62 ++++++++++++---- internal/cmd/stats_test.go | 118 ++++++++++++++++++++++++------ internal/tui/links/links_test.go | 5 ++ internal/tui/links/update.go | 9 +-- internal/tui/stats/data.go | 41 ++++++++--- internal/tui/stats/model.go | 32 ++++++-- internal/tui/stats/panels_test.go | 3 +- internal/tui/stats/stats_test.go | 94 +++++++++++++++++++++--- internal/tui/stats/switcher.go | 8 +- internal/tui/stats/update.go | 5 ++ internal/tui/stats/view.go | 7 +- 19 files changed, 636 insertions(+), 101 deletions(-) create mode 100644 internal/cmd/export_test.go diff --git a/internal/api/client.go b/internal/api/client.go index e6d2ac0..5f36382 100644 --- a/internal/api/client.go +++ b/internal/api/client.go @@ -72,6 +72,13 @@ func (e *APIError) Error() string { return e.Message } +// IsNotFound reports whether err is an API 404 — for the resolve-first +// endpoints that means "no such link, or not yours". +func IsNotFound(err error) bool { + var apiErr *APIError + return errors.As(err, &apiErr) && apiErr.Status == http.StatusNotFound +} + func (c *Client) do(ctx context.Context, method, path string, query url.Values, body, out any) error { resp, err := c.request(ctx, method, path, query, body) if err != nil { diff --git a/internal/api/export.go b/internal/api/export.go index cfaf4f6..f9df48e 100644 --- a/internal/api/export.go +++ b/internal/api/export.go @@ -6,15 +6,27 @@ import ( "io" "mime" "net/http" + "net/url" ) -// Export downloads stats in the given format (json, csv, xlsx, xml). +// Export downloads account-wide stats in the given format (json, csv, +// xlsx, xml). Auth is required — anonymous export no longer exists. // Returns the server-suggested filename and the file contents. // csv arrives as a ZIP archive (one CSV per dimension). func (c *Client) Export(ctx context.Context, q StatsQuery, format string) (string, []byte, error) { + return c.export(ctx, "/api/v1/export", q, format) +} + +// ExportLink downloads one owned link's stats by its url id (resolve +// an alias with ResolveAlias first). Unknown and foreign ids both 404. +func (c *Client) ExportLink(ctx context.Context, urlID string, q StatsQuery, format string) (string, []byte, error) { + return c.export(ctx, "/api/v1/export/links/"+url.PathEscape(urlID), q, format) +} + +func (c *Client) export(ctx context.Context, path string, q StatsQuery, format string) (string, []byte, error) { v := q.values() v.Set("format", format) - resp, err := c.request(ctx, http.MethodGet, "/api/v1/export", v, nil) + resp, err := c.request(ctx, http.MethodGet, path, v, nil) if err != nil { return "", nil, err } diff --git a/internal/api/stats.go b/internal/api/stats.go index aaa9dfa..74cb4d8 100644 --- a/internal/api/stats.go +++ b/internal/api/stats.go @@ -9,12 +9,13 @@ import ( "strings" ) +// StatsQuery parameterizes the authed stats endpoints. The account +// endpoint accepts every field; the per-link endpoint rejects a +// short_code group_by (the link is already picked by the path). type StatsQuery struct { - ShortCode string - Scope string // "all" (authed, optional code) or "anon" (code required) StartDate string EndDate string - GroupBy []string // time, browser, os, country, city, referrer, short_code + GroupBy []string // time, browser, os, country, city, referrer; account-only: short_code Timezone string // IANA name // Filters narrows results server-side; keys are the filterable // dimensions (browser, os, country, city, referrer, short_code). @@ -23,12 +24,6 @@ type StatsQuery struct { func (q StatsQuery) values() url.Values { v := url.Values{} - if q.Scope != "" { - v.Set("scope", q.Scope) - } - if q.ShortCode != "" { - v.Set("short_code", q.ShortCode) - } if q.StartDate != "" { v.Set("start_date", q.StartDate) } @@ -65,9 +60,10 @@ type StatsTimeRange struct { // StatsResponse keeps Metrics loosely typed: keys are dynamic // ("clicks_by_browser", "unique_clicks_by_time", ...) and each point // carries its dimension label under the dimension's own name. +// The wire still carries a legacy "scope" key — tolerated, never read. type StatsResponse struct { - Scope string `json:"scope"` - ShortCode string `json:"short_code"` + URLID string `json:"url_id"` // per-link responses echo the link + Alias string `json:"alias"` Summary StatsSummary `json:"summary"` TimeRange StatsTimeRange `json:"time_range"` Metrics map[string][]map[string]any `json:"metrics"` @@ -102,6 +98,8 @@ func (r *StatsResponse) Points(dimension, metric string) []MetricPoint { return out } +// Stats aggregates clicks across every link the account owns. +// Auth is required — anonymous stats live on PublicStats. func (c *Client) Stats(ctx context.Context, q StatsQuery) (*StatsResponse, error) { var out StatsResponse if err := c.do(ctx, http.MethodGet, "/api/v1/stats", q.values(), nil, &out); err != nil { @@ -109,3 +107,40 @@ func (c *Client) Stats(ctx context.Context, q StatsQuery) (*StatsResponse, error } return &out, nil } + +// LinkStats returns stats for one owned link by its url id (resolve an +// alias with ResolveAlias first). Unknown and foreign ids both 404. +func (c *Client) LinkStats(ctx context.Context, urlID string, q StatsQuery) (*StatsResponse, error) { + var out StatsResponse + path := "/api/v1/stats/links/" + url.PathEscape(urlID) + if err := c.do(ctx, http.MethodGet, path, q.values(), nil, &out); err != nil { + return nil, err + } + return &out, nil +} + +// PublicStats returns anyone's per-link stats without auth. The +// endpoint takes only a date range and timezone — no group_by — and +// answers with every dimension at once; private links 404 and +// password-protected ones 401. The {generation, link, stats} envelope +// is unwrapped to the standard stats wire. +func (c *Client) PublicStats(ctx context.Context, shortCode, startDate, endDate, timezone string) (*StatsResponse, error) { + v := url.Values{} + if startDate != "" { + v.Set("start_date", startDate) + } + if endDate != "" { + v.Set("end_date", endDate) + } + if timezone != "" { + v.Set("timezone", timezone) + } + var out struct { + Stats StatsResponse `json:"stats"` + } + path := "/api/v1/public/stats/" + url.PathEscape(shortCode) + if err := c.do(ctx, http.MethodGet, path, v, nil, &out); err != nil { + return nil, err + } + return &out.Stats, nil +} diff --git a/internal/api/stats_test.go b/internal/api/stats_test.go index ead7f6d..caed7ec 100644 --- a/internal/api/stats_test.go +++ b/internal/api/stats_test.go @@ -10,9 +10,12 @@ import ( func TestStatsQueryAndDecode(t *testing.T) { srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { q := r.URL.Query() - if q.Get("group_by") != "time,browser" || q.Get("scope") != "all" || q.Get("timezone") != "UTC" { + if q.Get("group_by") != "time,browser" || q.Get("timezone") != "UTC" { t.Errorf("unexpected query: %v", q) } + if q.Has("scope") { + t.Errorf("scope param must not be sent: %v", q) + } w.Write([]byte(`{ "scope": "all", "summary": {"total_clicks": 100, "unique_clicks": 60, "avg_redirection_time": 0.12}, @@ -27,7 +30,7 @@ func TestStatsQueryAndDecode(t *testing.T) { c := New(srv.URL, newTestStore(t, nil)) res, err := c.Stats(context.Background(), StatsQuery{ - Scope: "all", GroupBy: []string{"time", "browser"}, Timezone: "UTC", + GroupBy: []string{"time", "browser"}, Timezone: "UTC", }) if err != nil { t.Fatal(err) @@ -44,18 +47,81 @@ func TestStatsQueryAndDecode(t *testing.T) { } } +func TestLinkStatsHitsPerLinkPath(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path != "/api/v1/stats/links/65f0abc123" { + t.Errorf("path = %s", r.URL.Path) + } + if q := r.URL.Query(); q.Has("scope") || q.Has("short_code") { + t.Errorf("legacy params must not be sent: %v", q) + } + w.Write([]byte(`{ + "url_id": "65f0abc123", "alias": "launch", "scope": "all", + "summary": {"total_clicks": 42, "unique_clicks": 30}, + "metrics": {} + }`)) + })) + defer srv.Close() + + c := New(srv.URL, newTestStore(t, nil)) + res, err := c.LinkStats(context.Background(), "65f0abc123", StatsQuery{GroupBy: []string{"time"}}) + if err != nil { + t.Fatal(err) + } + if res.URLID != "65f0abc123" || res.Alias != "launch" || res.Summary.TotalClicks != 42 { + t.Fatalf("res = %+v", res) + } +} + +func TestPublicStatsUnwrapsEnvelope(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path != "/api/v1/public/stats/launch" { + t.Errorf("path = %s", r.URL.Path) + } + q := r.URL.Query() + if q.Get("start_date") != "2026-01-01T00:00:00Z" || q.Get("timezone") != "UTC" { + t.Errorf("unexpected query: %v", q) + } + if q.Has("group_by") || q.Has("scope") { + t.Errorf("public endpoint takes only a range and timezone: %v", q) + } + w.Write([]byte(`{ + "generation": "v2", + "link": {"alias": "launch", "domain": "spoo.me"}, + "stats": { + "scope": "anon", + "summary": {"total_clicks": 9, "unique_clicks": 5}, + "metrics": {"clicks_by_browser": [{"browser": "Chrome", "clicks": 9}]} + } + }`)) + })) + defer srv.Close() + + c := New(srv.URL, newTestStore(t, nil)) + res, err := c.PublicStats(context.Background(), "launch", "2026-01-01T00:00:00Z", "", "UTC") + if err != nil { + t.Fatal(err) + } + if res.Summary.TotalClicks != 9 || len(res.Metrics["clicks_by_browser"]) != 1 { + t.Fatalf("res = %+v", res) + } +} + func TestExportReturnsFilenameAndBytes(t *testing.T) { srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { if r.URL.Path != "/api/v1/export" || r.URL.Query().Get("format") != "xlsx" { t.Errorf("unexpected request: %s %v", r.URL.Path, r.URL.Query()) } + if r.URL.Query().Has("scope") { + t.Errorf("scope param must not be sent: %v", r.URL.Query()) + } w.Header().Set("Content-Disposition", `attachment; filename="stats-launch.xlsx"`) w.Write([]byte("FAKEXLSX")) })) defer srv.Close() c := New(srv.URL, newTestStore(t, nil)) - name, data, err := c.Export(context.Background(), StatsQuery{ShortCode: "launch"}, "xlsx") + name, data, err := c.Export(context.Background(), StatsQuery{}, "xlsx") if err != nil { t.Fatal(err) } @@ -63,3 +129,23 @@ func TestExportReturnsFilenameAndBytes(t *testing.T) { t.Fatalf("name=%q data=%q", name, data) } } + +func TestExportLinkHitsPerLinkPath(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path != "/api/v1/export/links/65f0abc123" { + t.Errorf("path = %s", r.URL.Path) + } + w.Header().Set("Content-Disposition", `attachment; filename="stats-launch.zip"`) + w.Write([]byte("FAKEZIP")) + })) + defer srv.Close() + + c := New(srv.URL, newTestStore(t, nil)) + name, data, err := c.ExportLink(context.Background(), "65f0abc123", StatsQuery{}, "csv") + if err != nil { + t.Fatal(err) + } + if name != "stats-launch.zip" || string(data) != "FAKEZIP" { + t.Fatalf("name=%q data=%q", name, data) + } +} diff --git a/internal/api/urls.go b/internal/api/urls.go index ea18022..2d6f36f 100644 --- a/internal/api/urls.go +++ b/internal/api/urls.go @@ -83,6 +83,24 @@ func (c *Client) ListURLs(ctx context.Context, opts ListURLsOptions) (*URLPage, return &out, nil } +// ResolveAlias looks up an owned link by alias via GET +// /api/v1/urls/{domain}/{alias}, mainly to obtain its url id for the +// per-link stats and export endpoints. The domain is the API base +// URL's hostname, which covers spoo.me links. Unknown and foreign +// aliases both answer 404 (no ownership oracle). +func (c *Client) ResolveAlias(ctx context.Context, alias string) (*URLItem, error) { + base, err := url.Parse(c.base) + if err != nil { + return nil, err + } + path := "/api/v1/urls/" + url.PathEscape(base.Hostname()) + "/" + url.PathEscape(alias) + var out URLItem + if err := c.do(ctx, http.MethodGet, path, nil, nil, &out); err != nil { + return nil, err + } + return &out, nil +} + // UpdatedURL mirrors UpdateUrlResponse — unlike the shorten response it // carries no short_url, and timestamps are Unix seconds. type UpdatedURL struct { diff --git a/internal/api/urls_test.go b/internal/api/urls_test.go index 196618d..de48327 100644 --- a/internal/api/urls_test.go +++ b/internal/api/urls_test.go @@ -37,6 +37,43 @@ func TestListURLsBuildsQueryAndFilter(t *testing.T) { } } +func TestResolveAliasUsesAPIHostAndEscapes(t *testing.T) { + var gotPath string + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + gotPath = r.URL.EscapedPath() + w.Write([]byte(`{"id":"65f0abc123","alias":"🚀","long_url":"https://x.com","status":"ACTIVE"}`)) + })) + defer srv.Close() + + c := New(srv.URL, newTestStore(t, nil)) + u, err := c.ResolveAlias(context.Background(), "🚀") + if err != nil { + t.Fatal(err) + } + // httptest serves on 127.0.0.1, and the emoji alias must arrive + // percent-encoded + if gotPath != "/api/v1/urls/127.0.0.1/%F0%9F%9A%80" { + t.Fatalf("path = %q", gotPath) + } + if u.ID != "65f0abc123" { + t.Fatalf("id = %q", u.ID) + } +} + +func TestResolveAliasNotFound(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusNotFound) + w.Write([]byte(`{"error":"URL not found","code":"not_found"}`)) + })) + defer srv.Close() + + c := New(srv.URL, newTestStore(t, nil)) + _, err := c.ResolveAlias(context.Background(), "nope") + if !IsNotFound(err) { + t.Fatalf("err = %v, want IsNotFound", err) + } +} + func TestUpdateURLSendsPatch(t *testing.T) { srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { if r.Method != http.MethodPatch || r.URL.Path != "/api/v1/urls/abc123" { diff --git a/internal/cmd/export.go b/internal/cmd/export.go index 6a66858..c4ba2b8 100644 --- a/internal/cmd/export.go +++ b/internal/cmd/export.go @@ -19,6 +19,9 @@ func newExportCmd() *cobra.Command { Short: "Export click analytics to a file", Long: `Export click analytics as json, csv, xlsx, or xml. +Requires login. Without a short code, exports across all your links; +with one, exports that link (it must be yours). + csv arrives as a ZIP archive with one CSV per dimension; xlsx is a workbook with one sheet per dimension.`, Example: ` spoo export --format xlsx @@ -36,20 +39,25 @@ workbook with one sheet per dimension.`, if err != nil { return err } + if _, err := d.store.Load(); errors.Is(err, auth.ErrNotLoggedIn) { + return fmt.Errorf("export requires login — run `spoo auth login`") + } q := api.StatsQuery{StartDate: from, EndDate: to} + var name string + var data []byte if len(args) == 1 { - q.ShortCode = args[0] - } - if _, err := d.store.Load(); errors.Is(err, auth.ErrNotLoggedIn) { - if q.ShortCode == "" { - return fmt.Errorf("not logged in — pass a short code for public stats, or run `spoo auth login`") + u, err := d.client.ResolveAlias(cmd.Context(), args[0]) + if api.IsNotFound(err) { + return fmt.Errorf("%s is not one of your links — export covers only links you own", args[0]) } - q.Scope = "anon" - } else { - q.Scope = "all" - } - name, data, err := d.client.Export(cmd.Context(), q, format) - if err != nil { + if err != nil { + return err + } + name, data, err = d.client.ExportLink(cmd.Context(), u.ID, q, format) + if err != nil { + return err + } + } else if name, data, err = d.client.Export(cmd.Context(), q, format); err != nil { return err } if output == "-" { diff --git a/internal/cmd/export_test.go b/internal/cmd/export_test.go new file mode 100644 index 0000000..1425d3f --- /dev/null +++ b/internal/cmd/export_test.go @@ -0,0 +1,96 @@ +package cmd + +import ( + "bytes" + "net/http" + "net/http/httptest" + "strings" + "testing" +) + +func TestExportRequiresLogin(t *testing.T) { + pointDepsAt(t, "http://unused.invalid") + root := NewRootCmd() + root.SetOut(new(bytes.Buffer)) + root.SetErr(new(bytes.Buffer)) + root.SetArgs([]string{"export", "launch"}) + if err := root.Execute(); err == nil || !strings.Contains(err.Error(), "requires login") { + t.Fatalf("err = %v, want a login requirement", err) + } +} + +func TestExportAccountWide(t *testing.T) { + var gotPath string + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + gotPath = r.URL.Path + if r.URL.Query().Has("scope") { + t.Errorf("scope param must not be sent: %v", r.URL.Query()) + } + w.Write([]byte(`{"export":"ok"}`)) + })) + defer srv.Close() + pointDepsAtLoggedIn(t, srv.URL) + + root := NewRootCmd() + var out bytes.Buffer + root.SetOut(&out) + root.SetErr(&out) + root.SetArgs([]string{"export", "-o", "-"}) + if err := root.Execute(); err != nil { + t.Fatal(err) + } + if gotPath != "/api/v1/export" { + t.Fatalf("path = %q, want /api/v1/export", gotPath) + } + if !strings.Contains(out.String(), `{"export":"ok"}`) { + t.Fatalf("stdout = %q", out.String()) + } +} + +// exporting one link resolves the alias first and uses the per-link +// export endpoint. +func TestExportOwnedLink(t *testing.T) { + var paths []string + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + paths = append(paths, r.URL.Path) + if strings.HasPrefix(r.URL.Path, "/api/v1/urls/") { + w.Write([]byte(`{"id":"65f0abc123","alias":"launch","long_url":"https://x.com","status":"ACTIVE"}`)) + return + } + w.Write([]byte(`{"export":"ok"}`)) + })) + defer srv.Close() + pointDepsAtLoggedIn(t, srv.URL) + + root := NewRootCmd() + var out bytes.Buffer + root.SetOut(&out) + root.SetErr(&out) + root.SetArgs([]string{"export", "launch", "-o", "-"}) + if err := root.Execute(); err != nil { + t.Fatal(err) + } + want := []string{"/api/v1/urls/127.0.0.1/launch", "/api/v1/export/links/65f0abc123"} + if len(paths) != 2 || paths[0] != want[0] || paths[1] != want[1] { + t.Fatalf("paths = %v, want %v", paths, want) + } +} + +// anonymous export died with the scope param; a foreign code has no +// export surface at all, so the error must say so instead of retrying. +func TestExportForeignCodeErrors(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusNotFound) + w.Write([]byte(`{"error":"URL not found","code":"not_found"}`)) + })) + defer srv.Close() + pointDepsAtLoggedIn(t, srv.URL) + + root := NewRootCmd() + root.SetOut(new(bytes.Buffer)) + root.SetErr(new(bytes.Buffer)) + root.SetArgs([]string{"export", "launch"}) + if err := root.Execute(); err == nil || !strings.Contains(err.Error(), "links you own") { + t.Fatalf("err = %v, want an ownership explanation", err) + } +} diff --git a/internal/cmd/stats.go b/internal/cmd/stats.go index df476cb..5d57a4a 100644 --- a/internal/cmd/stats.go +++ b/internal/cmd/stats.go @@ -1,6 +1,7 @@ package cmd import ( + "context" "encoding/json" "errors" "fmt" @@ -14,6 +15,28 @@ import ( "github.com/spoo-me/spoo-cli/internal/tui/stats" ) +// resolveTarget maps a short code and login state onto a stats surface. +// Logged in with a code, the alias resolves to an owned link's url id; +// a 404 means the link isn't yours (or doesn't exist), so it falls back +// to the public endpoint, which answers for anyone's public link. +func resolveTarget(ctx context.Context, client *api.Client, code string, loggedIn bool) (stats.Target, error) { + switch { + case code == "": + return stats.Target{Kind: stats.KindAccount}, nil + case loggedIn: + u, err := client.ResolveAlias(ctx, code) + if api.IsNotFound(err) { + return stats.Target{Kind: stats.KindPublicLink, Alias: code}, nil + } + if err != nil { + return stats.Target{}, err + } + return stats.Target{Kind: stats.KindOwnedLink, Alias: code, URLID: u.ID}, nil + default: + return stats.Target{Kind: stats.KindPublicLink, Alias: code}, nil + } +} + func newStatsCmd() *cobra.Command { var from, to, tz string var plain bool @@ -43,22 +66,24 @@ With a short code, shows that link — public stats work without login.`, if err != nil { return err } - var target string + var code string if len(args) == 1 { - target = args[0] + code = args[0] } - scope := "all" - if _, err := d.store.Load(); errors.Is(err, auth.ErrNotLoggedIn) { - if target == "" { - return fmt.Errorf("not logged in — pass a short code for public stats, or run `spoo auth login`") - } - scope = "anon" + _, loadErr := d.store.Load() + loggedIn := !errors.Is(loadErr, auth.ErrNotLoggedIn) + if !loggedIn && code == "" { + return fmt.Errorf("not logged in — pass a short code for public stats, or run `spoo auth login`") + } + target, err := resolveTarget(cmd.Context(), d.client, code, loggedIn) + if err != nil { + return err } asJSON, _ := cmd.Flags().GetBool("json") customRange := from != "" || to != "" if !asJSON && !plain && !customRange && stdoutIsTerminal(cmd) { - model := stats.New(d.client, target, scope, tz) + model := stats.New(d.client, target, loggedIn, tz) final, err := tea.NewProgram(model).Run() if err != nil { return err @@ -74,14 +99,23 @@ With a short code, shows that link — public stats work without login.`, if from == "" && to == "" { from = timeNow().UTC().AddDate(0, 0, -api.MaxRangeDays).Format(time.RFC3339) } - res, err := d.client.Stats(cmd.Context(), api.StatsQuery{ - Scope: scope, - ShortCode: target, + q := api.StatsQuery{ StartDate: from, EndDate: to, Timezone: tz, GroupBy: []string{"time", "browser", "os", "country", "referrer"}, - }) + } + var res *api.StatsResponse + switch target.Kind { + case stats.KindOwnedLink: + res, err = d.client.LinkStats(cmd.Context(), target.URLID, q) + case stats.KindPublicLink: + // no group_by here — the public endpoint returns every + // dimension in one response + res, err = d.client.PublicStats(cmd.Context(), code, from, to, tz) + default: + res, err = d.client.Stats(cmd.Context(), q) + } if err != nil { return err } @@ -90,7 +124,7 @@ With a short code, shows that link — public stats work without login.`, enc.SetIndent("", " ") return enc.Encode(res) } - fmt.Fprintln(prettyOut(cmd), renderStats(res, target)) + fmt.Fprintln(prettyOut(cmd), renderStats(res, code)) return nil }, } diff --git a/internal/cmd/stats_test.go b/internal/cmd/stats_test.go index 0884040..535b457 100644 --- a/internal/cmd/stats_test.go +++ b/internal/cmd/stats_test.go @@ -14,6 +14,8 @@ import ( "github.com/spoo-me/spoo-cli/internal/config" ) +// statsBody is the standard stats wire; the stray "scope" key proves +// the decoder tolerates it (the response wire still carries one). const statsBody = `{ "scope": "anon", "summary": {"total_clicks": 100, "unique_clicks": 60, "first_click": "2026-05-01T10:00:00Z", "last_click": "2026-06-01T10:00:00Z", "avg_redirection_time": 0.12}, @@ -29,6 +31,25 @@ const statsBody = `{ } }` +const publicStatsBody = `{"generation": "v2", "link": {"alias": "launch"}, "stats": ` + statsBody + `}` + +// pointDepsAtLoggedIn is pointDepsAt with stored credentials, so the +// command under test sees a logged-in session. +func pointDepsAtLoggedIn(t *testing.T, srvURL string) { + t.Helper() + keyring.MockInit() + _ = keyring.Delete("spoo-cli", "credentials") + store := auth.NewStore(t.TempDir()) + if err := store.Save(auth.Credentials{Mode: auth.ModeAPIKey, APIKey: "spoo_k"}); err != nil { + t.Fatal(err) + } + orig := newDeps + newDeps = func() (*deps, error) { + return &deps{client: api.New(srvURL, store), store: store, cfg: config.Config{APIBase: srvURL}}, nil + } + t.Cleanup(func() { newDeps = orig }) +} + func TestStatsAnonymousRequiresShortCode(t *testing.T) { pointDepsAt(t, "http://unused.invalid") root := NewRootCmd() @@ -40,12 +61,15 @@ func TestStatsAnonymousRequiresShortCode(t *testing.T) { } } -func TestStatsRendersChartsForAnonCode(t *testing.T) { - var gotScope, gotCode string +// anonymous + code goes straight to the public endpoint. +func TestStatsAnonymousCodeUsesPublicEndpoint(t *testing.T) { + var gotPath string srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - gotScope = r.URL.Query().Get("scope") - gotCode = r.URL.Query().Get("short_code") - w.Write([]byte(statsBody)) + gotPath = r.URL.Path + if q := r.URL.Query(); q.Has("scope") || q.Has("group_by") { + t.Errorf("public request must send no scope/group_by: %v", q) + } + w.Write([]byte(publicStatsBody)) })) defer srv.Close() pointDepsAt(t, srv.URL) @@ -58,8 +82,8 @@ func TestStatsRendersChartsForAnonCode(t *testing.T) { if err := root.Execute(); err != nil { t.Fatal(err) } - if gotScope != "anon" || gotCode != "launch" { - t.Fatalf("scope=%q code=%q, want anon/launch", gotScope, gotCode) + if gotPath != "/api/v1/public/stats/launch" { + t.Fatalf("path = %q, want the public stats endpoint", gotPath) } text := out.String() for _, want := range []string{"100 clicks", "60 unique", "Chrome", "Browsers", "Clicks over time"} { @@ -69,35 +93,87 @@ func TestStatsRendersChartsForAnonCode(t *testing.T) { } } -func TestStatsUsesAllScopeWhenLoggedIn(t *testing.T) { - var gotScope string +// logged in without a code reads the account surface — and never +// sends the removed scope param. +func TestStatsAccountWideWhenLoggedIn(t *testing.T) { + var gotPath string srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - gotScope = r.URL.Query().Get("scope") + gotPath = r.URL.Path + if r.URL.Query().Has("scope") { + t.Errorf("scope param must not be sent: %v", r.URL.Query()) + } w.Write([]byte(statsBody)) })) defer srv.Close() + pointDepsAtLoggedIn(t, srv.URL) - keyring.MockInit() - _ = keyring.Delete("spoo-cli", "credentials") - store := auth.NewStore(t.TempDir()) - if err := store.Save(auth.Credentials{Mode: auth.ModeAPIKey, APIKey: "spoo_k"}); err != nil { + root := NewRootCmd() + var out bytes.Buffer + root.SetOut(&out) + root.SetErr(&out) + root.SetArgs([]string{"stats"}) + if err := root.Execute(); err != nil { t.Fatal(err) } - orig := newDeps - newDeps = func() (*deps, error) { - return &deps{client: api.New(srv.URL, store), store: store, cfg: config.Config{APIBase: srv.URL}}, nil + if gotPath != "/api/v1/stats" { + t.Fatalf("path = %q, want /api/v1/stats", gotPath) } - t.Cleanup(func() { newDeps = orig }) +} + +// logged in + code resolves the alias to a url id, then reads the +// per-link endpoint. +func TestStatsResolvesOwnedLinkToPerLinkEndpoint(t *testing.T) { + var paths []string + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + paths = append(paths, r.URL.Path) + if strings.HasPrefix(r.URL.Path, "/api/v1/urls/") { + w.Write([]byte(`{"id":"65f0abc123","alias":"launch","long_url":"https://x.com","status":"ACTIVE"}`)) + return + } + w.Write([]byte(statsBody)) + })) + defer srv.Close() + pointDepsAtLoggedIn(t, srv.URL) root := NewRootCmd() var out bytes.Buffer root.SetOut(&out) root.SetErr(&out) - root.SetArgs([]string{"stats"}) + root.SetArgs([]string{"stats", "launch"}) + if err := root.Execute(); err != nil { + t.Fatal(err) + } + want := []string{"/api/v1/urls/127.0.0.1/launch", "/api/v1/stats/links/65f0abc123"} + if len(paths) != 2 || paths[0] != want[0] || paths[1] != want[1] { + t.Fatalf("paths = %v, want %v", paths, want) + } +} + +// a code that doesn't resolve may still be someone else's public link, +// so stats falls back to the public endpoint. +func TestStatsFallsBackToPublicForForeignCode(t *testing.T) { + var paths []string + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + paths = append(paths, r.URL.Path) + if strings.HasPrefix(r.URL.Path, "/api/v1/urls/") { + w.WriteHeader(http.StatusNotFound) + w.Write([]byte(`{"error":"URL not found","code":"not_found"}`)) + return + } + w.Write([]byte(publicStatsBody)) + })) + defer srv.Close() + pointDepsAtLoggedIn(t, srv.URL) + + root := NewRootCmd() + var out bytes.Buffer + root.SetOut(&out) + root.SetErr(&out) + root.SetArgs([]string{"stats", "launch"}) if err := root.Execute(); err != nil { t.Fatal(err) } - if gotScope != "all" { - t.Fatalf("scope = %q, want all", gotScope) + if len(paths) != 2 || paths[1] != "/api/v1/public/stats/launch" { + t.Fatalf("paths = %v, want a public-stats fallback", paths) } } diff --git a/internal/tui/links/links_test.go b/internal/tui/links/links_test.go index ed69add..93ed198 100644 --- a/internal/tui/links/links_test.go +++ b/internal/tui/links/links_test.go @@ -310,9 +310,11 @@ func TestDetailViewShowsFullFields(t *testing.T) { // each move only re-arms the debounce, and stale ticks are dropped. func TestStatsDebounceDropsStaleTicks(t *testing.T) { var statsCalls int + var statsPath string srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { if strings.HasPrefix(r.URL.Path, "/api/v1/stats") { statsCalls++ + statsPath = r.URL.Path } w.Write([]byte(`{"scope":"all","summary":{"total_clicks":1},"metrics":{}}`)) })) @@ -355,6 +357,9 @@ func TestStatsDebounceDropsStaleTicks(t *testing.T) { if !ok || sm.alias != "third" { t.Fatalf("fetched %+v, want stats for third (the rested row)", msg) } + if statsPath != "/api/v1/stats/links/id-third" { + t.Fatalf("path = %q, want the per-link endpoint", statsPath) + } } // cached rows schedule nothing — revisiting is free. diff --git a/internal/tui/links/update.go b/internal/tui/links/update.go index 29ee1b4..6517be7 100644 --- a/internal/tui/links/update.go +++ b/internal/tui/links/update.go @@ -64,7 +64,7 @@ func (m Model) Update(msg tea.Msg) (tea.Model, tea.Cmd) { if _, ok := m.stats[it.Alias]; ok { return m, nil } - return m, m.fetchStats(it.Alias) + return m, m.fetchStats(it) case statsMsg: m.stats[msg.alias] = statsEntry{res: msg.res, err: msg.err} @@ -318,14 +318,13 @@ func (m *Model) scheduleStats() tea.Cmd { }) } -func (m Model) fetchStats(alias string) tea.Cmd { +func (m Model) fetchStats(it *api.URLItem) tea.Cmd { client := m.client + id, alias := it.ID, it.Alias return func() tea.Msg { // the endpoint defaults to a 7-day window; ask for the maximum from := time.Now().UTC().AddDate(0, 0, -api.MaxRangeDays).Format(time.RFC3339) - res, err := client.Stats(context.Background(), api.StatsQuery{ - Scope: "all", - ShortCode: alias, + res, err := client.LinkStats(context.Background(), id, api.StatsQuery{ StartDate: from, GroupBy: []string{"time", "browser", "os", "country", "referrer"}, }) diff --git a/internal/tui/stats/data.go b/internal/tui/stats/data.go index e6845fe..90d338f 100644 --- a/internal/tui/stats/data.go +++ b/internal/tui/stats/data.go @@ -26,15 +26,15 @@ func (m Model) window() (start, end time.Time) { } // query builds the stats request for the current dashboard state. +// The public endpoint reads only the range and timezone from it — no +// group_by (it answers with every dimension at once) and no filters. func (m Model) query() api.StatsQuery { start, end := m.window() groupBy := []string{"time", "browser", "os", "country", "city", "referrer"} - if m.target == "" { + if m.target.Kind == KindAccount { groupBy = append(groupBy, "short_code") } q := api.StatsQuery{ - Scope: m.scope, - ShortCode: m.target, StartDate: start.Format(time.RFC3339), Timezone: m.tz, GroupBy: groupBy, @@ -49,10 +49,21 @@ func (m Model) query() api.StatsQuery { return q } +// getStats routes a query to the target's endpoint. +func (m Model) getStats(ctx context.Context, q api.StatsQuery) (*api.StatsResponse, error) { + switch m.target.Kind { + case KindOwnedLink: + return m.client.LinkStats(ctx, m.target.URLID, q) + case KindPublicLink: + return m.client.PublicStats(ctx, m.target.Alias, q.StartDate, q.EndDate, q.Timezone) + default: + return m.client.Stats(ctx, q) + } +} + // fetch loads the current window and, for the overview deltas, the // window before it (summary only) — one command, one message. func (m Model) fetch() tea.Cmd { - client := m.client q := m.query() prevQ := q prevQ.GroupBy = []string{"time"} @@ -61,20 +72,25 @@ func (m Model) fetch() tea.Cmd { prevQ.EndDate = start.Format(time.RFC3339) return func() tea.Msg { - res, err := client.Stats(context.Background(), q) + res, err := m.getStats(context.Background(), q) var prev *api.StatsResponse if err == nil { - prev, _ = client.Stats(context.Background(), prevQ) // best-effort + prev, _ = m.getStats(context.Background(), prevQ) // best-effort } return statsLoadedMsg{res: res, prev: prev, err: err} } } // openExport pops the export dialog with a dated default filename. +// Export is an owner surface — the public view has nothing to offer. func (m Model) openExport() (tea.Model, tea.Cmd) { + if m.target.Kind == KindPublicLink { + m.status = ui.Dim.Render("export needs a login — run `spoo auth login`") + return m, nil + } subject := "stats-all" - if m.target != "" { - subject = "stats-" + m.target + if m.target.Alias != "" { + subject = "stats-" + m.target.Alias } var cmd tea.Cmd m.exportBox, cmd = m.exportBox.show(defaultExportName(subject, time.Now().Format("2006-01-02"))) @@ -85,9 +101,16 @@ func (m Model) openExport() (tea.Model, tea.Cmd) { // writes it where the dialog pointed. func (m Model) export(req exportRequest) tea.Cmd { client := m.client + target := m.target q := m.query() return func() tea.Msg { - _, data, err := client.Export(context.Background(), q, req.format) + var data []byte + var err error + if target.Kind == KindOwnedLink { + _, data, err = client.ExportLink(context.Background(), target.URLID, q, req.format) + } else { + _, data, err = client.Export(context.Background(), q, req.format) + } if err == nil { err = os.WriteFile(req.path, data, 0o644) } diff --git a/internal/tui/stats/model.go b/internal/tui/stats/model.go index 328560f..034a7ba 100644 --- a/internal/tui/stats/model.go +++ b/internal/tui/stats/model.go @@ -26,6 +26,24 @@ var defaultWindow = timeWindow{span: api.MaxRangeDays * 24 * time.Hour, label: " type panelDef struct{ key, title string } +// TargetKind says which stats surface the dashboard reads from. +type TargetKind int + +const ( + KindAccount TargetKind = iota // GET /v1/stats — all your links + KindOwnedLink // GET /v1/stats/links/{id} — one link you own + KindPublicLink // GET /v1/public/stats/{code} — no auth, fixed dimensions +) + +// Target is what the dashboard is pointed at. Alias is empty for the +// account view; URLID is set only for owned links (resolved by the +// caller, or picked with its id from the link switcher). +type Target struct { + Kind TargetKind + Alias string + URLID string +} + type statsLoadedMsg struct { res *api.StatsResponse prev *api.StatsResponse // previous window, for period-over-period deltas @@ -48,10 +66,10 @@ type filterEntry struct { // period deltas, a dual-series time chart, focusable breakdown panels // with server-side drill-down, window paging, and a focus mode. type Model struct { - client *api.Client - target string // short code, or "" for account-wide - scope string // all | anon - tz string + client *api.Client + target Target + loggedIn bool // gates the link switcher and export + tz string win timeWindow offset int // how many windows back in time ('[' / ']') @@ -89,7 +107,7 @@ type Model struct { height int } -func New(client *api.Client, target, scope, tz string) Model { +func New(client *api.Client, target Target, loggedIn bool, tz string) Model { rangeBox := textinput.New() rangeBox.Placeholder = "type a range…" rangeBox.SetWidth(36) // fits "2026-01-01 to 2026-02-15" with room; keeps the cheat-sheet column still @@ -99,7 +117,7 @@ func New(client *api.Client, target, scope, tz string) Model { return Model{ client: client, target: target, - scope: scope, + loggedIn: loggedIn, tz: tz, win: defaultWindow, rangeBox: rangeBox, @@ -119,7 +137,7 @@ func New(client *api.Client, target, scope, tz string) Model { // wide gets the drillable top-links leaderboard first; a single link // gets the weekday distribution instead. func (m Model) panels() []panelDef { - if m.target == "" { + if m.target.Kind == KindAccount { return []panelDef{ {"short_code", "top links"}, {"browser", "browsers"}, diff --git a/internal/tui/stats/panels_test.go b/internal/tui/stats/panels_test.go index 5685ee1..8864e8e 100644 --- a/internal/tui/stats/panels_test.go +++ b/internal/tui/stats/panels_test.go @@ -22,9 +22,8 @@ var ansiRe = regexp.MustCompile("\x1b\\[[0-9;]*m") func TestPanelRowsAligned(t *testing.T) { keyring.MockInit() client := api.New("http://x", auth.NewStore(t.TempDir())) - m := New(client, "", "all", "") + m := New(client, Target{}, true, "") resp := &api.StatsResponse{ - Scope: "all", Summary: api.StatsSummary{TotalClicks: 287558}, Metrics: map[string][]map[string]any{ "clicks_by_browser": { diff --git a/internal/tui/stats/stats_test.go b/internal/tui/stats/stats_test.go index 346ef2f..96b2458 100644 --- a/internal/tui/stats/stats_test.go +++ b/internal/tui/stats/stats_test.go @@ -18,7 +18,6 @@ import ( func testStatsResponse() *api.StatsResponse { return &api.StatsResponse{ - Scope: "all", Summary: api.StatsSummary{TotalClicks: 100, UniqueClicks: 40, AvgRedirectionTime: 88}, TimeRange: api.StatsTimeRange{ StartDate: "2026-03-12T00:00:00Z", EndDate: "2026-06-10T00:00:00Z", @@ -44,7 +43,7 @@ func newStatsModel(t *testing.T, srvURL string) Model { t.Helper() keyring.MockInit() client := api.New(srvURL, auth.NewStore(t.TempDir())) - m := New(client, "", "all", "") + m := New(client, Target{}, true, "") next, _ := m.Update(statsLoadedMsg{res: testStatsResponse()}) return next.(Model) } @@ -384,15 +383,16 @@ func TestMouseClickAndWheel(t *testing.T) { } } -// g opens the link picker; choosing a link re-targets the dashboard. +// g opens the link picker; choosing a link re-targets the dashboard +// at the per-link endpoint, using the id the list already carries. func TestLinkSwitcher(t *testing.T) { - var gotCode string + var gotPath string srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { if strings.Contains(r.URL.Path, "/urls") { - w.Write([]byte(`{"items":[{"alias":"launch","long_url":"https://a.com","total_clicks":60},{"alias":"promo","long_url":"https://b.com","total_clicks":40}],"page":1,"pageSize":100,"total":2}`)) + w.Write([]byte(`{"items":[{"id":"id-launch","alias":"launch","long_url":"https://a.com","total_clicks":60},{"id":"id-promo","alias":"promo","long_url":"https://b.com","total_clicks":40}],"page":1,"pageSize":100,"total":2}`)) return } - gotCode = r.URL.Query().Get("short_code") + gotPath = r.URL.Path w.Write([]byte(`{"scope":"all","summary":{"total_clicks":1},"metrics":{}}`)) })) defer srv.Close() @@ -417,22 +417,74 @@ func TestLinkSwitcher(t *testing.T) { } m, _ = statsKey(t, m, "down") m, cmd = statsKey(t, m, "enter") - if m.switchMode || m.target != "promo" || cmd == nil { - t.Fatalf("switch failed: mode=%v target=%q", m.switchMode, m.target) + want := Target{Kind: KindOwnedLink, Alias: "promo", URLID: "id-promo"} + if m.switchMode || m.target != want || cmd == nil { + t.Fatalf("switch failed: mode=%v target=%+v", m.switchMode, m.target) } cmd() - if gotCode != "promo" { - t.Fatalf("short_code param = %q, want promo", gotCode) + if gotPath != "/api/v1/stats/links/id-promo" { + t.Fatalf("path = %q, want the per-link endpoint", gotPath) } // esc just closes without switching m, _ = statsKey(t, m, "g") m, _ = statsKey(t, m, "esc") - if m.switchMode || m.target != "promo" { + if m.switchMode || m.target != want { t.Fatal("esc should close the picker and keep the target") } } +// a public-link view reads only the public endpoint — which takes no +// group_by and no filters — so drill-down, export, and the link +// switcher are all gated off. +func TestPublicViewIsReadOnly(t *testing.T) { + var gotPath string + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + gotPath = r.URL.Path + if q := r.URL.Query(); q.Has("scope") || q.Has("group_by") { + t.Errorf("public request must send no scope/group_by: %v", q) + } + w.Write([]byte(`{"generation":"v2","link":{"alias":"launch"},"stats":{"summary":{"total_clicks":5},"metrics":{"clicks_by_browser":[{"browser":"Chrome","clicks":5.0}]}}}`)) + })) + defer srv.Close() + + keyring.MockInit() + client := api.New(srv.URL, auth.NewStore(t.TempDir())) + m := New(client, Target{Kind: KindPublicLink, Alias: "launch"}, false, "") + for _, msg := range drainCmd(m.Init()) { + next, _ := m.Update(msg) + m = next.(Model) + } + if gotPath != "/api/v1/public/stats/launch" { + t.Fatalf("path = %q, want the public stats endpoint", gotPath) + } + if m.res == nil || m.res.Summary.TotalClicks != 5 { + t.Fatalf("envelope not unwrapped: %+v", m.res) + } + if !strings.Contains(m.View().Content, "(public)") { + t.Fatal("header should label the public view") + } + + // drill-down is a server-side filter the endpoint can't serve + m, _ = statsKey(t, m, "tab") // focus panel 1 (browsers) + m, cmd := statsKey(t, m, "enter") + if len(m.filters) != 0 || cmd != nil { + t.Fatalf("public drill must be a no-op: filters=%v cmd=%v", m.filters, cmd) + } + + // export is an owner surface now + m, _ = statsKey(t, m, "e") + if m.exportBox.open { + t.Fatal("public view must not open the export dialog") + } + + // switching links needs a login + m, _ = statsKey(t, m, "g") + if m.switchMode { + t.Fatal("public view must not open the link switcher") + } +} + // drainCmd runs a command (flattening batches) and returns its messages. func drainCmd(cmd tea.Cmd) []tea.Msg { if cmd == nil { @@ -532,6 +584,26 @@ func TestExportModal(t *testing.T) { } } +// an owned-link view exports through the per-link endpoint. +func TestExportRoutesOwnedLinkToPerLinkEndpoint(t *testing.T) { + var gotPath string + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + gotPath = r.URL.Path + w.Write([]byte(`{}`)) + })) + defer srv.Close() + + m := newStatsModel(t, srv.URL) + m.target = Target{Kind: KindOwnedLink, Alias: "launch", URLID: "id-launch"} + cmd := m.export(exportRequest{path: filepath.Join(t.TempDir(), "x.json"), format: "json"}) + if done, ok := cmd().(exportDoneMsg); !ok || done.err != nil { + t.Fatalf("export failed: %+v", done) + } + if gotPath != "/api/v1/export/links/id-launch" { + t.Fatalf("path = %q, want the per-link export endpoint", gotPath) + } +} + func mustGetwd(t *testing.T) string { t.Helper() wd, err := os.Getwd() diff --git a/internal/tui/stats/switcher.go b/internal/tui/stats/switcher.go index cf1473b..1dc91e4 100644 --- a/internal/tui/stats/switcher.go +++ b/internal/tui/stats/switcher.go @@ -25,7 +25,7 @@ type linksListMsg struct { // openSwitcher pops the picker, fetching the link list on first use. func (m Model) openSwitcher() (tea.Model, tea.Cmd) { - if m.scope == "anon" { + if !m.loggedIn { m.status = ui.Dim.Render("log in to switch links") return m, nil } @@ -81,12 +81,14 @@ func (m Model) updateSwitcher(msg tea.KeyPressMsg) (tea.Model, tea.Cmd) { return m, nil case "enter": cands := m.switchCandidates() - target := "" // row 0 is "all links" + target := Target{Kind: KindAccount} // row 0 is "all links" if m.switchSel > 0 { if m.switchSel-1 >= len(cands) { return m, nil } - target = cands[m.switchSel-1].Alias + it := cands[m.switchSel-1] + // picked from your own list, so the id is already in hand + target = Target{Kind: KindOwnedLink, Alias: it.Alias, URLID: it.ID} } m.switchMode = false m.switchBox.Blur() diff --git a/internal/tui/stats/update.go b/internal/tui/stats/update.go index 001849a..5c6f2b0 100644 --- a/internal/tui/stats/update.go +++ b/internal/tui/stats/update.go @@ -294,7 +294,12 @@ func (m Model) updateDashboard(msg tea.KeyPressMsg) (tea.Model, tea.Cmd) { } // drill adds a server-side filter for the selected row of panel idx. +// The public endpoint takes no filters, so the public view is read-only. func (m Model) drill(idx, topN int) (tea.Model, tea.Cmd) { + if m.target.Kind == KindPublicLink { + m.status = ui.Dim.Render("public stats — log in to drill down") + return m, nil + } dim := m.panels()[idx].key if dim == "weekday" { m.status = ui.Dim.Render("weekdays are computed locally — nothing to drill into") diff --git a/internal/tui/stats/view.go b/internal/tui/stats/view.go index 947e1b9..58bd72d 100644 --- a/internal/tui/stats/view.go +++ b/internal/tui/stats/view.go @@ -90,10 +90,13 @@ func (m Model) View() tea.View { func (m Model) headerLine() string { target := "all links" - if m.target != "" { - target = m.target + if m.target.Alias != "" { + target = m.target.Alias } h := ui.Title.Render("✦ spoo stats") + ui.Dim.Render(" · ") + target + if m.target.Kind == KindPublicLink { + h += ui.Dim.Render(" (public)") + } if m.res != nil && m.res.TimeRange.StartDate != "" { h += ui.Dim.Render(" · " + kit.ISODate(m.res.TimeRange.StartDate) + " → " + kit.ISODate(m.res.TimeRange.EndDate)) } else { From a6601d6c24a8249d7e257bff97d148c574d88ed5 Mon Sep 17 00:00:00 2001 From: zingzy Date: Sun, 16 Aug 2026 17:04:43 +0530 Subject: [PATCH 2/2] fix(stats): honor custom domains and stop misleading fallbacks Review follow-ups: stats and export gain --domain and thread it through ResolveAlias; a resolve 404 with --domain errors instead of silently rendering a stranger's default-domain stats, and the convenience fallback without --domain is announced on stderr. Password-protected 401s no longer trigger a pointless token refresh or a fake session expiry, and the public-view notices now speak to ownership. --- internal/api/client.go | 24 +++++++---- internal/api/client_test.go | 37 +++++++++++++++++ internal/api/urls.go | 20 +++++---- internal/api/urls_test.go | 23 ++++++++++- internal/cmd/export.go | 12 ++++-- internal/cmd/export_test.go | 28 +++++++++++++ internal/cmd/helpers.go | 10 +++++ internal/cmd/stats.go | 29 +++++++++---- internal/cmd/stats_test.go | 79 ++++++++++++++++++++++++++++++++++-- internal/tui/stats/data.go | 2 +- internal/tui/stats/update.go | 2 +- 11 files changed, 234 insertions(+), 32 deletions(-) diff --git a/internal/api/client.go b/internal/api/client.go index 5f36382..dbfc05c 100644 --- a/internal/api/client.go +++ b/internal/api/client.go @@ -99,14 +99,24 @@ func (c *Client) request(ctx context.Context, method, path string, query url.Val if err != nil { return nil, err } - if resp.StatusCode == http.StatusUnauthorized && creds != nil && - creds.Mode == auth.ModeDevice && creds.RefreshToken != "" { - resp.Body.Close() - if creds, err = c.refreshTokens(ctx, creds); err != nil { - return nil, err + if resp.StatusCode == http.StatusUnauthorized { + // The public stats endpoint answers 401 for password-protected + // links. That is a property of the link, not of the session, so + // refreshing tokens can't help — and the CLI doesn't supply link + // passwords, so say so instead of blaming the login. + switch resp.Header.Get("X-Error-Code") { + case "password_required", "invalid_password": + resp.Body.Close() + return nil, errors.New("this link's stats are password protected") } - if resp, err = c.send(ctx, method, path, query, body, creds); err != nil { - return nil, err + if creds != nil && creds.Mode == auth.ModeDevice && creds.RefreshToken != "" { + resp.Body.Close() + if creds, err = c.refreshTokens(ctx, creds); err != nil { + return nil, err + } + if resp, err = c.send(ctx, method, path, query, body, creds); err != nil { + return nil, err + } } } return resp, nil diff --git a/internal/api/client_test.go b/internal/api/client_test.go index 8d24a4a..885ad5c 100644 --- a/internal/api/client_test.go +++ b/internal/api/client_test.go @@ -5,6 +5,7 @@ import ( "errors" "net/http" "net/http/httptest" + "strings" "sync/atomic" "testing" @@ -173,3 +174,39 @@ func TestClientHeaderKeptOnSameHostRedirect(t *testing.T) { t.Fatalf("X-Spoo-Client after same-host redirect = %q, want cli/dev", gotClient) } } + +// a password-protected link answers 401 too, but that's about the link, +// not the session — no token refresh, and an honest error instead of +// "session expired". +func TestDo401PasswordRequiredSkipsRefresh(t *testing.T) { + var refreshCalls atomic.Int32 + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path == "/auth/device/refresh" { + refreshCalls.Add(1) + w.Write([]byte(`{"access_token":"newAT","refresh_token":"newRT"}`)) + return + } + w.Header().Set("X-Error-Code", "password_required") + w.WriteHeader(http.StatusUnauthorized) + w.Write([]byte(`{"error":"Password required","code":"password_required"}`)) + })) + defer srv.Close() + + store := newTestStore(t, &auth.Credentials{Mode: auth.ModeDevice, AccessToken: "goodAT", RefreshToken: "goodRT"}) + c := New(srv.URL, store) + err := c.do(context.Background(), http.MethodGet, "/api/v1/public/stats/secret", nil, nil, nil) + if err == nil || !strings.Contains(err.Error(), "password protected") { + t.Fatalf("err = %v, want a password-protected explanation", err) + } + if refreshCalls.Load() != 0 { + t.Fatalf("refresh called %d times, want 0", refreshCalls.Load()) + } + // the healthy session must survive untouched + got, loadErr := store.Load() + if loadErr != nil { + t.Fatal(loadErr) + } + if got.AccessToken != "goodAT" || got.RefreshToken != "goodRT" { + t.Fatalf("tokens rotated pointlessly: %+v", got) + } +} diff --git a/internal/api/urls.go b/internal/api/urls.go index 2d6f36f..3801d6d 100644 --- a/internal/api/urls.go +++ b/internal/api/urls.go @@ -85,15 +85,19 @@ func (c *Client) ListURLs(ctx context.Context, opts ListURLsOptions) (*URLPage, // ResolveAlias looks up an owned link by alias via GET // /api/v1/urls/{domain}/{alias}, mainly to obtain its url id for the -// per-link stats and export endpoints. The domain is the API base -// URL's hostname, which covers spoo.me links. Unknown and foreign -// aliases both answer 404 (no ownership oracle). -func (c *Client) ResolveAlias(ctx context.Context, alias string) (*URLItem, error) { - base, err := url.Parse(c.base) - if err != nil { - return nil, err +// per-link stats and export endpoints. An empty domain defaults to the +// API base URL's hostname, which covers spoo.me links; pass one of the +// user's custom domains to resolve links living there. Unknown and +// foreign aliases both answer 404 (no ownership oracle). +func (c *Client) ResolveAlias(ctx context.Context, alias, domain string) (*URLItem, error) { + if domain == "" { + base, err := url.Parse(c.base) + if err != nil { + return nil, err + } + domain = base.Hostname() } - path := "/api/v1/urls/" + url.PathEscape(base.Hostname()) + "/" + url.PathEscape(alias) + path := "/api/v1/urls/" + url.PathEscape(domain) + "/" + url.PathEscape(alias) var out URLItem if err := c.do(ctx, http.MethodGet, path, nil, nil, &out); err != nil { return nil, err diff --git a/internal/api/urls_test.go b/internal/api/urls_test.go index de48327..bb0f9a0 100644 --- a/internal/api/urls_test.go +++ b/internal/api/urls_test.go @@ -46,7 +46,7 @@ func TestResolveAliasUsesAPIHostAndEscapes(t *testing.T) { defer srv.Close() c := New(srv.URL, newTestStore(t, nil)) - u, err := c.ResolveAlias(context.Background(), "🚀") + u, err := c.ResolveAlias(context.Background(), "🚀", "") if err != nil { t.Fatal(err) } @@ -68,12 +68,31 @@ func TestResolveAliasNotFound(t *testing.T) { defer srv.Close() c := New(srv.URL, newTestStore(t, nil)) - _, err := c.ResolveAlias(context.Background(), "nope") + _, err := c.ResolveAlias(context.Background(), "nope", "") if !IsNotFound(err) { t.Fatalf("err = %v, want IsNotFound", err) } } +// a custom domain replaces the API host in the resolve path, so links +// on the user's own domains resolve to their real url ids. +func TestResolveAliasCustomDomain(t *testing.T) { + var gotPath string + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + gotPath = r.URL.EscapedPath() + w.Write([]byte(`{"id":"65f0abc123","alias":"promo","long_url":"https://x.com","status":"ACTIVE"}`)) + })) + defer srv.Close() + + c := New(srv.URL, newTestStore(t, nil)) + if _, err := c.ResolveAlias(context.Background(), "promo", "links.example.com"); err != nil { + t.Fatal(err) + } + if gotPath != "/api/v1/urls/links.example.com/promo" { + t.Fatalf("path = %q, want the custom domain in the path", gotPath) + } +} + func TestUpdateURLSendsPatch(t *testing.T) { srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { if r.Method != http.MethodPatch || r.URL.Path != "/api/v1/urls/abc123" { diff --git a/internal/cmd/export.go b/internal/cmd/export.go index c4ba2b8..8b2914a 100644 --- a/internal/cmd/export.go +++ b/internal/cmd/export.go @@ -13,7 +13,7 @@ import ( ) func newExportCmd() *cobra.Command { - var format, output, from, to string + var format, output, from, to, domain string cmd := &cobra.Command{ Use: "export [short-code]", Short: "Export click analytics to a file", @@ -46,9 +46,13 @@ workbook with one sheet per dimension.`, var name string var data []byte if len(args) == 1 { - u, err := d.client.ResolveAlias(cmd.Context(), args[0]) + u, err := d.client.ResolveAlias(cmd.Context(), args[0], domain) if api.IsNotFound(err) { - return fmt.Errorf("%s is not one of your links — export covers only links you own", args[0]) + where := args[0] + if domain != "" { + where += " on " + domain + } + return fmt.Errorf("%s is not one of your links — export covers only links you own", where) } if err != nil { return err @@ -79,6 +83,8 @@ workbook with one sheet per dimension.`, cmd.Flags().StringVarP(&output, "output", "o", "", "output file (default: server-suggested name; - for stdout)") cmd.Flags().StringVar(&from, "from", "", "start date (ISO 8601)") cmd.Flags().StringVar(&to, "to", "", "end date (ISO 8601)") + cmd.Flags().StringVar(&domain, "domain", "", "the link is on one of your custom domains") fixed(cmd, "format", "json", "csv", "xlsx", "xml") + flagComp(cmd, "domain", completeDomain) return cmd } diff --git a/internal/cmd/export_test.go b/internal/cmd/export_test.go index 1425d3f..b704e5d 100644 --- a/internal/cmd/export_test.go +++ b/internal/cmd/export_test.go @@ -94,3 +94,31 @@ func TestExportForeignCodeErrors(t *testing.T) { t.Fatalf("err = %v, want an ownership explanation", err) } } + +// --domain must reach the resolve path, replacing the API host. +func TestExportDomainFlagResolvesOnThatDomain(t *testing.T) { + var paths []string + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + paths = append(paths, r.URL.Path) + if strings.HasPrefix(r.URL.Path, "/api/v1/urls/") { + w.Write([]byte(`{"id":"65f0abc123","alias":"promo","long_url":"https://x.com","status":"ACTIVE"}`)) + return + } + w.Write([]byte(`{"export":"ok"}`)) + })) + defer srv.Close() + pointDepsAtLoggedIn(t, srv.URL) + + root := NewRootCmd() + var out bytes.Buffer + root.SetOut(&out) + root.SetErr(&out) + root.SetArgs([]string{"export", "promo", "--domain", "links.example.com", "-o", "-"}) + if err := root.Execute(); err != nil { + t.Fatal(err) + } + want := []string{"/api/v1/urls/links.example.com/promo", "/api/v1/export/links/65f0abc123"} + if len(paths) != 2 || paths[0] != want[0] || paths[1] != want[1] { + t.Fatalf("paths = %v, want %v", paths, want) + } +} diff --git a/internal/cmd/helpers.go b/internal/cmd/helpers.go index c501ee6..da252f6 100644 --- a/internal/cmd/helpers.go +++ b/internal/cmd/helpers.go @@ -2,6 +2,7 @@ package cmd import ( "io" + "net/url" "strings" "time" @@ -9,6 +10,15 @@ import ( "github.com/spf13/cobra" ) +// apiHost is the hostname of the configured API base — the system +// default domain that short links live on unless --domain says otherwise. +func apiHost(base string) string { + if u, err := url.Parse(base); err == nil && u.Hostname() != "" { + return u.Hostname() + } + return base +} + // timeNow is a seam for tests that need deterministic expiry math. var timeNow = time.Now diff --git a/internal/cmd/stats.go b/internal/cmd/stats.go index 5d57a4a..c2edbae 100644 --- a/internal/cmd/stats.go +++ b/internal/cmd/stats.go @@ -5,6 +5,7 @@ import ( "encoding/json" "errors" "fmt" + "io" "time" tea "charm.land/bubbletea/v2" @@ -16,16 +17,24 @@ import ( ) // resolveTarget maps a short code and login state onto a stats surface. -// Logged in with a code, the alias resolves to an owned link's url id; -// a 404 means the link isn't yours (or doesn't exist), so it falls back -// to the public endpoint, which answers for anyone's public link. -func resolveTarget(ctx context.Context, client *api.Client, code string, loggedIn bool) (stats.Target, error) { +// Logged in with a code, the alias resolves to an owned link's url id on +// the given domain (empty means the system default). On a 404 with +// --domain set there is nowhere to fall back to — the public endpoint +// serves only default-domain links — so it errors instead of silently +// showing a different link's stats. Without --domain the code may still +// be someone else's public default-domain link, so it falls back to the +// public endpoint, announcing the switch on errOut. +func resolveTarget(ctx context.Context, client *api.Client, code, domain, defaultDomain string, loggedIn bool, errOut io.Writer) (stats.Target, error) { switch { case code == "": return stats.Target{Kind: stats.KindAccount}, nil case loggedIn: - u, err := client.ResolveAlias(ctx, code) + u, err := client.ResolveAlias(ctx, code, domain) if api.IsNotFound(err) { + if domain != "" { + return stats.Target{}, fmt.Errorf("%s on %s is not one of your links — public stats cover only %s links", code, domain, defaultDomain) + } + fmt.Fprintf(errOut, "note: %s isn't one of your links — showing public stats for %s/%s\n", code, defaultDomain, code) return stats.Target{Kind: stats.KindPublicLink, Alias: code}, nil } if err != nil { @@ -38,7 +47,7 @@ func resolveTarget(ctx context.Context, client *api.Client, code string, loggedI } func newStatsCmd() *cobra.Command { - var from, to, tz string + var from, to, tz, domain string var plain bool cmd := &cobra.Command{ Use: "stats [short-code]", @@ -75,7 +84,11 @@ With a short code, shows that link — public stats work without login.`, if !loggedIn && code == "" { return fmt.Errorf("not logged in — pass a short code for public stats, or run `spoo auth login`") } - target, err := resolveTarget(cmd.Context(), d.client, code, loggedIn) + defaultDomain := apiHost(d.cfg.APIBase) + if domain != "" && !loggedIn { + return fmt.Errorf("--domain requires login — public stats cover only %s links", defaultDomain) + } + target, err := resolveTarget(cmd.Context(), d.client, code, domain, defaultDomain, loggedIn, cmd.ErrOrStderr()) if err != nil { return err } @@ -132,5 +145,7 @@ With a short code, shows that link — public stats work without login.`, cmd.Flags().StringVar(&to, "to", "", "end date, ISO 8601 (static report; default: now)") cmd.Flags().StringVar(&tz, "tz", "", "IANA timezone for time buckets (default UTC)") cmd.Flags().BoolVar(&plain, "plain", false, "print the static report instead of the dashboard") + cmd.Flags().StringVar(&domain, "domain", "", "the link is on one of your custom domains") + flagComp(cmd, "domain", completeDomain) return cmd } diff --git a/internal/cmd/stats_test.go b/internal/cmd/stats_test.go index 535b457..813401f 100644 --- a/internal/cmd/stats_test.go +++ b/internal/cmd/stats_test.go @@ -150,7 +150,8 @@ func TestStatsResolvesOwnedLinkToPerLinkEndpoint(t *testing.T) { } // a code that doesn't resolve may still be someone else's public link, -// so stats falls back to the public endpoint. +// so stats falls back to the public endpoint — but says so on stderr +// instead of silently switching surfaces. func TestStatsFallsBackToPublicForForeignCode(t *testing.T) { var paths []string srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { @@ -166,9 +167,9 @@ func TestStatsFallsBackToPublicForForeignCode(t *testing.T) { pointDepsAtLoggedIn(t, srv.URL) root := NewRootCmd() - var out bytes.Buffer + var out, errOut bytes.Buffer root.SetOut(&out) - root.SetErr(&out) + root.SetErr(&errOut) root.SetArgs([]string{"stats", "launch"}) if err := root.Execute(); err != nil { t.Fatal(err) @@ -176,4 +177,76 @@ func TestStatsFallsBackToPublicForForeignCode(t *testing.T) { if len(paths) != 2 || paths[1] != "/api/v1/public/stats/launch" { t.Fatalf("paths = %v, want a public-stats fallback", paths) } + notice := errOut.String() + if !strings.Contains(notice, "isn't one of your links") || + !strings.Contains(notice, "public stats for 127.0.0.1/launch") { + t.Fatalf("stderr = %q, want an announced fallback", notice) + } +} + +// --domain must reach the resolve path, replacing the API host. +func TestStatsDomainFlagResolvesOnThatDomain(t *testing.T) { + var paths []string + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + paths = append(paths, r.URL.Path) + if strings.HasPrefix(r.URL.Path, "/api/v1/urls/") { + w.Write([]byte(`{"id":"65f0abc123","alias":"promo","long_url":"https://x.com","status":"ACTIVE"}`)) + return + } + w.Write([]byte(statsBody)) + })) + defer srv.Close() + pointDepsAtLoggedIn(t, srv.URL) + + root := NewRootCmd() + var out bytes.Buffer + root.SetOut(&out) + root.SetErr(&out) + root.SetArgs([]string{"stats", "promo", "--domain", "links.example.com"}) + if err := root.Execute(); err != nil { + t.Fatal(err) + } + want := []string{"/api/v1/urls/links.example.com/promo", "/api/v1/stats/links/65f0abc123"} + if len(paths) != 2 || paths[0] != want[0] || paths[1] != want[1] { + t.Fatalf("paths = %v, want %v", paths, want) + } +} + +// with --domain there is no public surface to fall back to (the public +// endpoint serves only the default domain), so a 404 must be an error — +// never a stranger's default-domain stats under the same alias. +func TestStatsDomainNotFoundErrorsInsteadOfFallingBack(t *testing.T) { + var paths []string + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + paths = append(paths, r.URL.Path) + w.WriteHeader(http.StatusNotFound) + w.Write([]byte(`{"error":"URL not found","code":"not_found"}`)) + })) + defer srv.Close() + pointDepsAtLoggedIn(t, srv.URL) + + root := NewRootCmd() + root.SetOut(new(bytes.Buffer)) + root.SetErr(new(bytes.Buffer)) + root.SetArgs([]string{"stats", "promo", "--domain", "links.example.com"}) + err := root.Execute() + if err == nil || !strings.Contains(err.Error(), "not one of your links") { + t.Fatalf("err = %v, want an ownership explanation", err) + } + if len(paths) != 1 || paths[0] != "/api/v1/urls/links.example.com/promo" { + t.Fatalf("paths = %v, want the resolve call only (no public fallback)", paths) + } +} + +// anonymous + --domain has no surface at all: resolution needs login and +// the public endpoint serves only the default domain. +func TestStatsDomainRequiresLogin(t *testing.T) { + pointDepsAt(t, "http://unused.invalid") + root := NewRootCmd() + root.SetOut(new(bytes.Buffer)) + root.SetErr(new(bytes.Buffer)) + root.SetArgs([]string{"stats", "promo", "--domain", "links.example.com"}) + if err := root.Execute(); err == nil || !strings.Contains(err.Error(), "--domain requires login") { + t.Fatalf("err = %v, want a login requirement", err) + } } diff --git a/internal/tui/stats/data.go b/internal/tui/stats/data.go index 90d338f..8a34231 100644 --- a/internal/tui/stats/data.go +++ b/internal/tui/stats/data.go @@ -85,7 +85,7 @@ func (m Model) fetch() tea.Cmd { // Export is an owner surface — the public view has nothing to offer. func (m Model) openExport() (tea.Model, tea.Cmd) { if m.target.Kind == KindPublicLink { - m.status = ui.Dim.Render("export needs a login — run `spoo auth login`") + m.status = ui.Dim.Render("export is available only for links you own") return m, nil } subject := "stats-all" diff --git a/internal/tui/stats/update.go b/internal/tui/stats/update.go index 5c6f2b0..19ee485 100644 --- a/internal/tui/stats/update.go +++ b/internal/tui/stats/update.go @@ -297,7 +297,7 @@ func (m Model) updateDashboard(msg tea.KeyPressMsg) (tea.Model, tea.Cmd) { // The public endpoint takes no filters, so the public view is read-only. func (m Model) drill(idx, topN int) (tea.Model, tea.Cmd) { if m.target.Kind == KindPublicLink { - m.status = ui.Dim.Render("public stats — log in to drill down") + m.status = ui.Dim.Render("filters aren't available for public stats") return m, nil } dim := m.panels()[idx].key