diff --git a/app/controllers/reports_controller.rb b/app/controllers/reports_controller.rb index 2688d0b1e..57791639a 100644 --- a/app/controllers/reports_controller.rb +++ b/app/controllers/reports_controller.rb @@ -25,9 +25,6 @@ class ReportsController < ApplicationController # Calculate summary metrics @summary_metrics = build_summary_metrics - # Build comparison data - @comparison_data = build_comparison_data - # Build trend data (last 6 months) @trends_data = build_trends_data @@ -195,25 +192,6 @@ class ReportsController < ApplicationController nil end - def build_comparison_data - currency_symbol = Money::Currency.new(Current.family.currency).symbol - - # Totals are BigDecimal amounts in dollars - pass directly to Money.new() - { - current: { - income: @current_income_totals.total, - expenses: @current_expense_totals.total, - net: @current_income_totals.total - @current_expense_totals.total - }, - previous: { - income: @previous_income_totals.total, - expenses: @previous_expense_totals.total, - net: @previous_income_totals.total - @previous_expense_totals.total - }, - currency_symbol: currency_symbol - } - end - def build_trends_data # Generate month-by-month data based on the current period filter trends = [] diff --git a/app/models/lunchflow_item/importer.rb b/app/models/lunchflow_item/importer.rb index c0fbe94d9..56104c157 100644 --- a/app/models/lunchflow_item/importer.rb +++ b/app/models/lunchflow_item/importer.rb @@ -29,8 +29,12 @@ class LunchflowItem::Importer accounts_failed = 0 if accounts_data[:accounts].present? - # Get all existing lunchflow account IDs for this item (normalize to strings for comparison) - existing_account_ids = lunchflow_item.lunchflow_accounts.pluck(:account_id).map(&:to_s) + # Get only linked lunchflow account IDs (ones actually imported/used by the user) + # This prevents updating orphaned accounts from old behavior that saved everything + existing_account_ids = lunchflow_item.lunchflow_accounts + .joins(:account_provider) + .pluck(:account_id) + .map(&:to_s) accounts_data[:accounts].each do |account_data| account_id = account_data[:id]&.to_s diff --git a/app/views/reports/_comparison_chart.html.erb b/app/views/reports/_comparison_chart.html.erb deleted file mode 100644 index c48531eb9..000000000 --- a/app/views/reports/_comparison_chart.html.erb +++ /dev/null @@ -1,250 +0,0 @@ -<% - currency = Current.family.currency - - # Helper to calculate percentage change and determine if it's good or bad - def comparison_class(current, previous, inverse: false) - return "text-primary" if previous.zero? - - change = current - previous - is_positive_change = change > 0 - - # For expenses, lower is better (inverse logic) - is_good = inverse ? !is_positive_change : is_positive_change - - is_good ? "text-green-600" : "text-gray-600" - end - - def percentage_change(current, previous) - return 0 if previous.zero? - ((current - previous) / previous.abs * 100).round(1) - end -%> - -
-
-

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

-

- <%= t("reports.comparison.currency", symbol: comparison_data[:currency_symbol]) %> -

-
- -
- <%# Income Comparison %> -
-
-

- <%= icon("trending-up", class: "w-4 h-4 text-success") %> - <%= t("reports.comparison.income") %> -

