feat(goals/edit): funding-accounts editor in the edit modal

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.
This commit is contained in:
Guillem Arias
2026-05-11 20:28:45 +02:00
parent e179abd0b3
commit f4b360bb96
4 changed files with 91 additions and 11 deletions

View File

@@ -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)

View File

@@ -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") %>
<div>
<div class="mb-2">
<span class="block text-sm font-medium text-primary"><%= t("goals.form_stepper.step1.fields.funding_accounts") %></span>
<p class="text-xs text-secondary mt-0.5"><%= t("goals.form_stepper.step1.fields.funding_accounts_hint") %></p>
</div>
<div class="bg-container-inset rounded-lg p-1">
<% grouped = linkable_accounts.group_by { |a| a.subtype.to_s.presence || "other" } %>
<% grouped.each_with_index do |(subtype, accts), group_idx| %>
<div class="px-3 py-2 text-[11px] font-medium uppercase tracking-wide text-secondary"><%= t("goals.form_stepper.step1.subtypes.#{subtype}", default: subtype.titleize) %></div>
<div class="bg-container rounded-md <%= "mb-1" if group_idx < grouped.size - 1 %>">
<% accts.each_with_index do |account, idx| %>
<label class="flex items-center gap-3 px-3 py-2.5 cursor-pointer <%= "border-t border-subdued" if idx > 0 %>">
<%= check_box_tag "goal[account_ids][]",
account.id,
currently_linked_account_ids.include?(account.id.to_s),
id: "goal_account_ids_#{account.id}",
class: "checkbox checkbox--light shrink-0" %>
<%= render Goals::AvatarComponent.new(name: account.name, color: Goals::AvatarComponent.color_for(account.name), size: "md") %>
<div class="flex-1 min-w-0">
<p class="text-sm font-medium text-primary truncate"><%= account.name %></p>
<p class="text-xs text-secondary"><%= (account.subtype || subtype).titleize %></p>
</div>
<span class="text-sm text-primary tabular-nums"><%= Money.new(account.balance, account.currency).format %></span>
</label>
<% end %>
</div>
<% end %>
</div>
</div>
<div>
<span class="block text-sm text-secondary mb-2"><%= t("goals.form_stepper.step1.fields.color") %></span>
<div class="flex flex-wrap gap-2">

View File

@@ -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 %>

View File

@@ -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)