mirror of
https://github.com/we-promise/sure.git
synced 2026-05-29 15:34:58 +00:00
fix(retirement): isolate retirement goals from savings goal routes
Addresses Codex P2 on #2044. A Goal::Retirement row lives in Current.family.goals, so the shared GoalsController and GoalPledgesController loaded it through `family.goals.find(...)` — never calling Goal::Retirement#editable_by?. Any preview-enabled family member could therefore open /goals/:id and edit/archive/delete another member's owner-scoped retirement plan, hit its pledge routes, and see it listed in the savings Goals grid. Adds `Goal.savings` (base type only) and scopes both savings controllers to it, so retirement goals are unreachable through the shared routes (RecordNotFound -> goals_path redirect) and absent from the savings index. Owner-only retirement access stays in RetirementController; editable_by? is retained for it. Tests: savings scope excludes retirement; retirement goal absent from goals index; show + pledge routes redirect not-found for retirement. (The Codex schema.rb null:false finding is a false positive — this branch's schema.rb retains null:false on all IBKR payload columns and the diff vs the base branch touches no IBKR lines; Codex compared against main rather than the PR base.)
This commit is contained in:
@@ -60,7 +60,7 @@ class GoalPledgesController < ApplicationController
|
||||
# Preload linked accounts + their providers so any_connected_account?
|
||||
# and the new-pledge form's per-account helpers don't trigger N+1
|
||||
# queries on account_providers.
|
||||
@goal = Current.family.goals
|
||||
@goal = Current.family.goals.savings
|
||||
.includes(:open_pledges, linked_accounts: :account_providers)
|
||||
.find(params[:goal_id])
|
||||
end
|
||||
|
||||
@@ -7,12 +7,12 @@ class GoalsController < ApplicationController
|
||||
ACTIVE_STATUS_RANK = { behind: 0, on_track: 1, no_target_date: 2 }.freeze
|
||||
|
||||
def index
|
||||
state_counts = Current.family.goals.group(:state).count
|
||||
state_counts = Current.family.goals.savings.group(:state).count
|
||||
@counts = STATE_FILTERS.each_with_object({}) do |state, h|
|
||||
h[state] = state == "all" ? state_counts.values.sum : (state_counts[state] || 0)
|
||||
end
|
||||
|
||||
all_goals = Current.family.goals
|
||||
all_goals = Current.family.goals.savings
|
||||
.alphabetically
|
||||
.includes(:open_pledges, linked_accounts: :account_providers)
|
||||
.to_a
|
||||
@@ -46,7 +46,7 @@ class GoalsController < ApplicationController
|
||||
end
|
||||
|
||||
def new
|
||||
@goal = Current.family.goals.new(
|
||||
@goal = Current.family.goals.savings.new(
|
||||
color: Goal::COLORS.sample,
|
||||
currency: Current.family.primary_currency_code
|
||||
)
|
||||
@@ -59,7 +59,7 @@ class GoalsController < ApplicationController
|
||||
end
|
||||
|
||||
def create
|
||||
@goal = Current.family.goals.new(goal_params)
|
||||
@goal = Current.family.goals.savings.new(goal_params)
|
||||
accounts = lookup_accounts(params.dig(:goal, :account_ids))
|
||||
@goal.currency = accounts.first.currency if accounts.any? && @goal.currency.blank?
|
||||
|
||||
@@ -148,7 +148,7 @@ class GoalsController < ApplicationController
|
||||
|
||||
private
|
||||
def set_goal
|
||||
@goal = Current.family.goals
|
||||
@goal = Current.family.goals.savings
|
||||
.includes(:open_pledges, linked_accounts: :account_providers)
|
||||
.find(params[:id])
|
||||
end
|
||||
|
||||
@@ -32,6 +32,12 @@ class Goal < ApplicationRecord
|
||||
scope :active_first, lambda {
|
||||
order(Arel.sql("CASE state WHEN 'active' THEN 0 WHEN 'paused' THEN 1 WHEN 'completed' THEN 2 ELSE 3 END"))
|
||||
}
|
||||
# Savings goals only — the base Goal type. STI subtypes such as
|
||||
# Goal::Retirement have their own surfaces (RetirementController) and
|
||||
# must NOT be reachable through the shared goal/pledge routes, where
|
||||
# any preview-enabled family member could otherwise view, edit, or
|
||||
# delete another member's owner-scoped retirement plan.
|
||||
scope :savings, -> { where(type: "Goal") }
|
||||
|
||||
def self.advisory_lock_key_for(family_id)
|
||||
Digest::SHA1.hexdigest("goals:family:#{family_id}").to_i(16) % (2**63)
|
||||
|
||||
@@ -91,4 +91,15 @@ class GoalPledgesControllerTest < ActionDispatch::IntegrationTest
|
||||
get new_goal_pledge_url(other_goal)
|
||||
assert_redirected_to goals_path
|
||||
end
|
||||
|
||||
test "pledge routes are not reachable for retirement goals" do
|
||||
retirement = Goal::Retirement.create!(
|
||||
family: @user.family, owner: @user,
|
||||
name: "Retire", target_amount: 1_000_000, currency: "USD"
|
||||
)
|
||||
|
||||
get new_goal_pledge_url(retirement), headers: { "Turbo-Frame" => "modal" }
|
||||
|
||||
assert_redirected_to goals_path
|
||||
end
|
||||
end
|
||||
|
||||
@@ -239,4 +239,28 @@ class GoalsControllerTest < ActionDispatch::IntegrationTest
|
||||
assert_redirected_to goals_path
|
||||
assert_equal I18n.t("goals.errors.not_found"), flash[:alert]
|
||||
end
|
||||
|
||||
test "retirement goals are excluded from the savings index" do
|
||||
Goal::Retirement.create!(
|
||||
family: @user.family, owner: @user,
|
||||
name: "My Retirement Plan", target_amount: 1_000_000, currency: "USD"
|
||||
)
|
||||
|
||||
get goals_url
|
||||
|
||||
assert_response :success
|
||||
assert_no_match(/My Retirement Plan/, response.body)
|
||||
end
|
||||
|
||||
test "retirement goal is not reachable via the savings show route" do
|
||||
retirement = Goal::Retirement.create!(
|
||||
family: @user.family, owner: @user,
|
||||
name: "Retire", target_amount: 1_000_000, currency: "USD"
|
||||
)
|
||||
|
||||
get goal_url(retirement)
|
||||
|
||||
assert_redirected_to goals_path
|
||||
assert_equal I18n.t("goals.errors.not_found"), flash[:alert]
|
||||
end
|
||||
end
|
||||
|
||||
@@ -261,4 +261,15 @@ class GoalTest < ActiveSupport::TestCase
|
||||
assert_not @goal.editable_by?(foreign_user)
|
||||
assert_not @goal.editable_by?(nil)
|
||||
end
|
||||
|
||||
test "savings scope excludes retirement subtype" do
|
||||
retirement = Goal::Retirement.create!(
|
||||
family: @family, owner: users(:family_admin),
|
||||
name: "Retire", target_amount: 1_000_000, currency: "USD"
|
||||
)
|
||||
|
||||
savings_ids = Goal.savings.pluck(:id)
|
||||
assert_includes savings_ids, @goal.id
|
||||
assert_not_includes savings_ids, retirement.id
|
||||
end
|
||||
end
|
||||
|
||||
Reference in New Issue
Block a user