-
- -
-
- - <%= Money.new(comparison_data[:current][:income], currency).format %> - - <% change = percentage_change(comparison_data[:current][:income], comparison_data[:previous][:income]) %> - <% if change != 0 %> - <% income_improved = comparison_data[:current][:income] > comparison_data[:previous][:income] %> -
- - <%= change >= 0 ? "+" : "" %><%= change %>% - - - <%= icon(income_improved ? "trending-up" : "trending-down", class: "w-3 h-3") %> - <%= t(income_improved ? "reports.comparison.status.improved" : "reports.comparison.status.decreased") %> - -
- <% end %> -
- - <%= t("reports.comparison.previous") %>: <%= Money.new(comparison_data[:previous][:income], currency).format %> - -
- - <%# Overlapping bars %> -
- <% - current_income_abs = comparison_data[:current][:income].to_f.abs - previous_income_abs = comparison_data[:previous][:income].to_f.abs - max_income = [current_income_abs, previous_income_abs].max - - if max_income > 0 - current_width = [3, (current_income_abs / max_income * 100)].max - previous_width = [3, (previous_income_abs / max_income * 100)].max - else - current_width = 0 - previous_width = 0 - end - - # Income: green if increased, gray/primary if decreased - income_increased = comparison_data[:current][:income] >= comparison_data[:previous][:income] - income_bar_color = income_increased ? "bg-green-500" : "bg-gray-600" - income_bg_color = income_increased ? "bg-green-200" : "bg-gray-300" - %> - <% if previous_width > 0 || current_width > 0 %> - <%# Previous period bar (background) %> - <% if previous_width > 0 %> -
- <% end %> - <%# Current period bar (foreground) %> - <% if current_width > 0 %> -
- <% end %> - <% else %> -
- <%= t("reports.comparison.no_data") %> -
- <% end %> -
-
- - <%# Expenses Comparison %> -
-
-

- <%= icon("trending-down", class: "w-4 h-4 text-danger") %> - <%= t("reports.comparison.expenses") %> -

-
- -
-
- - <%= Money.new(comparison_data[:current][:expenses], currency).format %> - - <% change = percentage_change(comparison_data[:current][:expenses], comparison_data[:previous][:expenses]) %> - <% if change != 0 %> - <% expenses_improved = comparison_data[:current][:expenses] < comparison_data[:previous][:expenses] %> -
- - <%= change >= 0 ? "+" : "" %><%= change %>% - - - <%= icon(expenses_improved ? "trending-down" : "trending-up", class: "w-3 h-3") %> - <%= t(expenses_improved ? "reports.comparison.status.reduced" : "reports.comparison.status.increased") %> - -
- <% end %> -
- - <%= t("reports.comparison.previous") %>: <%= Money.new(comparison_data[:previous][:expenses], currency).format %> - -
- - <%# Overlapping bars %> -
- <% - current_expenses_abs = comparison_data[:current][:expenses].to_f.abs - previous_expenses_abs = comparison_data[:previous][:expenses].to_f.abs - max_expenses = [current_expenses_abs, previous_expenses_abs].max - - if max_expenses > 0 - current_width = [3, (current_expenses_abs / max_expenses * 100)].max - previous_width = [3, (previous_expenses_abs / max_expenses * 100)].max - else - current_width = 0 - previous_width = 0 - end - - # Expenses: green if decreased (inverse logic), gray/primary if increased - expenses_decreased = comparison_data[:current][:expenses] <= comparison_data[:previous][:expenses] - expenses_bar_color = expenses_decreased ? "bg-green-500" : "bg-gray-600" - expenses_bg_color = expenses_decreased ? "bg-green-200" : "bg-gray-300" - %> - <% if previous_width > 0 || current_width > 0 %> - <%# Previous period bar (background) %> - <% if previous_width > 0 %> -
- <% end %> - <%# Current period bar (foreground) %> - <% if current_width > 0 %> -
- <% end %> - <% else %> -
- <%= t("reports.comparison.no_data") %> -
- <% end %> -
-
- - <%# Net Savings Comparison %> -
-
-

- <%= icon("piggy-bank", class: "w-4 h-4 text-primary") %> - <%= t("reports.comparison.net_savings") %> -

