diff --git a/app/controllers/reports_controller.rb b/app/controllers/reports_controller.rb index 763dd68f2..8dfea80ee 100644 --- a/app/controllers/reports_controller.rb +++ b/app/controllers/reports_controller.rb @@ -374,13 +374,13 @@ class ReportsController < ApplicationController family_currency = Current.family.currency # Helper to initialize a category group hash - init_category_group = ->(id, name, color, type) do - { category_id: id, category_name: name, category_color: color, type: type, total: 0, count: 0, subcategories: {} } + init_category_group = ->(id, name, color, icon, type) do + { category_id: id, category_name: name, category_color: color, category_icon: icon, type: type, total: 0, count: 0, subcategories: {} } end # Helper to initialize a subcategory hash init_subcategory = ->(category) do - { category_id: category.id, category_name: category.name, category_color: category.color, total: 0, count: 0 } + { category_id: category.id, category_name: category.name, category_color: category.color, category_icon: category.lucide_icon, total: 0, count: 0 } end # Helper to process an entry (transaction or trade) @@ -392,16 +392,16 @@ class ReportsController < ApplicationController # Uncategorized or Other Investments (for trades) if is_trade parent_key = [ :other_investments, type ] - grouped_data[parent_key] ||= init_category_group.call(:other_investments, Category.other_investments_name, Category::OTHER_INVESTMENTS_COLOR, type) + grouped_data[parent_key] ||= init_category_group.call(:other_investments, Category.other_investments.name, Category.other_investments.color, Category.other_investments.lucide_icon, type) else parent_key = [ :uncategorized, type ] - grouped_data[parent_key] ||= init_category_group.call(:uncategorized, Category.uncategorized_name, Category::UNCATEGORIZED_COLOR, type) + grouped_data[parent_key] ||= init_category_group.call(:uncategorized, Category.uncategorized.name, Category.uncategorized.color, Category.uncategorized.lucide_icon, type) end elsif category.parent_id.present? # This is a subcategory - group under parent parent = category.parent parent_key = [ parent.id, type ] - grouped_data[parent_key] ||= init_category_group.call(parent.id, parent.name, parent.color || Category::UNCATEGORIZED_COLOR, type) + grouped_data[parent_key] ||= init_category_group.call(parent.id, parent.name, parent.color || Category::UNCATEGORIZED_COLOR, parent.lucide_icon, type) # Add to subcategory grouped_data[parent_key][:subcategories][category.id] ||= init_subcategory.call(category) @@ -410,7 +410,7 @@ class ReportsController < ApplicationController else # This is a root category (no parent) parent_key = [ category.id, type ] - grouped_data[parent_key] ||= init_category_group.call(category.id, category.name, category.color || Category::UNCATEGORIZED_COLOR, type) + grouped_data[parent_key] ||= init_category_group.call(category.id, category.name, category.color || Category::UNCATEGORIZED_COLOR, category.lucide_icon, type) end grouped_data[parent_key][:count] += 1 diff --git a/app/views/reports/_breakdown_table.html.erb b/app/views/reports/_breakdown_table.html.erb index 83d15350d..7f6b897be 100644 --- a/app/views/reports/_breakdown_table.html.erb +++ b/app/views/reports/_breakdown_table.html.erb @@ -8,78 +8,54 @@ %>
-

+
<%= icon(icon_name, class: "w-5 h-5") %> - <%= t(title_key) %> - (<%= Money.new(total, Current.family.currency).format %>) -

