Skip to content
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 14 additions & 5 deletions app/controllers/insights_controller.rb
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
class InsightsController < ApplicationController
before_action :set_insight, only: %i[dismiss undismiss]
before_action :set_insight, only: %i[acknowledge unacknowledge]

def index
load_feed
Expand All @@ -14,18 +14,20 @@ def index
end
end

def dismiss
@insight.dismiss!
def acknowledge
@insight.acknowledge!
load_widget_feed

respond_to do |format|
format.turbo_stream
format.html { redirect_back_or_to insights_path }
end
end

def undismiss
@insight.undismiss!
def unacknowledge
@insight.unacknowledge!
load_feed
load_widget_feed

respond_to do |format|
format.turbo_stream
Expand Down Expand Up @@ -54,6 +56,13 @@ def load_feed
@unread_ids = @insights.select(&:active?).map(&:id).to_set
end

# Acknowledging is reachable from the dashboard widget as well as this page,
# so the response re-renders the widget's top three. Removing a row there
# should promote the next insight into the freed slot, not leave a gap.
def load_widget_feed
@feed_insights = Current.family.insights.visible.ordered.limit(Insight::FEED_LIMIT).to_a
end

# Turbo sends X-Sec-Purpose (the fetch spec forbids setting Sec-Purpose
# from JS) on hover-prefetch requests.
def prefetch_request?
Expand Down
2 changes: 1 addition & 1 deletion app/controllers/pages_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ def dashboard

@cashflow_sankey_data = build_cashflow_sankey_data(net_totals, income_totals, expense_totals, family_currency)
@outflows_data = build_outflows_donut_data(net_totals)
@feed_insights = Current.family.insights.visible.ordered.limit(3)
@feed_insights = Current.family.insights.visible.ordered.limit(Insight::FEED_LIMIT)

@money_flow_accounts = income_statement.eligible_accounts
@money_flow_month = money_flow_month_param
Expand Down
9 changes: 8 additions & 1 deletion app/helpers/insights_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,14 @@ def insight_key_figure(insight)
facts["amount"] && [ facts["amount"], t("insights.figures.days_overdue", count: facts["days_overdue"].to_i) ]
when "idle_cash"
facts["balance"] && [ facts["balance"], t("insights.figures.idle_days", count: facts["idle_days"].to_i) ]
when "budget_at_risk", "budget_on_track"
when "budget_at_risk"
# Not budget_spent_pct: this card's headline is "N categories need
# attention", and total consumption ("14% of budget") reads as reassurance
# next to it — a focal figure arguing against its own card. The flagged
# count is what the card is actually about.
facts["count"] && [ facts["count"].to_s, t("insights.figures.need_attention") ]
when "budget_on_track"
# Still the right figure here, where overall usage *is* the subject.
facts["budget_spent_pct"] && [ "#{facts["budget_spent_pct"]}%", t("insights.figures.of_budget") ]
end
end
Expand Down
30 changes: 21 additions & 9 deletions app/models/insight.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,17 @@
# the LLM (when configured) only writes the `body` prose from pre-computed
# numbers, so rows are safe to render verbatim.
#
# Status semantics: `read` and `dismissed` are user actions; `expired` is the
# Status semantics: `read` and `acknowledged` are user actions; `expired` is the
# system's — set when a signal stops being generated (the condition cleared).
# A returning condition reactivates an expired row but never a dismissed one.
#
# "Acknowledged" rather than "dismissed" because that is what the state has
# always actually meant. GenerateInsightsJob resurfaces a row whose bucketed
# metadata changes materially even if the user acknowledged the stale version,
# and 6 of 8 generators scope `dedup_key` to a month, so acknowledging July's
# budget card says nothing about August's. The contract is: acknowledgement
# covers the numbers you saw; new numbers are a new insight. The DB value stays
# `"dismissed"` (and the `dismissed_at` column keeps its name) so this needed no
# migration — only the vocabulary the code and the UI speak was wrong.
class Insight < ApplicationRecord
belongs_to :family

Expand All @@ -20,7 +28,11 @@ class Insight < ApplicationRecord
budget_on_track
].freeze

enum :status, { active: "active", read: "read", dismissed: "dismissed", expired: "expired" }
# How many the dashboard widget shows. Shared so PagesController (first render)
# and InsightsController (re-render after acknowledging) can't drift apart.
FEED_LIMIT = 3