-
- -
-
- - <%= Money.new(comparison_data[:current][:net], currency).format %> - - <% change = percentage_change(comparison_data[:current][:net], comparison_data[:previous][:net]) %> - <% if change != 0 %> - <% net_improved = comparison_data[:current][:net] > comparison_data[:previous][:net] %> -
- - <%= change >= 0 ? "+" : "" %><%= change %>% - - - <%= icon(net_improved ? "trending-up" : "trending-down", class: "w-3 h-3") %> - <%= t(net_improved ? "reports.comparison.status.improved" : "reports.comparison.status.decreased") %> - -
- <% end %> -
- - <%= t("reports.comparison.previous") %>: <%= Money.new(comparison_data[:previous][:net], currency).format %> - -
- - <%# Overlapping bars %> -
- <% - current_net_abs = comparison_data[:current][:net].to_f.abs - previous_net_abs = comparison_data[:previous][:net].to_f.abs - max_net = [current_net_abs, previous_net_abs].max - - if max_net > 0 - current_width = [3, (current_net_abs / max_net * 100)].max - previous_width = [3, (previous_net_abs / max_net * 100)].max - else - current_width = 0 - previous_width = 0 - end - - # Net Savings: green if improved (increased), gray/primary if got worse - net_improved = comparison_data[:current][:net] >= comparison_data[:previous][:net] - net_bar_color = net_improved ? "bg-green-500" : "bg-gray-600" - net_bg_color = net_improved ? "bg-green-200" : "bg-gray-300" - %> - <% if previous_width > 0 || current_width > 0 %> - <%# Previous period bar (background) %> - <% if previous_width > 0 %> -
- <% end %> - <%# Current period bar (foreground) %> - <% if current_width > 0 %> -
- <% end %> - <% else %> -
- <%= t("reports.comparison.no_data") %> -
- <% end %> -
-
-
-
diff --git a/app/views/reports/index.html.erb b/app/views/reports/index.html.erb index dd659bf87..6a75e28d4 100644 --- a/app/views/reports/index.html.erb +++ b/app/views/reports/index.html.erb @@ -87,15 +87,6 @@ } %> - <%# Comparison Chart %> -
- <%= render partial: "reports/comparison_chart", locals: { - comparison_data: @comparison_data, - period_type: @period_type, - start_date: @start_date - } %> -
- <%# Trends & Insights %>
<%= render partial: "reports/trends_insights", locals: { diff --git a/config/locales/views/reports/en.yml b/config/locales/views/reports/en.yml index 3b4e952e2..87dd04b9a 100644 --- a/config/locales/views/reports/en.yml +++ b/config/locales/views/reports/en.yml @@ -24,20 +24,6 @@ en: income_minus_expenses: Income minus expenses of_budget_used: of budget used no_budget_data: No budget data for this period - comparison: - title: Period Comparison - currency: "Currency: %{symbol}" - income: Income - expenses: Expenses - net_savings: Net Savings - current: Current Period - previous: Previous Period - no_data: No data available - status: - improved: Improved - decreased: Decreased - reduced: Reduced - increased: Increased budget_performance: title: Budget Performance spent: Spent diff --git a/test/controllers/reports_controller_test.rb b/test/controllers/reports_controller_test.rb index c2e7a8cb2..4c454bf6a 100644 --- a/test/controllers/reports_controller_test.rb +++ b/test/controllers/reports_controller_test.rb @@ -76,14 +76,6 @@ class ReportsControllerTest < ActionDispatch::IntegrationTest assert_select "h3", text: I18n.t("reports.summary.net_savings") end - test "index builds comparison data" do - get reports_path(period_type: :monthly) - assert_response :ok - assert_select "h2", text: I18n.t("reports.comparison.title") - assert_select "h3", text: I18n.t("reports.comparison.income") - assert_select "h3", text: I18n.t("reports.comparison.expenses") - end - test "index builds trends data" do get reports_path(period_type: :monthly) assert_response :ok diff --git a/test/models/period_test.rb b/test/models/period_test.rb index b7e3e4f5d..fa1221eb9 100644 --- a/test/models/period_test.rb +++ b/test/models/period_test.rb @@ -45,25 +45,11 @@ class PeriodTest < ActiveSupport::TestCase assert_equal "Custom Period", period.label end - test "comparison_label returns correct label for known period" do - period = Period.from_key("last_30_days") - assert_equal "vs. last month", period.comparison_label - end - - test "comparison_label returns date range for unknown period" do - start_date = Date.current - 15.days - end_date = Date.current - period = Period.new(start_date: start_date, end_date: end_date) - expected = "#{start_date.strftime("%b %d, %Y")} to #{end_date.strftime("%b %d, %Y")}" - assert_equal expected, period.comparison_label - end - test "all_time period can be created" do period = Period.from_key("all_time") assert_equal "all_time", period.key assert_equal "All Time", period.label assert_equal "All", period.label_short - assert_equal "vs. beginning", period.comparison_label end test "all_time period uses family's oldest entry date" do