+ <%= t(title_key) %>: + <%= Money.new(total, Current.family.currency).format %> +
-
- - - - - - - - - - <% groups.each do |group| %> - <% percentage = total.zero? ? 0 : (group[:total].to_f / total * 100).round(1) %> - <% has_subcategories = group[:subcategories].present? && group[:subcategories].any? %> - - - - - + <% end %> + +
<%= t("reports.transactions_breakdown.table.percentage") %>
+ + +
+ <% groups.each_with_index do |group, idx| %> + <%= render "reports/category_row", + item: group, + total: total, + color_class: color_class, + level: :category + %> + <% if idx < group.size - 1 %> + <%= render "shared/ruler", classes: "mx-3 lg:mx-4" %> + <% end %> <%# Render subcategories if present %> - <% if has_subcategories %> - <% group[:subcategories].each do |subcategory| %> - <% sub_percentage = total.zero? ? 0 : (subcategory[:total].to_f / total * 100).round(1) %> -
- - - - + <% if group[:subcategories].present? && group[:subcategories].any? %> + <% group[:subcategories].each_with_index do |subcategory, idx| %> + <%= render "reports/category_row", + item: subcategory, + total: total, + color_class: color_class, + level: :subcategory + %> + <% end %> + <% if idx < group.size - 1 %> + <%= render "shared/ruler", classes: "mx-3 lg:mx-4" %> <% end %> <% end %> <% end %> - -
<%= t("reports.transactions_breakdown.table.category") %> - <%= link_to reports_path(amount_sort_params), class: "inline-flex items-center gap-1 hover:text-primary" do %> - <%= t("reports.transactions_breakdown.table.amount") %> - <% if current_sort_by == "amount" %> - <%= icon(current_sort_direction == "desc" ? "chevron-down" : "chevron-up", class: "w-3 h-3") %> - <% end %> +
+
+
+
<%= t("reports.transactions_breakdown.table.category") %>
+
+ <%= link_to reports_path(amount_sort_params), class: "inline-flex items-center gap-1 hover:text-primary" do %> + <%= t("reports.transactions_breakdown.table.amount") %> + <% if current_sort_by == "amount" %> + <%= icon(current_sort_direction == "desc" ? "chevron-down" : "chevron-up", class: "w-3 h-3") %> <% end %> -
<%= t("reports.transactions_breakdown.table.percentage") %>
-
- - <%= group[:category_name] %> - (<%= t("reports.transactions_breakdown.table.entries", count: group[:count]) %>) -
-
- - <%= Money.new(group[:total], Current.family.currency).format %> - - - - <%= percentage %>% - -
-
- - <%= subcategory[:category_name] %> - (<%= t("reports.transactions_breakdown.table.entries", count: subcategory[:count]) %>) -
-
- - <%= Money.new(subcategory[:total], Current.family.currency).format %> - - - - <%= sub_percentage %>% - -
+
+ diff --git a/app/views/reports/_category_row.html.erb b/app/views/reports/_category_row.html.erb new file mode 100644 index 000000000..449cac8bb --- /dev/null +++ b/app/views/reports/_category_row.html.erb @@ -0,0 +1,50 @@ +<% + percentage = total.zero? ? 0 : (item[:total].to_f / total * 100).round(1) + is_sub = level == :subcategory +%> + +
"> +
+ <% if is_sub %> +
+ <%= icon "corner-down-right" %> +
+ <% end %> + <% if item[:category_icon] %> +
+ <%= icon(item[:category_icon], color: "current", size: "sm") %> +
+ <% else %> + <%= render DS::FilledIcon.new( + variant: :text, + hex_color: item[:category_color], + text: item[:category_name], + size: "md", + rounded: true + ) %> + <% end %> + + <%= item[:category_name] %> + + + (<%= t("reports.transactions_breakdown.table.entries", count: item[:count]) %>) + +
+ +
+ + <%= Money.new(item[:total], Current.family.currency).format %> + +
+ +
+ + <%= percentage %>% + +
+
diff --git a/app/views/reports/_investment_performance.html.erb b/app/views/reports/_investment_performance.html.erb index b81a0a544..aa63cbf4d 100644 --- a/app/views/reports/_investment_performance.html.erb +++ b/app/views/reports/_investment_performance.html.erb @@ -2,14 +2,12 @@ <% if investment_metrics[:has_investments] %>
-

<%= t("reports.investment_performance.title") %>

- <%# Investment Summary Cards %>
<%# Portfolio Value Card %>
-
- <%= icon("briefcase", class: "w-4 h-4 text-secondary") %> +
+ <%= icon("briefcase", size: "sm") %> <%= t("reports.investment_performance.portfolio_value") %>

@@ -19,8 +17,8 @@ <%# Total Return Card %>