enum :status, { active: "active", read: "read", acknowledged: "dismissed", expired: "expired" }
enum :priority, { high: "high", medium: "medium", low: "low" }, prefix: true

validates :insight_type, presence: true, inclusion: { in: TYPES }
Expand All @@ -29,7 +41,7 @@ class Insight < ApplicationRecord
# instead of ActiveRecord::RecordNotUnique; races still hit the index.
validates :dedup_key, uniqueness: { scope: :family_id }

# Everything the user hasn't dismissed; what the feed renders.
# Everything the user hasn't acknowledged; what the feed renders.
scope :visible, -> { where(status: [ :active, :read ]) }
scope :ordered, -> {
order(Arel.sql("CASE insights.priority WHEN 'high' THEN 0 WHEN 'medium' THEN 1 ELSE 2 END"))
Expand All @@ -42,13 +54,13 @@ def mark_read!
update!(status: :read, read_at: Time.current)
end

def dismiss!
update!(status: :dismissed, dismissed_at: Time.current)
def acknowledge!
update!(status: :acknowledged, dismissed_at: Time.current)
end

# Undoes a dismissal without re-badging the insight as new — the user has
# obviously seen it, so it returns as read.
def undismiss!
# Undoes an acknowledgement without re-badging the insight as new — the user
# has obviously seen it, so it returns as read.
def unacknowledge!
update!(status: :read, dismissed_at: nil, read_at: read_at || Time.current)
Comment thread
coderabbitai[bot] marked this conversation as resolved.
end
end
74 changes: 45 additions & 29 deletions app/views/insights/_insight_card.html.erb
Original file line number Diff line number Diff line change
@@ -1,46 +1,62 @@
<%# locals: (insight:, unread: false) %>

<div id="<%= dom_id(insight) %>" class="bg-container rounded-xl shadow-border-xs p-4 flex gap-3">
<div class="shrink-0 mt-0.5">
<%= icon(insight_icon_key(insight), color: insight_icon_color(insight)) %>
</div>
<div id="<%= dom_id(insight) %>" class="bg-container rounded-xl shadow-border-xs">
<div class="p-4 flex gap-3">
<div class="shrink-0 mt-0.5">
<%= icon(insight_icon_key(insight), color: insight_icon_color(insight)) %>
</div>

<div class="min-w-0 grow space-y-1">
<p class="text-xs text-secondary"><%= insight_meta_line(insight) %></p>
<div class="min-w-0 grow space-y-1">
<p class="text-xs text-secondary"><%= insight_meta_line(insight) %></p>

<div class="flex items-start justify-between gap-3">
<h3 class="text-sm font-medium text-primary inline-flex items-center gap-2 min-w-0">
<%= insight.title %>
<% if unread %>
<%= render DS::Pill.new(label: t("insights.card.new"), tone: :info, size: :sm) %>
<% end %>
</h3>
<div class="flex items-start justify-between gap-3">
<h3 class="text-sm font-medium text-primary inline-flex items-center gap-2 min-w-0">
<%= insight.title %>
<% if unread %>
<%# A dot, not a labelled chip. Visiting this page marks every insight
read in one go, so at first paint the badge is on *every* row and
differentiates nothing — an uppercase tracked chip that heavy just
steals weight from the title it sits beside. %>
<%= render DS::Pill.new(label: t("insights.card.new"), tone: :info, dot_only: true) %>
<% end %>
</h3>

<div class="flex items-start gap-1 shrink-0">
<%# The figure now owns this corner outright. It used to share it with the
acknowledge control, so the card's data and its escape hatch competed
for the same spot. %>
<% if (figure = insight_key_figure(insight)) %>
<div class="text-right mr-1">
<div class="text-right shrink-0">
<p class="text-sm font-mono font-medium privacy-sensitive <%= insight_sentiment(insight) == :positive ? "text-success" : "text-primary" %>"><%= figure.first %></p>
<p class="text-xs text-subdued"><%= figure.last %></p>
</div>
<% end %>

<%= render DS::Button.new(
variant: "icon",
icon: "x",
href: dismiss_insight_path(insight),
method: :patch,
title: t("insights.card.dismiss"),
aria_label: t("insights.card.dismiss")
) %>
</div>
</div>

<p class="text-sm text-secondary"><%= insight.body %></p>
<p class="text-sm text-secondary"><%= insight.body %></p>
</div>
</div>

<%# Both actions in a footer strip, which fixes an inverted action pyramid: the
escape hatch used to be a chromed icon button in the top-right corner while
the card's actual purpose ("View budget") was a borderless ghost link
buried under the body text. The subject action gets the chrome now, and
acknowledging is labelled text beside it — quieter, and honest about being
an acknowledgement rather than a deletion. %>
<div class="px-4 py-2.5 bg-container-inset rounded-b-xl flex items-center justify-between gap-3">
<% if (action = insight_action(insight)) %>
<div class="pt-1">
<%= render DS::Link.new(text: action[:text], href: action[:href], variant: "ghost", size: "sm") %>
</div>
<%= render DS::Link.new(text: action[:text], href: action[:href], variant: "outline", size: "sm") %>
<% else %>
<span></span>
<% end %>

<%= render DS::Button.new(
text: t("insights.card.acknowledge"),
icon: "check",
variant: "ghost",
size: "sm",
href: acknowledge_insight_path(insight),
method: :patch,
title: t("insights.card.acknowledge_title")
) %>
</div>
</div>
28 changes: 22 additions & 6 deletions app/views/insights/_undo_toast.html.erb
Original file line number Diff line number Diff line change
@@ -1,19 +1,35 @@
<%# locals: (insight:) %>

<%# The card leaves the page via a Turbo `remove`, which is silent to a screen
reader — this toast is the only announcement of what happened, so it carries
the live region (same contract as shared/notifications/_sync_toast). %>
<div id="<%= dom_id(insight, :undo) %>"
role="status"
aria-live="polite"
class="relative flex items-center gap-3 rounded-lg bg-container p-4 group w-full md:max-w-80 shadow-border-xs"
data-controller="element-removal">
<p class="text-primary text-sm font-medium grow"><%= t("insights.card.dismissed") %></p>
<p class="text-primary text-sm font-medium grow"><%= t("insights.card.acknowledged") %></p>

<%# autofocus so Undo is one keystroke away: the acknowledged card is gone from
the DOM, which drops focus to <body>, and Turbo focuses the first
[autofocus] element in stream-rendered content. %>
<%= render DS::Button.new(
text: t("insights.card.undo"),
variant: "ghost",
size: "sm",
href: undismiss_insight_path(insight),
method: :patch
href: unacknowledge_insight_path(insight),
method: :patch,
autofocus: true
) %>

<%= icon "x",
class: "p-0.5 rounded-lg text-subdued hover:text-primary cursor-pointer shrink-0",
data: { action: "click->element-removal#remove" } %>
<%# A real button, not a bare icon with a click action: the glyph alone is
neither focusable nor named, so the toast could only be closed by mouse. %>
<%= render DS::Button.new(
variant: "icon",
size: "sm",
icon: "x",
type: "button",
"aria-label": t("defaults.common.close"),
data: { action: "click->element-removal#remove" }
) %>
</div>
16 changes: 16 additions & 0 deletions app/views/insights/acknowledge.turbo_stream.erb
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
<%# Acknowledging is reachable from two surfaces now, so this response carries
the update for both and each applies only the streams whose targets it has —
Turbo silently ignores a stream whose target is absent. %>

<%# /insights: drop the card. %>
<%= turbo_stream.remove dom_id(@insight) %>

<%# Dashboard: re-render the well rather than removing a row, so the next
insight is promoted into the freed slot instead of leaving a gap. %>
<%= turbo_stream.replace "insights-feed" do %>
<%= render "pages/dashboard/insights_feed", insights: @feed_insights %>
<% end %>

<%= turbo_stream.append "notification-tray" do %>
<%= render "insights/undo_toast", insight: @insight %>
<% end %>
5 changes: 0 additions & 5 deletions app/views/insights/dismiss.turbo_stream.erb

This file was deleted.

11 changes: 11 additions & 0 deletions app/views/insights/unacknowledge.turbo_stream.erb
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
<%= turbo_stream.remove dom_id(@insight, :undo) %>

<%= turbo_stream.replace "insights-list" do %>
<%= render "insights/list", insights: @insights, unread_ids: @unread_ids %>
<% end %>

<%# Undo can be pressed from the dashboard, where the toast is the only part of
this flow on screen — so the well needs restoring there too. %>
<%= turbo_stream.replace "insights-feed" do %>
<%= render "pages/dashboard/insights_feed", insights: @feed_insights %>
<% end %>
5 changes: 0 additions & 5 deletions app/views/insights/undismiss.turbo_stream.erb

This file was deleted.

58 changes: 43 additions & 15 deletions app/views/pages/dashboard/_insights_feed.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@

<%# Mirrors the outflows/balance-sheet list idiom: an inset well with an
uppercase mini-header, white rows inside, tinted icon circles, and
right-aligned figures. Dismissal lives on /insights; here each row just
links through. Prefetch is disabled because Turbo would serve the
right-aligned figures. Each row links through to /insights and carries its
own acknowledge control. Prefetch is disabled because Turbo would serve the
hover-prefetched response on click, skipping the mark-as-read in
InsightsController#index (it deliberately ignores prefetch requests). %>
<%# -mt-2 evens the frame: the section shell gives 24px above the content but
Expand Down Expand Up @@ -31,23 +31,28 @@

<div class="py-1.5 shadow-border-xs rounded-lg bg-container text-sm">
<% insights.each do |insight| %>
<%# No hover background, matching the outflows rows — the affordance is
the cursor plus the icon's gentle scale. %>
<%= link_to insights_path,
data: { turbo_prefetch: false },
class: "flex items-center mx-3 px-3 py-2.5 rounded-lg gap-3 cursor-pointer group" do %>
<div class="h-7 w-7 shrink-0 rounded-full flex justify-center items-center group-hover:scale-105 transition-all duration-300"
<%# Stretched link rather than a link wrapping the row, because the
acknowledge control is a button_to — a <form>, which cannot be
nested inside an <a>. Same idiom as the outflows rows. %>
<div class="relative flex items-center mx-3 px-3 py-2.5 rounded-lg gap-3 group/insight">
<%# Empty body with an aria-label: the visible title is a sibling, and
passing a name would render it as link text on top of the row. %>
<%= link_to "", insights_path,
class: "absolute inset-0 rounded-lg cursor-pointer",
aria: { label: insight.title },
data: { turbo_prefetch: false } %>

<div class="h-7 w-7 shrink-0 rounded-full flex justify-center items-center group-hover/insight:scale-105 transition-all duration-300"
style="background-color: color-mix(in oklab, <%= insight_icon_css_color(insight) %> 10%, transparent); color: <%= insight_icon_css_color(insight) %>;">
<%= icon(insight_icon_key(insight), color: "current", size: "sm") %>
</div>

<div class="min-w-0 grow">
<div class="flex items-center gap-2">
<span class="text-sm font-medium text-primary truncate"><%= insight.title %></span>
<% if insight.active? %>
<%= render DS::Pill.new(label: t("insights.card.new"), tone: :info, size: :sm) %>
<% end %>
</div>
<%# No per-row "New" pill: the well's header already counts the
unread ones ("New · 3"), and with only three rows the pill was
usually on all of them — saying the same thing twice while
crowding the title. %>
<span class="text-sm font-medium text-primary truncate block"><%= insight.title %></span>
<p class="text-sm text-secondary truncate"><%= insight.body %></p>
</div>

Expand All @@ -57,7 +62,30 @@
<p class="text-xs text-secondary whitespace-nowrap"><%= figure.last %></p>
</div>
<% end %>
<% end %>

<%# Acknowledging used to be possible only on /insights, so the surface
people actually look at couldn't clear anything. `relative z-10`
lifts this above the stretched link. Visibility is progressive and
never hover-only: pointer hover, keyboard focus, and always shown
on touch, where there is no hover to reveal it. No gesture, so the
section's drag-to-reorder handlers are untouched.

The group is *named*: the dashboard <section> wrapping this widget
is itself a `.group` (for its header controls), and a bare
`group-hover:` matches any ancestor group — so hovering one row, or
even the section header, revealed the control on every row. %>
<div class="relative z-10 shrink-0 opacity-0 group-hover/insight:opacity-100 group-focus-within/insight:opacity-100 pointer-coarse:opacity-100 transition-opacity">
<%= render DS::Button.new(
variant: "icon",
size: "sm",
icon: "check",
href: acknowledge_insight_path(insight),
method: :patch,
title: t("insights.card.acknowledge"),
aria_label: t("insights.feed.acknowledge_aria", title: insight.title)
) %>
</div>
</div>
<% end %>
</div>
</div>
Expand Down
Loading
Loading