From f4b360bb96e209ee058e505a01cdae37a4dd9a2e Mon Sep 17 00:00:00 2001 From: Guillem Arias Date: Mon, 11 May 2026 20:28:45 +0200 Subject: [PATCH] feat(goals/edit): funding-accounts editor in the edit modal MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Previously a user who linked the wrong account at creation had to delete + recreate the goal. Now the edit modal carries the same funding-accounts checkbox group as Step 1 of the stepper, pre-checked with the goal's current links. - GoalsController#edit loads @linkable_accounts + @currently_linked_account_ids. - #update accepts account_ids; when supplied, runs the create / update inside a Goal.transaction and syncs linked accounts via sync_linked_accounts! (set-diff: destroy_all unselected goal_accounts, create the new ones). Validates at least one account before touching goal_accounts so the user gets a clean re-render. - Removing an account preserves the goal's existing contributions — GoalContribution#account_must_be_linked_to_goal only fires on save, so historical rows stay valid. - _form_edit partial accepts new locals; edit.html.erb threads them through. - 3 new controller tests: identity-only patch leaves links intact; account_ids patch replaces the link set; empty account_ids re-renders with error. --- app/controllers/goals_controller.rb | 49 ++++++++++++++++++----- app/views/goals/_form_edit.html.erb | 32 ++++++++++++++- app/views/goals/edit.html.erb | 2 +- test/controllers/goals_controller_test.rb | 19 +++++++++ 4 files changed, 91 insertions(+), 11 deletions(-) diff --git a/app/controllers/goals_controller.rb b/app/controllers/goals_controller.rb index 76461af72..70ae70c34 100644 --- a/app/controllers/goals_controller.rb +++ b/app/controllers/goals_controller.rb @@ -71,20 +71,39 @@ class GoalsController < ApplicationController end def edit + @linkable_accounts = linkable_accounts_for_new + @currently_linked_account_ids = @goal.goal_accounts.pluck(:account_id).map(&:to_s) end def update - if @goal.update(goal_update_params) - flash[:notice] = t(".success") - respond_to do |format| - format.html { redirect_to goal_path(@goal) } - format.turbo_stream do - render turbo_stream: turbo_stream.action(:redirect, goal_path(@goal)) - end - end - else + account_ids = params.dig(:goal, :account_ids) + accounts_supplied = !account_ids.nil? + accounts = accounts_supplied ? lookup_accounts(account_ids) : [] + + if accounts_supplied && accounts.empty? + @goal.errors.add(:base, :at_least_one_linked_account_required) + @linkable_accounts = linkable_accounts_for_new + @currently_linked_account_ids = @goal.goal_accounts.pluck(:account_id).map(&:to_s) render :edit, status: :unprocessable_entity + return end + + Goal.transaction do + @goal.update!(goal_update_params) + sync_linked_accounts!(@goal, accounts) if accounts_supplied + end + + flash[:notice] = t(".success") + respond_to do |format| + format.html { redirect_to goal_path(@goal) } + format.turbo_stream do + render turbo_stream: turbo_stream.action(:redirect, goal_path(@goal)) + end + end + rescue ActiveRecord::RecordInvalid + @linkable_accounts = linkable_accounts_for_new + @currently_linked_account_ids = @goal.goal_accounts.pluck(:account_id).map(&:to_s) + render :edit, status: :unprocessable_entity end def destroy @@ -148,6 +167,18 @@ class GoalsController < ApplicationController Current.family.accounts.where(accountable_type: "Depository").visible.alphabetically.to_a end + def sync_linked_accounts!(goal, accounts) + desired = accounts.map(&:id).to_set + current = goal.goal_accounts.pluck(:account_id).to_set + + (current - desired).each do |id| + goal.goal_accounts.where(account_id: id).destroy_all + end + (desired - current).each do |id| + goal.goal_accounts.create!(account_id: id) + end + end + def create_initial_contribution_if_provided!(goal, accounts) amount = params.dig(:goal, :initial_contribution_amount) account_id = params.dig(:goal, :initial_contribution_account_id) diff --git a/app/views/goals/_form_edit.html.erb b/app/views/goals/_form_edit.html.erb index d2cb87a90..b0cf4fa26 100644 --- a/app/views/goals/_form_edit.html.erb +++ b/app/views/goals/_form_edit.html.erb @@ -1,4 +1,4 @@ -<%# locals: (goal:) %> +<%# locals: (goal:, linkable_accounts:, currently_linked_account_ids:) %> <% if goal.errors.any? %> <%= render "shared/form_errors", model: goal %> @@ -20,6 +20,36 @@ <%= f.date_field :target_date, label: t("goals.form_stepper.step1.fields.target_date") %> +
+
+ <%= t("goals.form_stepper.step1.fields.funding_accounts") %> +

<%= t("goals.form_stepper.step1.fields.funding_accounts_hint") %>

+
+
+ <% grouped = linkable_accounts.group_by { |a| a.subtype.to_s.presence || "other" } %> + <% grouped.each_with_index do |(subtype, accts), group_idx| %> +
<%= t("goals.form_stepper.step1.subtypes.#{subtype}", default: subtype.titleize) %>
+
"> + <% accts.each_with_index do |account, idx| %> + + <% end %> +
+ <% end %> +
+
+
<%= t("goals.form_stepper.step1.fields.color") %>
diff --git a/app/views/goals/edit.html.erb b/app/views/goals/edit.html.erb index 8c7cab6bb..68d17ba25 100644 --- a/app/views/goals/edit.html.erb +++ b/app/views/goals/edit.html.erb @@ -1,6 +1,6 @@ <%= render DS::Dialog.new do |dialog| %> <% dialog.with_header(title: t(".heading")) %> <% dialog.with_body do %> - <%= render "form_edit", goal: @goal %> + <%= render "form_edit", goal: @goal, linkable_accounts: @linkable_accounts, currently_linked_account_ids: @currently_linked_account_ids %> <% end %> <% end %> diff --git a/test/controllers/goals_controller_test.rb b/test/controllers/goals_controller_test.rb index 7d905d4af..0df441dd6 100644 --- a/test/controllers/goals_controller_test.rb +++ b/test/controllers/goals_controller_test.rb @@ -104,6 +104,25 @@ class GoalsControllerTest < ActionDispatch::IntegrationTest assert_equal "Renamed", @goal.reload.name end + test "update without account_ids leaves linked accounts intact" do + before = @goal.goal_accounts.pluck(:account_id).sort + patch goal_url(@goal), params: { goal: { name: "Still here" } } + assert_redirected_to goal_path(@goal) + assert_equal before, @goal.reload.goal_accounts.pluck(:account_id).sort + end + + test "update with account_ids syncs linked accounts (add + remove)" do + patch goal_url(@goal), params: { goal: { account_ids: [ @connected.id ] } } + assert_redirected_to goal_path(@goal) + assert_equal [ @connected.id ], @goal.reload.goal_accounts.pluck(:account_id) + end + + test "update with empty account_ids re-renders with error" do + patch goal_url(@goal), params: { goal: { account_ids: [ "" ] } } + assert_response :unprocessable_entity + assert_not_empty @goal.reload.goal_accounts + end + test "pause/resume/complete/archive/unarchive flow" do fresh = goals(:emergency_fund) patch pause_goal_url(fresh)