-
- <%= icon("trending-up", class: "w-4 h-4 text-secondary") %> +
+ <%= icon("trending-up", size: "sm") %> <%= t("reports.investment_performance.total_return") %>
<% if investment_metrics[:unrealized_trend] %> @@ -35,8 +33,8 @@ <%# Period Contributions Card %>
-
- <%= icon("arrow-down-to-line", class: "w-4 h-4 text-secondary") %> +
+ <%= icon("arrow-down-to-line", size: "sm") %> <%= t("reports.investment_performance.contributions") %>

@@ -46,8 +44,8 @@ <%# Period Withdrawals Card %>

-
- <%= icon("arrow-up-from-line", class: "w-4 h-4 text-secondary") %> +
+ <%= icon("arrow-up-from-line", size: "sm") %> <%= t("reports.investment_performance.withdrawals") %>

@@ -59,57 +57,50 @@ <%# Top Holdings Table %> <% if investment_metrics[:top_holdings].any? %>

-

<%= t("reports.investment_performance.top_holdings") %>

+

<%= t("reports.investment_performance.top_holdings") %>

-
- - - - - - - - - - - <% investment_metrics[:top_holdings].each do |holding| %> - - - - - - + <% end %> +
+

<%= holding.ticker %>

+

<%= truncate(holding.name, length: 25) %>

+
+ +
<%= number_to_percentage(holding.weight || 0, precision: 1) %>
+
<%= format_money(holding.amount_money) %>
+
+ <% if holding.trend %> + + <%= holding.trend.percent_formatted %> + + <% else %> + <%= t("reports.investment_performance.no_data") %> + <% end %> +
+ + <% if idx < investment_metrics[:top_holdings].size - 1 %> + <%= render "shared/ruler", classes: "mx-3 lg:mx-4" %> <% end %> - -
<%= t("reports.investment_performance.holding") %><%= t("reports.investment_performance.weight") %><%= t("reports.investment_performance.value") %><%= t("reports.investment_performance.return") %>
-
- <% if holding.security.brandfetch_icon_url.present? %> - <%= holding.ticker %> - <% elsif holding.security.logo_url.present? %> - <%= holding.ticker %> - <% else %> -
- <%= holding.ticker[0..1] %> -
- <% end %> -
-

<%= holding.ticker %>

-

<%= truncate(holding.name, length: 25) %>

-
+
+
+
<%= t("reports.investment_performance.holding") %>
+
<%= t("reports.investment_performance.weight") %>
+
<%= t("reports.investment_performance.value") %>
+
<%= t("reports.investment_performance.return") %>
+
+
+ <% investment_metrics[:top_holdings].each_with_index do |holding, idx| %> +
+
+ <% if holding.security.brandfetch_icon_url.present? %> + <%= holding.ticker %> + <% elsif holding.security.logo_url.present? %> + <%= holding.ticker %> + <% else %> +
+ <%= holding.ticker[0..1] %>
-
- <%= number_to_percentage(holding.weight || 0, precision: 1) %> - - <%= format_money(holding.amount_money) %> - - <% if holding.trend %> - - <%= holding.trend.percent_formatted %> - - <% else %> - <%= t("reports.investment_performance.no_data") %> - <% end %> -
+ <% end %> +
<% end %> @@ -117,7 +108,7 @@ <%# Gains by Tax Treatment %> <% if investment_metrics[:gains_by_tax_treatment].present? %>
-

<%= t("reports.investment_performance.gains_by_tax_treatment") %>

+

<%= t("reports.investment_performance.gains_by_tax_treatment") %>

<% investment_metrics[:gains_by_tax_treatment].each do |treatment, data| %> @@ -202,7 +193,7 @@ <%# Investment Accounts Summary %> <% if investment_metrics[:accounts].any? %>
-

<%= t("reports.investment_performance.accounts") %>

+

<%= t("reports.investment_performance.accounts") %>

<% investment_metrics[:accounts].each do |account| %> diff --git a/app/views/reports/_net_worth.html.erb b/app/views/reports/_net_worth.html.erb index d7fa0beff..cc1be261f 100644 --- a/app/views/reports/_net_worth.html.erb +++ b/app/views/reports/_net_worth.html.erb @@ -3,31 +3,31 @@
<%# Current Net Worth %>
-

<%= t("reports.net_worth.current_net_worth") %>

-

"> +

<%= t("reports.net_worth.current_net_worth") %>

