From ec6ab0d91fab340ddbcd21b0d6ec2867c8c1da3e Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 24 Jun 2026 06:33:17 +0000 Subject: [PATCH] fix(rules): show counterparty/series name (not short_id) in rule detail, lift icon contrast MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The rule detail "Then" card rendered "Assign to counterparty wvmXi45D" instead of "Assign to counterparty Netflix" whenever an assign_counterparty / assign_series action binds an existing entity by short_id (no inline name stored). ruleActionEntityName fell straight back to the surrogate. Resolve the short_id to the live entity's display name in the rule detail handler (cheap — a rule carries a handful of actions) and pass it through RuleDetailProps.ActionEntityNames; the templ helper now prefers inline name → resolved name → short_id as a last resort. Also fix the near-invisible counterparty icon: the tile used bg-secondary/10 + text-secondary, and --color-secondary is ~white in light mode (and ~dark in dark mode), so the store glyph washed out in both themes. Switch to the chromatic bg-info/10 + text-info, matching the visibility of the assign_series (primary) tile. Fixes #1916 Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01FBR5SK8z4L71JAHcA3DdAH --- internal/admin/rules.go | 35 ++++++++++++++- .../components/pages/rule_detail.templ | 12 +++--- .../components/pages/rule_detail_types.go | 28 +++++++++--- .../pages/rule_detail_types_test.go | 43 +++++++++++++++++++ 4 files changed, 106 insertions(+), 12 deletions(-) diff --git a/internal/admin/rules.go b/internal/admin/rules.go index 9b347a65..ca962b15 100644 --- a/internal/admin/rules.go +++ b/internal/admin/rules.go @@ -6,6 +6,7 @@ import ( "errors" "net/http" "strconv" + "strings" "time" "breadbox/internal/service" @@ -312,8 +313,38 @@ func RuleDetailPageHandler(svc *service.Service, sm *scs.SessionManager, tr *Tem } } + // Resolve display names for assign_series / assign_counterparty actions + // that bind by short_id only (no inline name). Without this the "Then" + // card shows the opaque surrogate ("wvmXi45D") instead of the entity's + // name ("Netflix"). Keyed by short_id; a failed lookup just leaves the + // short_id fallback in place. A rule carries at most a handful of + // actions, so these per-action lookups are cheap. + actionEntityNames := make(map[string]string) + for _, a := range rule.Actions { + switch a.Type { + case "assign_series": + if strings.TrimSpace(a.SeriesName) != "" { + continue + } + if sid := strings.TrimSpace(a.SeriesShortID); sid != "" { + if sr, err := svc.GetSeries(ctx, sid); err == nil { + actionEntityNames[sid] = sr.Name + } + } + case "assign_counterparty": + if strings.TrimSpace(a.CounterpartyName) != "" { + continue + } + if sid := strings.TrimSpace(a.CounterpartyShortID); sid != "" { + if cp, err := svc.GetCounterparty(ctx, sid); err == nil { + actionEntityNames[sid] = cp.Name + } + } + } + } + data := BaseTemplateData(r, sm, "rules", rule.Name) - renderRuleDetail(w, r, tr, data, rule, preview, stats, applications, applicationTxns, applicationMeta, previewTxns, hasMoreApps, syncHistory, actionCategoryName, categories) + renderRuleDetail(w, r, tr, data, rule, preview, stats, applications, applicationTxns, applicationMeta, previewTxns, hasMoreApps, syncHistory, actionCategoryName, actionEntityNames, categories) } } @@ -337,6 +368,7 @@ func renderRuleDetail( hasMoreApps bool, syncHistory []map[string]any, actionCategoryName string, + actionEntityNames map[string]string, categories []service.CategoryResponse, ) { data["Breadcrumbs"] = []components.Breadcrumb{ @@ -354,6 +386,7 @@ func renderRuleDetail( HasMoreApplications: hasMoreApps, SyncHistory: syncHistory, ActionCategoryName: actionCategoryName, + ActionEntityNames: actionEntityNames, Categories: categories, ConditionSummary: service.ConditionSummary(rule.Conditions), TriggerLabel: service.TriggerLabel(rule.Trigger), diff --git a/internal/templates/components/pages/rule_detail.templ b/internal/templates/components/pages/rule_detail.templ index fae615f7..f2329592 100644 --- a/internal/templates/components/pages/rule_detail.templ +++ b/internal/templates/components/pages/rule_detail.templ @@ -235,20 +235,20 @@ templ ruleDetailActionRow(p RuleDetailProps, a service.RuleAction) { @components.LucideIcon("repeat", "w-4 h-4 text-primary")
- if ruleActionEntityName(a) != "" { -

Assign to series { ruleActionEntityName(a) }

+ if ruleActionEntityName(p, a) != "" { +

Assign to series { ruleActionEntityName(p, a) }

} else {

Assign to series

}

Matching transactions are linked to the series during sync.

case "assign_counterparty": -
- @components.LucideIcon("store", "w-4 h-4 text-secondary") +
+ @components.LucideIcon("store", "w-4 h-4 text-info")
- if ruleActionEntityName(a) != "" { -

Assign to counterparty { ruleActionEntityName(a) }

+ if ruleActionEntityName(p, a) != "" { +

Assign to counterparty { ruleActionEntityName(p, a) }

} else {

Assign to counterparty

} diff --git a/internal/templates/components/pages/rule_detail_types.go b/internal/templates/components/pages/rule_detail_types.go index f1083612..5b735217 100644 --- a/internal/templates/components/pages/rule_detail_types.go +++ b/internal/templates/components/pages/rule_detail_types.go @@ -37,6 +37,14 @@ type RuleDetailProps struct { ConditionRows []components.ConditionRowProps // TriggerLabel is the human label for Rule.Trigger (service.TriggerLabel). TriggerLabel string + // ActionEntityNames maps a short_id targeted by an assign_series / + // assign_counterparty action to the live entity's display name. The + // handler hydrates it so the "Then" card can show "Assign to + // counterparty Netflix" instead of the opaque surrogate "wvmXi45D" when + // the rule binds by short_id (no inline name stored). Keyed by short_id; + // empty when the action already carries an inline name or the lookup + // failed. + ActionEntityNames map[string]string } // RuleApplicationMeta carries the per-application action info shown above @@ -258,20 +266,30 @@ func syncHistoryHitCount(h map[string]any) int { // ruleActionEntityName returns the human display name for an assign_series / // assign_counterparty action — the by-name value when the rule mints/binds by -// name, otherwise the short ID it targets, otherwise "". Used by the rule -// detail "Then" card so surrogate-first actions read legibly. -func ruleActionEntityName(a service.RuleAction) string { +// name, else the resolved name for the short_id it targets (hydrated by the +// handler into p.ActionEntityNames), else the short_id as a last resort, +// otherwise "". Used by the rule detail "Then" card so surrogate-first actions +// read legibly ("Netflix" rather than the opaque "wvmXi45D"). +func ruleActionEntityName(p RuleDetailProps, a service.RuleAction) string { switch a.Type { case "assign_series": if n := strings.TrimSpace(a.SeriesName); n != "" { return n } - return strings.TrimSpace(a.SeriesShortID) + sid := strings.TrimSpace(a.SeriesShortID) + if n := strings.TrimSpace(p.ActionEntityNames[sid]); n != "" { + return n + } + return sid case "assign_counterparty": if n := strings.TrimSpace(a.CounterpartyName); n != "" { return n } - return strings.TrimSpace(a.CounterpartyShortID) + sid := strings.TrimSpace(a.CounterpartyShortID) + if n := strings.TrimSpace(p.ActionEntityNames[sid]); n != "" { + return n + } + return sid } return "" } diff --git a/internal/templates/components/pages/rule_detail_types_test.go b/internal/templates/components/pages/rule_detail_types_test.go index 9cbeb719..e9d3fe5c 100644 --- a/internal/templates/components/pages/rule_detail_types_test.go +++ b/internal/templates/components/pages/rule_detail_types_test.go @@ -47,6 +47,49 @@ func TestRuleHasRetroactiveActionExcludesCommentOnly(t *testing.T) { } } +func TestRuleActionEntityNamePrefersInlineName(t *testing.T) { + // An action that carries an inline name renders it verbatim, regardless of + // any resolved-name map. + p := RuleDetailProps{ActionEntityNames: map[string]string{"wvmXi45D": "Resolved"}} + cp := service.RuleAction{Type: "assign_counterparty", CounterpartyName: "Netflix", CounterpartyShortID: "wvmXi45D"} + if got := ruleActionEntityName(p, cp); got != "Netflix" { + t.Errorf("ruleActionEntityName(counterparty inline) = %q, want Netflix", got) + } + sr := service.RuleAction{Type: "assign_series", SeriesName: "Hulu", SeriesShortID: "abcd1234"} + if got := ruleActionEntityName(p, sr); got != "Hulu" { + t.Errorf("ruleActionEntityName(series inline) = %q, want Hulu", got) + } +} + +func TestRuleActionEntityNameResolvesShortID(t *testing.T) { + // The bug from #1916: a rule that binds an existing counterparty by short_id + // stores no inline name, so the card used to render the opaque surrogate. + // With the handler-hydrated ActionEntityNames map, the short_id resolves to + // the entity's display name. + p := RuleDetailProps{ActionEntityNames: map[string]string{ + "wvmXi45D": "Netflix", + "abcd1234": "Spotify", + }} + cp := service.RuleAction{Type: "assign_counterparty", CounterpartyShortID: "wvmXi45D"} + if got := ruleActionEntityName(p, cp); got != "Netflix" { + t.Errorf("ruleActionEntityName(counterparty short_id) = %q, want Netflix", got) + } + sr := service.RuleAction{Type: "assign_series", SeriesShortID: "abcd1234"} + if got := ruleActionEntityName(p, sr); got != "Spotify" { + t.Errorf("ruleActionEntityName(series short_id) = %q, want Spotify", got) + } +} + +func TestRuleActionEntityNameFallsBackToShortID(t *testing.T) { + // When the lookup misses (entity deleted, resolution failed), the short_id + // is the last-resort display value rather than an empty label. + p := RuleDetailProps{ActionEntityNames: map[string]string{}} + cp := service.RuleAction{Type: "assign_counterparty", CounterpartyShortID: "wvmXi45D"} + if got := ruleActionEntityName(p, cp); got != "wvmXi45D" { + t.Errorf("ruleActionEntityName(unresolved counterparty) = %q, want wvmXi45D", got) + } +} + func TestRuleHasRetroactiveActionMixedRetroWins(t *testing.T) { // A rule that mixes a sync-only action with a materializing one is still // retroactive — the materializing action gets backfilled.