mirror of
https://github.com/we-promise/sure.git
synced 2026-05-29 23:39:03 +00:00
fix(goals): apply CodeRabbit findings
- Switch the goal_accounts → accounts FK from on_delete: :cascade to
:restrict. `Goal#must_have_at_least_one_linked_account` is enforced
at write time; the cascade let a raw DELETE silently orphan a Goal
whose only link pointed at the deleted account. Normal Rails
Account#destroy still cleans up via `dependent: :destroy`, but the
restrict guarantees the DB rejects any path that bypasses the
association.
- projection_payload: required_monthly is now monthly_target_amount&.to_f
so open-ended (no-target-date) goals serialize required_monthly: null
instead of 0, matching the absence of a required pace.
- index page + sidebar nav-rail dot now read the Beta label via
t("shared.beta") (and a new shared.beta locale key) instead of the
hardcoded "Beta" literal.
- _status_callout uses the view-helper t(...) instead of I18n.t(...)
for the status label so it follows the same convention as the rest
of the goals views.
- goal_projection_chart: read the computed style before stamping
position: relative so a stylesheet-defined position (fixed/sticky/
absolute) isn't clobbered.
- preview-deploy: add `set -euo pipefail` around the wrangler
container lookup so a curl/jq failure fails the job instead of
producing an empty CONTAINER_ID and silently skipping cleanup.
This commit is contained in:
1
.github/workflows/preview-deploy.yml
vendored
1
.github/workflows/preview-deploy.yml
vendored
@@ -88,6 +88,7 @@ jobs:
|
||||
CLOUDFLARE_ACCOUNT_ID: ${{ secrets.CLOUDFLARE_ACCOUNT_ID }}
|
||||
working-directory: workers/preview
|
||||
run: |
|
||||
set -euo pipefail
|
||||
CONTAINER_NAME="sure-preview-${{ github.event.pull_request.number }}-railscontainer"
|
||||
echo "Looking for stale preview container app: $CONTAINER_NAME"
|
||||
|
||||
|
||||
@@ -433,7 +433,12 @@ export default class extends Controller {
|
||||
.attr("pointer-events", "none")
|
||||
.style("display", "none");
|
||||
|
||||
if (root.style.position !== "absolute") root.style.position = "relative";
|
||||
// Only promote root to a positioned ancestor when it currently has no
|
||||
// positioning context. Inline checks against `root.style.position`
|
||||
// miss positions set via CSS (the inline style is empty), so we'd
|
||||
// clobber a stylesheet `position: fixed/sticky/absolute` with our
|
||||
// own `relative`. Read the computed style instead.
|
||||
if (getComputedStyle(root).position === "static") root.style.position = "relative";
|
||||
const tooltip = document.createElement("div");
|
||||
tooltip.style.cssText = "position:absolute;pointer-events:none;display:none;background:var(--color-gray-900);color:var(--color-white);font-size:12px;line-height:1.35;padding:6px 8px;border-radius:6px;white-space:nowrap;z-index:5;box-shadow:0 2px 8px rgba(0,0,0,0.15);";
|
||||
root.appendChild(tooltip);
|
||||
|
||||
@@ -205,7 +205,7 @@ class Goal < ApplicationRecord
|
||||
currency_symbol: Money.new(0, currency).currency.symbol,
|
||||
current_amount: current_balance.to_f,
|
||||
avg_monthly: pace.to_f,
|
||||
required_monthly: monthly_target_amount.to_f,
|
||||
required_monthly: monthly_target_amount&.to_f,
|
||||
currency: currency,
|
||||
status: status.to_s,
|
||||
projection_end_value: proj_end.to_f,
|
||||
|
||||
@@ -18,7 +18,7 @@
|
||||
else "info"
|
||||
end
|
||||
|
||||
label = I18n.t("goals.status.#{goal.status}", default: goal.status.to_s.titleize)
|
||||
label = t("goals.status.#{goal.status}", default: goal.status.to_s.titleize)
|
||||
%>
|
||||
<div class="rounded-lg border px-3 py-2 text-sm flex items-center gap-2 <%= variant_classes %>">
|
||||
<span class="shrink-0"><%= icon(icon_glyph, size: "sm") %></span>
|
||||
|
||||
@@ -2,7 +2,7 @@
|
||||
<header>
|
||||
<div class="flex items-center gap-2">
|
||||
<h1 class="text-2xl font-semibold text-primary"><%= t(".title") %></h1>
|
||||
<%= render DS::Pill.new(label: "Beta", size: :md) %>
|
||||
<%= render DS::Pill.new(label: t("shared.beta"), size: :md) %>
|
||||
</div>
|
||||
<p class="text-sm text-secondary mt-1"><%= t(".subtitle") %></p>
|
||||
</header>
|
||||
|
||||
@@ -11,7 +11,7 @@
|
||||
<%= icon(icon, color: active ? "current" : "default", custom: icon_custom) %>
|
||||
<% if beta %>
|
||||
<span class="absolute -top-0.5 -right-0.5">
|
||||
<%= render DS::Pill.new(tone: :violet, dot_only: true, title: "Beta") %>
|
||||
<%= render DS::Pill.new(tone: :violet, dot_only: true, title: t("shared.beta")) %>
|
||||
</span>
|
||||
<% end %>
|
||||
<% end %>
|
||||
|
||||
@@ -4,6 +4,7 @@ en:
|
||||
self_hostable:
|
||||
redis_configured: "Redis is now configured properly! You can now setup your Sure application."
|
||||
shared:
|
||||
beta: Beta
|
||||
confirm_modal:
|
||||
accept: Confirm
|
||||
body_html: "<p>You will not be able to undo this decision</p>"
|
||||
|
||||
@@ -0,0 +1,19 @@
|
||||
class RestrictGoalAccountsAccountFk < ActiveRecord::Migration[7.2]
|
||||
# Goal#must_have_at_least_one_linked_account is enforced at write time
|
||||
# via model validation, but the original goal_accounts → accounts FK
|
||||
# was on_delete: :cascade. Deleting a linked account silently destroys
|
||||
# the goal_account row, and a Goal whose only link points at that
|
||||
# account ends up with zero linked accounts — the model invariant the
|
||||
# validation was meant to guarantee. Flip the FK to :restrict so the
|
||||
# DB rejects the deletion. Callers (Account#destroy paths) must detach
|
||||
# the account from goals first.
|
||||
def up
|
||||
remove_foreign_key :goal_accounts, :accounts
|
||||
add_foreign_key :goal_accounts, :accounts, on_delete: :restrict
|
||||
end
|
||||
|
||||
def down
|
||||
remove_foreign_key :goal_accounts, :accounts
|
||||
add_foreign_key :goal_accounts, :accounts, on_delete: :cascade
|
||||
end
|
||||
end
|
||||
8
db/schema.rb
generated
8
db/schema.rb
generated
@@ -10,7 +10,7 @@
|
||||
#
|
||||
# It's strongly recommended that you check this file into your version control system.
|
||||
|
||||
ActiveRecord::Schema[7.2].define(version: 2026_05_17_122500) do
|
||||
ActiveRecord::Schema[7.2].define(version: 2026_05_18_192904) do
|
||||
# These are extensions that must be enabled in order to support this database
|
||||
enable_extension "pgcrypto"
|
||||
enable_extension "plpgsql"
|
||||
@@ -91,9 +91,9 @@ ActiveRecord::Schema[7.2].define(version: 2026_05_17_122500) do
|
||||
t.check_constraint "match_confidence IS NULL OR match_confidence >= 0::numeric AND match_confidence <= 1::numeric", name: "chk_account_statements_match_confidence"
|
||||
t.check_constraint "parser_confidence IS NULL OR parser_confidence >= 0::numeric AND parser_confidence <= 1::numeric", name: "chk_account_statements_parser_confidence"
|
||||
t.check_constraint "period_start_on IS NULL OR period_end_on IS NULL OR period_start_on <= period_end_on", name: "chk_account_statements_period_order"
|
||||
t.check_constraint "review_status::text = ANY (ARRAY['unmatched'::character varying, 'linked'::character varying, 'rejected'::character varying]::text[])", name: "chk_account_statements_review_status"
|
||||
t.check_constraint "review_status::text = ANY (ARRAY['unmatched'::character varying::text, 'linked'::character varying::text, 'rejected'::character varying::text])", name: "chk_account_statements_review_status"
|
||||
t.check_constraint "source::text = 'manual_upload'::text", name: "chk_account_statements_source"
|
||||
t.check_constraint "upload_status::text = ANY (ARRAY['stored'::character varying, 'failed'::character varying]::text[])", name: "chk_account_statements_upload_status"
|
||||
t.check_constraint "upload_status::text = ANY (ARRAY['stored'::character varying::text, 'failed'::character varying::text])", name: "chk_account_statements_upload_status"
|
||||
end
|
||||
|
||||
create_table "accounts", id: :uuid, default: -> { "gen_random_uuid()" }, force: :cascade do |t|
|
||||
@@ -1969,7 +1969,7 @@ ActiveRecord::Schema[7.2].define(version: 2026_05_17_122500) do
|
||||
add_foreign_key "family_exports", "families"
|
||||
add_foreign_key "family_merchant_associations", "families"
|
||||
add_foreign_key "family_merchant_associations", "merchants"
|
||||
add_foreign_key "goal_accounts", "accounts", on_delete: :cascade
|
||||
add_foreign_key "goal_accounts", "accounts", on_delete: :restrict
|
||||
add_foreign_key "goal_accounts", "goals", on_delete: :cascade
|
||||
add_foreign_key "goal_pledges", "accounts", on_delete: :restrict
|
||||
add_foreign_key "goal_pledges", "goals", on_delete: :cascade
|
||||
|
||||
Reference in New Issue
Block a user