+

"> <%= net_worth_metrics[:current_net_worth].format %>

<%# Period Change %>
-

<%= t("reports.net_worth.period_change") %>

+

<%= t("reports.net_worth.period_change") %>

<% if net_worth_metrics[:trend] %> <% trend = net_worth_metrics[:trend] %> -

+

<%= trend.value.format(signify_positive: true) %>

<%= trend.value >= 0 ? "+" : "" %><%= trend.percent_formatted %>

<% else %> -

--

+

--

<% end %>
<%# Assets vs Liabilities %>
-

<%= t("reports.net_worth.assets_vs_liabilities") %>

+

<%= t("reports.net_worth.assets_vs_liabilities") %>

<%= net_worth_metrics[:total_assets].format %> - @@ -39,30 +39,40 @@ <%# Asset/Liability Breakdown %>
<%# Assets Summary %> -
-

<%= t("reports.net_worth.total_assets") %>

-
- <% net_worth_metrics[:asset_groups].each do |group| %> -
- <%= group[:name] %> - <%= group[:total].format %> +
+
+
<%= t("reports.net_worth.total_assets") %>
+
+
+ <% net_worth_metrics[:asset_groups].each_with_index do |group, idx| %> +
+
<%= group[:name] %>
+
<%= group[:total].format %>
+ <% if idx < net_worth_metrics[:asset_groups].size - 1 %> + <%= render "shared/ruler", classes: "mx-3 lg:mx-4" %> + <% end %> <% end %> <% if net_worth_metrics[:asset_groups].empty? %> -

<%= t("reports.net_worth.no_assets") %>

+

<%= t("reports.net_worth.no_assets") %>

<% end %>
<%# Liabilities Summary %> -
-

<%= t("reports.net_worth.total_liabilities") %>

-
- <% net_worth_metrics[:liability_groups].each do |group| %> -
- <%= group[:name] %> - <%= group[:total].format %> +
+
+
<%= t("reports.net_worth.total_liabilities") %>
+
+
+ <% net_worth_metrics[:liability_groups].each_with_index do |group, idx| %> +
+
<%= group[:name] %>
+
<%= group[:total].format %>
+ <% if idx < net_worth_metrics[:liability_groups].size - 1 %> + <%= render "shared/ruler", classes: "mx-3 lg:mx-4" %> + <% end %> <% end %> <% if net_worth_metrics[:liability_groups].empty? %>

<%= t("reports.net_worth.no_liabilities") %>

diff --git a/app/views/reports/_summary_dashboard.html.erb b/app/views/reports/_summary_dashboard.html.erb index 151a32d0a..bc0436174 100644 --- a/app/views/reports/_summary_dashboard.html.erb +++ b/app/views/reports/_summary_dashboard.html.erb @@ -3,7 +3,7 @@
- <%= icon("trending-up", class: "w-5 h-5 text-success") %> + <%= icon("trending-up", size: "sm") %>

<%= t("reports.summary.total_income") %>

@@ -18,12 +18,12 @@ <% if metrics[:income_change] %>
<% if metrics[:income_change] >= 0 %> - <%= icon("arrow-up", class: "w-4 h-4 text-success") %> + <%= icon("arrow-up", size: "sm") %> +<%= metrics[:income_change] %>% <% else %> - <%= icon("arrow-down", class: "w-4 h-4 text-destructive") %> + <%= icon("arrow-down", size: "sm") %> <%= metrics[:income_change] %>% @@ -40,7 +40,7 @@
- <%= icon("trending-down", class: "w-5 h-5 text-destructive") %> + <%= icon("trending-down", class: "w-5 h-5") %>

<%= t("reports.summary.total_expenses") %>

@@ -77,7 +77,7 @@
- <%= icon("piggy-bank", class: "w-5 h-5 text-primary") %> + <%= icon("piggy-bank", class: "w-5 h-5") %>

<%= t("reports.summary.net_savings") %>

@@ -99,7 +99,7 @@
- <%= icon("gauge", class: "w-5 h-5 text-primary") %> + <%= icon("gauge", class: "w-5 h-5") %>

<%= t("reports.summary.budget_performance") %>

