From fa5c544431a366295bacaaad8e159faff8dac5bd Mon Sep 17 00:00:00 2001 From: William Wei Ming <280573057+bittensorrider@users.noreply.github.com> Date: Sun, 16 Aug 2026 13:24:36 +0700 Subject: [PATCH] Make report income/expense categories clickable (#2923) * Make report category rows link to filtered transactions Match dashboard drill-down for income/expense categories on the reports breakdown, while leaving synthetic Other Investments non-clickable (#2850). Co-authored-by: Cursor * Only link report categories backed by transactions Track has_transactions while building breakdown groups so trade-only rows (e.g. Other Investments) are not sent to /transactions, which cannot show Trade entries. Co-authored-by: Cursor * Drop redundant trade-only reports link test Coverage for non-clickable Other Investments remains in the tax-advantaged breakdown test. Co-authored-by: Cursor * Strengthen reports category link coverage in tests Cover income and uncategorized drill-down links, and assert every Other Investments row has no transaction link. Co-authored-by: Cursor * Make report category rows fully clickable like outflows Use a stretched ::before link on the row for a larger hit target, and cover Uncategorized href localization against Transaction::Search. Co-authored-by: Cursor --------- Co-authored-by: Cursor --- app/controllers/reports_controller.rb | 24 +++++++- app/views/reports/_breakdown_table.html.erb | 6 +- app/views/reports/_category_row.html.erb | 26 +++++++-- .../reports/_transactions_breakdown.html.erb | 8 ++- test/controllers/reports_controller_test.rb | 56 +++++++++++++++++-- 5 files changed, 105 insertions(+), 15 deletions(-) diff --git a/app/controllers/reports_controller.rb b/app/controllers/reports_controller.rb index 7568a2949..d02169a59 100644 --- a/app/controllers/reports_controller.rb +++ b/app/controllers/reports_controller.rb @@ -398,12 +398,30 @@ class ReportsController < ApplicationController # Helper to initialize a category group hash 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: {} } + { + category_id: id, + category_name: name, + category_color: color, + category_icon: icon, + type: type, + total: 0, + count: 0, + has_transactions: false, + subcategories: {} + } end # Helper to initialize a subcategory hash init_subcategory = ->(category) do - { category_id: category.id, category_name: category.name, category_color: category.color, category_icon: category.lucide_icon, 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, + has_transactions: false + } end # Helper to process an entry (transaction or trade) @@ -434,6 +452,7 @@ class ReportsController < ApplicationController grouped_data[parent_key][:subcategories][category.id] ||= init_subcategory.call(category) grouped_data[parent_key][:subcategories][category.id][:count] += 1 grouped_data[parent_key][:subcategories][category.id][:total] += converted_amount + grouped_data[parent_key][:subcategories][category.id][:has_transactions] = true unless is_trade else # This is a root category (no parent) parent_key = [ category.id, type ] @@ -442,6 +461,7 @@ class ReportsController < ApplicationController grouped_data[parent_key][:count] += 1 grouped_data[parent_key][:total] += converted_amount + grouped_data[parent_key][:has_transactions] = true unless is_trade end # Process transactions diff --git a/app/views/reports/_breakdown_table.html.erb b/app/views/reports/_breakdown_table.html.erb index 95fbfab1b..e0bc31763 100644 --- a/app/views/reports/_breakdown_table.html.erb +++ b/app/views/reports/_breakdown_table.html.erb @@ -1,5 +1,5 @@ <%# Renders a breakdown table for income or expense groups %> -<%# Local variables: groups, total, type (:income or :expense), amount_sort_params, current_sort_by, current_sort_direction %> +<%# Local variables: groups, total, type (:income or :expense), amount_sort_params, current_sort_by, current_sort_direction, start_date, end_date %> <% color_class = type == :income ? "text-success" : "text-primary" @@ -38,6 +38,8 @@ total: total, color_class: color_class, level: :category, + start_date: start_date, + end_date: end_date, show_border: idx < groups.size - 1 || group[:subcategories].present? %> <%# Render subcategories if present %> <% if group[:subcategories].present? && group[:subcategories].any? %> @@ -47,6 +49,8 @@ total: total, color_class: color_class, level: :subcategory, + start_date: start_date, + end_date: end_date, show_border: sub_idx < group[:subcategories].size - 1 %> <% end %> <% end %> diff --git a/app/views/reports/_category_row.html.erb b/app/views/reports/_category_row.html.erb index 9ef572e7d..b1a7661c4 100644 --- a/app/views/reports/_category_row.html.erb +++ b/app/views/reports/_category_row.html.erb @@ -2,9 +2,16 @@ percentage = total.zero? ? 0 : (item[:total].to_f / total * 100).round(1) is_sub = level == :subcategory show_border = local_assigns.fetch(:show_border, false) + row_start_date = local_assigns[:start_date] + row_end_date = local_assigns[:end_date] + # /transactions only lists Transaction rows. Skip trade-only buckets (e.g. Other + # Investments) so the destination is not empty relative to the report row. + clickable = item[:has_transactions] && row_start_date.present? && row_end_date.present? + transactions_href = clickable ? transactions_path(q: { categories: [ item[:category_name] ], start_date: row_start_date, end_date: row_end_date }) : nil %> -"> + <%= clickable ? "relative group/category-row" : "" %>"> ">
<% if is_sub %> @@ -13,7 +20,7 @@
<% end %> <% if item[:category_icon] %> -
" style=" background-color: color-mix(in oklab, <%= item[:category_color] %> 10%, transparent); border-color: color-mix(in oklab, <%= item[:category_color] %> 10%, transparent); @@ -30,9 +37,18 @@ rounded: true ) %> <% end %> - - <%= item[:category_name] %> - + <% if clickable %> + <%# Stretched link covers the full row (mirrors dashboard outflows). + `before:absolute before:inset-0` is positioned against the relative . %> + <%= link_to item[:category_name], + transactions_href, + class: "font-medium text-primary hover:underline before:absolute before:inset-0 before:content-['']", + data: { turbo_frame: "_top", turbo_prefetch: false } %> + <% else %> + + <%= item[:category_name] %> + + <% end %> (<%= t("reports.transactions_breakdown.table.entries", count: item[:count]) %>) diff --git a/app/views/reports/_transactions_breakdown.html.erb b/app/views/reports/_transactions_breakdown.html.erb index 678d51e63..e90194f20 100644 --- a/app/views/reports/_transactions_breakdown.html.erb +++ b/app/views/reports/_transactions_breakdown.html.erb @@ -59,7 +59,9 @@ type: :income, amount_sort_params: amount_sort_params, current_sort_by: current_sort_by, - current_sort_direction: current_sort_direction %> + current_sort_direction: current_sort_direction, + start_date: start_date, + end_date: end_date %> <% end %> <%# Expenses Section %> @@ -70,7 +72,9 @@ type: :expense, amount_sort_params: amount_sort_params, current_sort_by: current_sort_by, - current_sort_direction: current_sort_direction %> + current_sort_direction: current_sort_direction, + start_date: start_date, + end_date: end_date %> <% end %>
diff --git a/test/controllers/reports_controller_test.rb b/test/controllers/reports_controller_test.rb index 59797f5e6..442efae1d 100644 --- a/test/controllers/reports_controller_test.rb +++ b/test/controllers/reports_controller_test.rb @@ -420,6 +420,49 @@ class ReportsControllerTest < ActionDispatch::IntegrationTest assert_select "tr[data-category='category-#{subcategory_games.id}']", text: /^Games/ end + test "index links income and expense categories to filtered transactions" do + start_date = Date.current.beginning_of_month + end_date = Date.current.end_of_month + expense_category = @family.categories.create!(name: "Reports Clickable Groceries", color: "#ABCDEF") + income_category = @family.categories.create!(name: "Reports Clickable Salary", color: "#FEDCBA") + account = @family.accounts.first + + create_transaction(account: account, name: "Groceries", amount: 42, category: expense_category, date: Date.current) + create_transaction(account: account, name: "Salary", amount: -100, category: income_category, date: Date.current) + create_transaction(account: account, name: "Uncategorized cash", amount: 25, date: Date.current) + + get reports_path(period_type: :monthly, start_date: start_date, end_date: end_date) + assert_response :ok + + expense_href = transactions_path(q: { categories: [ expense_category.name ], start_date: start_date, end_date: end_date }) + income_href = transactions_path(q: { categories: [ income_category.name ], start_date: start_date, end_date: end_date }) + uncategorized_href = transactions_path(q: { categories: [ Category.uncategorized.name ], start_date: start_date, end_date: end_date }) + + assert_select "tr[data-category='category-#{expense_category.id}'] a[href=?]", expense_href, text: expense_category.name + assert_select "tr[data-category='category-#{income_category.id}'] a[href=?]", income_href, text: income_category.name + assert_select "tr[data-category='category-uncategorized'] a[href=?]", uncategorized_href, text: Category.uncategorized.name + + # Full-row hit target via stretched ::before (mirrors dashboard outflows) + assert_select "tr.relative.group\\/category-row[data-category='category-#{expense_category.id}'] a[class*='before:absolute'][class*='before:inset-0']" + end + + test "index uncategorized category link uses localized name that Search accepts" do + start_date = Date.current.beginning_of_month + end_date = Date.current.end_of_month + account = @family.accounts.first + create_transaction(account: account, name: "Uncategorized cash", amount: 25, date: Date.current) + + @user.update!(locale: "zh-CN") + localized_name = I18n.with_locale(:"zh-CN") { Category.uncategorized.name } + assert_includes Category.all_uncategorized_names, localized_name + + get reports_path(period_type: :monthly, start_date: start_date, end_date: end_date) + assert_response :ok + + href = transactions_path(q: { categories: [ localized_name ], start_date: start_date, end_date: end_date }) + assert_select "tr[data-category='category-uncategorized'] a[href=?]", href, text: localized_name + end + test "index excludes tax-advantaged account transactions from activity breakdown" do @family.accounts.each { |account| account.entries.destroy_all } @@ -451,11 +494,14 @@ class ReportsControllerTest < ActionDispatch::IntegrationTest assert_select "tr[data-category='category-#{taxable_category.id}']", text: /Reports Taxable Income/ assert_select "tr[data-category='category-#{retirement_category.id}']", count: 0 - other_investments_row = css_select("tr[data-category='category-other_investments']").first - assert_not_nil other_investments_row - assert_match(/#{Regexp.escape(Category.other_investments.name)}/, other_investments_row.text) - assert_match(/#{Regexp.escape(I18n.t("reports.transactions_breakdown.table.entries", count: 1))}/, other_investments_row.text) - assert_match(/\$100\.00/, other_investments_row.text) + other_investments_rows = css_select("tr[data-category='category-other_investments']") + assert_operator other_investments_rows.size, :>=, 1 + other_investments_rows.each do |row| + assert_match(/#{Regexp.escape(Category.other_investments.name)}/, row.text) + assert_equal 0, row.css("a").size + end + assert_match(/#{Regexp.escape(I18n.t("reports.transactions_breakdown.table.entries", count: 1))}/, other_investments_rows.first.text) + assert_match(/\$100\.00/, other_investments_rows.first.text) end test "monthly period navigation shows previous month link" do