From 158e18cd0575199b5858ff272a5f50fd23231f6f Mon Sep 17 00:00:00 2001 From: lolimmlost Date: Tue, 3 Mar 2026 12:13:59 -0800 Subject: [PATCH] Add budget rollover: copy from previous month (#1100) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Add budget rollover: copy from previous month When navigating to an uninitialized budget month, show a prompt offering to copy amounts from the most recent initialized budget. Copies budgeted_spending, expected_income, and all matching category allocations. Also fixes over-allocation warning showing on uninitialized budgets. Co-Authored-By: Claude Opus 4.6 * Redirect copy_previous to categories wizard for review Matches the normal budget setup flow (edit → categories → show) so users can review/tweak copied allocations before confirming. Co-Authored-By: Claude Opus 4.6 * Address code review: eager-load categories, guard against overwrite - Add .includes(:budget_categories) to most_recent_initialized_budget to avoid N+1 when copy_from! iterates source categories - Guard copy_previous action against overwriting already-initialized budgets (prevents crafted POST from clobbering existing data) - Add i18n key for already_initialized flash message Co-Authored-By: Claude Opus 4.6 * Add invariant guards to copy_from! for defensive safety Validate that source budget belongs to the same family and precedes the target budget before copying. Protects against misuse from other callers beyond the controller. Co-Authored-By: Claude Opus 4.6 * Fix button overflow on small screens in copy previous prompt Stack buttons vertically on mobile, side-by-side on sm+ breakpoint. Co-Authored-By: Claude Opus 4.6 --------- Co-authored-by: Claude Opus 4.6 --- app/controllers/budgets_controller.rb | 19 +++- app/models/budget.rb | 30 ++++++ .../budgets/_copy_previous_prompt.html.erb | 26 +++++ app/views/budgets/show.html.erb | 4 +- config/locales/views/budgets/en.yml | 9 ++ config/routes.rb | 1 + test/models/budget_test.rb | 95 +++++++++++++++++++ 7 files changed, 182 insertions(+), 2 deletions(-) create mode 100644 app/views/budgets/_copy_previous_prompt.html.erb diff --git a/app/controllers/budgets_controller.rb b/app/controllers/budgets_controller.rb index 1ec8e81b6..837306ce7 100644 --- a/app/controllers/budgets_controller.rb +++ b/app/controllers/budgets_controller.rb @@ -1,11 +1,12 @@ class BudgetsController < ApplicationController - before_action :set_budget, only: %i[show edit update] + before_action :set_budget, only: %i[show edit update copy_previous] def index redirect_to_current_month_budget end def show + @source_budget = @budget.most_recent_initialized_budget unless @budget.initialized? end def edit @@ -17,6 +18,22 @@ class BudgetsController < ApplicationController redirect_to budget_budget_categories_path(@budget) end + def copy_previous + if @budget.initialized? + redirect_to budget_path(@budget), alert: t("budgets.copy_previous.already_initialized") + return + end + + source_budget = @budget.most_recent_initialized_budget + + if source_budget + @budget.copy_from!(source_budget) + redirect_to budget_budget_categories_path(@budget), notice: t("budgets.copy_previous.success", source_name: source_budget.name) + else + redirect_to budget_path(@budget), alert: t("budgets.copy_previous.no_source") + end + end + def picker render partial: "budgets/picker", locals: { family: Current.family, diff --git a/app/models/budget.rb b/app/models/budget.rb index c345801fc..aa8f900b8 100644 --- a/app/models/budget.rb +++ b/app/models/budget.rb @@ -126,6 +126,36 @@ class Budget < ApplicationRecord budgeted_spending.present? end + def most_recent_initialized_budget + family.budgets + .includes(:budget_categories) + .where("start_date < ?", start_date) + .where.not(budgeted_spending: nil) + .order(start_date: :desc) + .first + end + + def copy_from!(source_budget) + raise ArgumentError, "source budget must belong to the same family" unless source_budget.family_id == family_id + raise ArgumentError, "source budget must precede target budget" unless source_budget.start_date < start_date + + Budget.transaction do + update!( + budgeted_spending: source_budget.budgeted_spending, + expected_income: source_budget.expected_income + ) + + target_by_category = budget_categories.index_by(&:category_id) + + source_budget.budget_categories.each do |source_bc| + target_bc = target_by_category[source_bc.category_id] + next unless target_bc + + target_bc.update!(budgeted_spending: source_bc.budgeted_spending) + end + end + end + def income_category_totals income_totals.category_totals.reject { |ct| ct.category.subcategory? || ct.total.zero? }.sort_by(&:weight).reverse end diff --git a/app/views/budgets/_copy_previous_prompt.html.erb b/app/views/budgets/_copy_previous_prompt.html.erb new file mode 100644 index 000000000..8edd87c1d --- /dev/null +++ b/app/views/budgets/_copy_previous_prompt.html.erb @@ -0,0 +1,26 @@ +<%# locals: (budget:, source_budget:) %> + +
+ <%= icon "copy", size: "lg" %> + +
+

<%= t("budgets.copy_previous_prompt.title") %>

+

<%= t("budgets.copy_previous_prompt.description", source_name: source_budget.name) %>

+
+ +
+ <%= render DS::Button.new( + text: t("budgets.copy_previous_prompt.copy_button", source_name: source_budget.name), + href: copy_previous_budget_path(budget), + method: :post, + icon: "copy" + ) %> + + <%= render DS::Link.new( + text: t("budgets.copy_previous_prompt.fresh_button"), + variant: "secondary", + icon: "plus", + href: edit_budget_path(budget) + ) %> +
+
diff --git a/app/views/budgets/show.html.erb b/app/views/budgets/show.html.erb index 33eea2d50..8b810538e 100644 --- a/app/views/budgets/show.html.erb +++ b/app/views/budgets/show.html.erb @@ -10,7 +10,9 @@
<%# Budget Donut %>
- <% if @budget.available_to_allocate.negative? %> + <% if !@budget.initialized? && @source_budget.present? %> + <%= render "budgets/copy_previous_prompt", budget: @budget, source_budget: @source_budget %> + <% elsif @budget.initialized? && @budget.available_to_allocate.negative? %> <%= render "budgets/over_allocation_warning", budget: @budget %> <% else %> <%= render "budgets/budget_donut", budget: @budget %> diff --git a/config/locales/views/budgets/en.yml b/config/locales/views/budgets/en.yml index 6f98a5686..c727dc37c 100644 --- a/config/locales/views/budgets/en.yml +++ b/config/locales/views/budgets/en.yml @@ -8,3 +8,12 @@ en: tabs: actual: Actual budgeted: Budgeted + copy_previous_prompt: + title: "Set up your budget" + description: "You can copy your budget from %{source_name} or start fresh." + copy_button: "Copy from %{source_name}" + fresh_button: "Start fresh" + copy_previous: + success: "Budget copied from %{source_name}" + no_source: "No previous budget found to copy from" + already_initialized: "This budget has already been set up" diff --git a/config/routes.rb b/config/routes.rb index 1e5097fd2..f5e2fb767 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -211,6 +211,7 @@ Rails.application.routes.draw do end resources :budgets, only: %i[index show edit update], param: :month_year do + post :copy_previous, on: :member get :picker, on: :collection resources :budget_categories, only: %i[index show update] diff --git a/test/models/budget_test.rb b/test/models/budget_test.rb index cd3e95307..1896e887a 100644 --- a/test/models/budget_test.rb +++ b/test/models/budget_test.rb @@ -199,6 +199,101 @@ class BudgetTest < ActiveSupport::TestCase assert_equal 150, spending_without_refund - spending_with_refund end + test "most_recent_initialized_budget returns latest initialized budget before this one" do + family = families(:dylan_family) + + # Create an older initialized budget (2 months ago) + older_budget = Budget.create!( + family: family, + start_date: 2.months.ago.beginning_of_month, + end_date: 2.months.ago.end_of_month, + budgeted_spending: 3000, + expected_income: 5000, + currency: "USD" + ) + + # Create a middle uninitialized budget (1 month ago) + Budget.create!( + family: family, + start_date: 1.month.ago.beginning_of_month, + end_date: 1.month.ago.end_of_month, + currency: "USD" + ) + + current_budget = Budget.find_or_bootstrap(family, start_date: Date.current) + + assert_equal older_budget, current_budget.most_recent_initialized_budget + end + + test "most_recent_initialized_budget returns nil when none exist" do + family = families(:empty) + budget = Budget.create!( + family: family, + start_date: Date.current.beginning_of_month, + end_date: Date.current.end_of_month, + currency: "USD" + ) + + assert_nil budget.most_recent_initialized_budget + end + + test "copy_from copies budgeted_spending expected_income and matching category budgets" do + family = families(:dylan_family) + + # Use past months to avoid fixture conflict (fixture :one is at Date.current for dylan_family) + source_budget = Budget.find_or_bootstrap(family, start_date: 2.months.ago) + source_budget.update!(budgeted_spending: 4000, expected_income: 6000) + source_bc = source_budget.budget_categories.find_by(category: categories(:food_and_drink)) + source_bc.update!(budgeted_spending: 500) + + target_budget = Budget.find_or_bootstrap(family, start_date: 1.month.ago) + assert_nil target_budget.budgeted_spending + + target_budget.copy_from!(source_budget) + target_budget.reload + + assert_equal 4000, target_budget.budgeted_spending + assert_equal 6000, target_budget.expected_income + + target_bc = target_budget.budget_categories.find_by(category: categories(:food_and_drink)) + assert_equal 500, target_bc.budgeted_spending + end + + test "copy_from skips categories that dont exist in target" do + family = families(:dylan_family) + + source_budget = Budget.find_or_bootstrap(family, start_date: 2.months.ago) + source_budget.update!(budgeted_spending: 4000, expected_income: 6000) + + # Create a category only in the source budget + temp_category = Category.create!(name: "Temp #{Time.now.to_f}", family: family, color: "#aaa", classification: "expense") + source_budget.budget_categories.create!(category: temp_category, budgeted_spending: 100, currency: "USD") + + target_budget = Budget.find_or_bootstrap(family, start_date: 1.month.ago) + + # Should not raise even though target doesn't have the temp category + assert_nothing_raised { target_budget.copy_from!(source_budget) } + assert_equal 4000, target_budget.reload.budgeted_spending + end + + test "copy_from leaves new categories at zero" do + family = families(:dylan_family) + + source_budget = Budget.find_or_bootstrap(family, start_date: 2.months.ago) + source_budget.update!(budgeted_spending: 4000, expected_income: 6000) + + target_budget = Budget.find_or_bootstrap(family, start_date: 1.month.ago) + + # Add a new category only to the target + new_category = Category.create!(name: "New #{Time.now.to_f}", family: family, color: "#bbb", classification: "expense") + target_budget.budget_categories.create!(category: new_category, budgeted_spending: 0, currency: "USD") + + target_budget.copy_from!(source_budget) + + new_bc = target_budget.budget_categories.find_by(category: new_category) + assert_equal 0, new_bc.budgeted_spending + end + test "previous_budget_param returns param when date is valid" do budget = Budget.create!( family: @family,