diff --git a/app/views/reports/_transactions_breakdown.html.erb b/app/views/reports/_transactions_breakdown.html.erb index 66c3d0e18..678d51e63 100644 --- a/app/views/reports/_transactions_breakdown.html.erb +++ b/app/views/reports/_transactions_breakdown.html.erb @@ -14,14 +14,13 @@ <%# Export Options %>
- <%= t("reports.transactions_breakdown.export.label") %>: <%= link_to export_transactions_reports_path(base_params.merge(format: :csv)), - class: "inline-flex items-center gap-1 text-sm px-3 py-1 bg-surface-inset text-secondary hover:bg-surface-hover rounded-lg" do %> + class: "font-medium whitespace-nowrap inline-flex items-center gap-1 rounded-lg px-3 py-2 text-sm text-primary border border-secondary bg-transparent hover:bg-surface-hover" do %> <%= icon("download", class: "w-3 h-3") %> <%= t("reports.transactions_breakdown.export.csv") %> <% end %> <%= link_to google_sheets_instructions_reports_path(base_params), - class: "inline-flex items-center gap-1 text-sm px-3 py-1 bg-surface-inset text-secondary hover:bg-surface-hover rounded-lg", + class: "font-medium whitespace-nowrap inline-flex items-center gap-1 rounded-lg px-3 py-2 text-sm text-primary border border-secondary bg-transparent hover:bg-surface-hover", data: { turbo_frame: "modal" } do %> <%= icon("external-link", class: "w-3 h-3") %> <%= t("reports.transactions_breakdown.export.google_sheets") %> @@ -76,11 +75,11 @@
<%# Summary Stats %> -
+
<%= t("reports.transactions_breakdown.pagination.showing", count: transactions.sum { |g| g[:count] }) %>
<% else %> -
+
<%= t("reports.transactions_breakdown.no_transactions") %>
<% end %> diff --git a/app/views/reports/_trends_insights.html.erb b/app/views/reports/_trends_insights.html.erb index 279569c0e..66c2475ba 100644 --- a/app/views/reports/_trends_insights.html.erb +++ b/app/views/reports/_trends_insights.html.erb @@ -1,48 +1,49 @@
<%# Month-over-Month Trends %>
-

+

<%= t("reports.trends.monthly_breakdown") %>

<% if trends_data.any? %> -
- - - - - - - - - - - - <% trends_data.each do |trend| %> - "> - - - - - - + + + <% if idx < trends_data.size - 1 %> + <%= render "shared/ruler", classes: "mx-3 lg:mx-4" %> + <% end %> <% end %> - -
<%= t("reports.trends.month") %><%= t("reports.trends.income") %><%= t("reports.trends.expenses") %><%= t("reports.trends.net") %><%= t("reports.trends.savings_rate") %>
+
+
+
+
<%= t("reports.trends.month") %>
+
<%= t("reports.trends.income") %>
+
<%= t("reports.trends.expenses") %>
+
<%= t("reports.trends.net") %>
+
<%= t("reports.trends.savings_rate") %>
+
+
+ <% trends_data.each_with_index do |trend, idx| %> +
+
"> <%= trend[:month] %> <% if trend[:is_current_month] %> - (<%= t("reports.trends.current") %>) + (<%= t("reports.trends.current") %>) <% end %> -
+ +
<%= Money.new(trend[:income], Current.family.currency).format %> -
+ +
<%= Money.new(trend[:expenses], Current.family.currency).format %> -
"> + +
"> <%= Money.new(trend[:net], Current.family.currency).format %> -
"> + +
"> <% savings_rate = trend[:income] > 0 ? ((trend[:net].to_f / trend[:income].to_f) * 100).round(1) : 0 %> <%= savings_rate %>% -
+
+
<%# Trend Insights %> @@ -52,21 +53,21 @@ <% avg_net = trends_data.sum { |t| t[:net] } / trends_data.length %>
-

<%= t("reports.trends.avg_monthly_income") %>

+

<%= t("reports.trends.avg_monthly_income") %>

<%= Money.new(avg_income, Current.family.currency).format %>

-

<%= t("reports.trends.avg_monthly_expenses") %>

+

<%= t("reports.trends.avg_monthly_expenses") %>

<%= Money.new(avg_expenses, Current.family.currency).format %>

-

<%= t("reports.trends.avg_monthly_savings") %>

+

<%= t("reports.trends.avg_monthly_savings") %>

"> <%= Money.new(avg_net, Current.family.currency).format %>

diff --git a/app/views/reports/index.html.erb b/app/views/reports/index.html.erb index 31856fdb8..888271567 100644 --- a/app/views/reports/index.html.erb +++ b/app/views/reports/index.html.erb @@ -1,24 +1,36 @@ <% content_for :page_header do %>
-
-

- <%= t("reports.index.title") %> -

-

- <%= t("reports.index.subtitle") %> -

-
- - <%# Flash messages %> - <% if flash[:alert].present? %> -
- <%= flash[:alert] %> +
+
+

+ <%= t("reports.index.title") %> +

+

+ <%= t("reports.index.subtitle") %> +

- <% end %> + + <%# Flash messages %> + <% if flash[:alert].present? %> +
+ <%= flash[:alert] %> +
+ <% end %> + + <%# Print Report Button %> + <%= link_to print_reports_path(period_type: @period_type, start_date: @start_date, end_date: @end_date), + target: "_blank", + rel: "noopener", + aria: { label: t("reports.index.print_report") }, + class: "font-medium whitespace-nowrap inline-flex items-center gap-1 rounded-lg px-3 py-2 text-sm text-primary border border-secondary bg-transparent hover:bg-surface-hover" do %> + <%= icon("printer", size: "sm") %> + + <% end %> +
<%# Period Navigation Tabs %>
-
+
<%= render DS::Link.new( text: t("reports.index.periods.monthly"), variant: @period_type == :monthly ? "secondary" : "ghost", @@ -50,16 +62,6 @@ size: :sm ) %>
- - <%# Print Report Button %> - <%= link_to print_reports_path(period_type: @period_type, start_date: @start_date, end_date: @end_date), - target: "_blank", - rel: "noopener", - aria: { label: t("reports.index.print_report") }, - class: "inline-flex items-center gap-2 px-3 py-1.5 text-sm font-medium text-secondary bg-surface-inset hover:bg-surface-hover rounded-lg transition-colors flex-shrink-0" do %> - <%= icon("printer", size: "sm") %> - - <% end %>
<%# Custom Date Range Picker (only shown when custom is selected) %> @@ -88,7 +90,7 @@ <% end %> <%# Period Display %> -
+
<%= t("reports.index.showing_period", start: @start_date.strftime("%b %-d, %Y"), end: @end_date.strftime("%b %-d, %Y")) %> diff --git a/config/locales/views/reports/en.yml b/config/locales/views/reports/en.yml index fc1e85142..598070944 100644 --- a/config/locales/views/reports/en.yml +++ b/config/locales/views/reports/en.yml @@ -105,8 +105,8 @@ en: income: Income uncategorized: Uncategorized entries: - one: entry - other: entries + one: "%{count} entry" + other: "%{count} entries" percentage: "% of Total" pagination: showing: diff --git a/test/controllers/reports_controller_test.rb b/test/controllers/reports_controller_test.rb index a4524382d..afc81abd3 100644 --- a/test/controllers/reports_controller_test.rb +++ b/test/controllers/reports_controller_test.rb @@ -82,7 +82,9 @@ class ReportsControllerTest < ActionDispatch::IntegrationTest get reports_path(period_type: :monthly) assert_response :ok assert_select "h2", text: I18n.t("reports.trends.title") - assert_select "th", text: I18n.t("reports.trends.month") + assert_select '[role="columnheader"]' do + assert_select "div", text: I18n.t("reports.trends.month") + end end test "index handles invalid date parameters gracefully" do @@ -236,6 +238,12 @@ class ReportsControllerTest < ActionDispatch::IntegrationTest get reports_path(period_type: :monthly) assert_response :ok - assert_select "table.w-full" + + # Parent category + assert_select "div[data-category='category-#{parent_category.id}']", text: /^Entertainment/ + + # Subcategories + assert_select "div[data-category='category-#{subcategory_movies.id}']", text: /^Movies/ + assert_select "div[data-category='category-#{subcategory_games.id}']", text: /^Games/ end end