* fix(simplefin): treat Vanguard/Fidelity cost_basis as total when needed
PR #1692 normalized SimpleFIN holdings cost_basis under the assumption
that the `cost_basis` / `basis` keys carry a per-share value (per the
SimpleFIN spec) and only `total_cost` / `value` carry a total position
cost. Vanguard and Fidelity violate the spec — they populate
`cost_basis` with the *total* (see the payload in #1182). After PR
#1692 those holdings get stored with cost_basis = total, and
Holding#calculate_trend then computes previous = qty × avg_cost, so the
"previous" value is inflated by a factor of qty and an entire
investment account renders a phantom return of roughly -(1 − 1/qty),
i.e. -97% to -99%.
Fix: sanity-check raw cost_basis against the holding's market share
price. Let share_price = market_value / qty; the geometric midpoint
between "raw is per-share" (raw ≈ share_price) and "raw is total"
(raw ≈ qty × share_price) is share_price × √qty. If raw is above the
midpoint it is divided by qty; otherwise it is kept as per-share.
Falls back to the pre-fix behaviour (trust the spec) when market_value
or qty is unavailable, so confidently-correct readings are never made
worse.
Verified against the reported Vanguard payload (qty=139, cost_basis=
22004.40, market_value=22626.42): normalize_cost_basis now returns
$158.31/share, matching 22004.40 / 139, and the phantom -99% return
collapses to a realistic ~+2.8%. Per-share readings ($45 cost on a $50
share price) remain untouched.
Closes#1718. Refs #1182, #1692.
* fixup: replace cost_basis heuristic with institution allowlist
Codex and @EdeAbreu23 flagged a real false-positive in the previous
geometric-midpoint heuristic: a legitimate per-share `cost_basis` on a
holding with a large unrealized loss (e.g. 100 shares with $100/share
basis now worth $5/share) trips `share_price × √qty` and gets divided
to $1/share — corrupting any standards-compliant brokerage with a big
loss.
Adopt @EdeAbreu23's safer shape:
- total_cost / value: always divide by qty (unchanged from #1692).
- cost_basis / basis: keep as-is by default.
- Only divide cost_basis / basis when the holding's SimpleFIN account
is connected to a known-misbehaving institution. Allowlist starts
with `vanguard` and `fidelity`, matched case-insensitively against
the account's stored org name and domain. Easy to extend as more
brokerages turn up.
Trades a small maintenance cost (curated list) for zero risk of
corrupting compliant providers.
Verified against five scenarios (all expected):
Vanguard total in cost_basis (allowlist) → +2.83%
Fidelity total in basis (allowlist) → +33.33%
Big-loss per-share (Codex case) → -95.0% (preserved)
Honest per-share, small loss → +11.11% (unchanged)
total_cost on any institution → +11.11% (unchanged)
---------
Co-authored-by: plind-junior <plind-junior@users.noreply.github.com>
* fix(enable-banking): gracefully skip PDNG fetch for ASPSPs that don't support it
Some banks reject the PDNG transaction status filter with a 422 validation
error, causing the entire account sync to fail including booked transactions.
Wrap the pending transaction fetch in a rescue block to catch
validation errors from the provider. If the ASPSP does not support
the "PDNG" status, the error is logged and the process continues
without pending transactions instead of failing the entire import.
* fix(enable-banking): gate PDNG fallback on transactionStatus error detail
Tighten the rescue added in the previous commit so it only silences
422s that explicitly mention transactionStatus in the API error body.
Any other validation error (bad date_from, malformed headers, etc.)
re-raises and fails the sync as before, preventing silent data loss.
Tests added for both branches: ASPSP-rejects-PDNG (success) and
unrelated-validation-error (failure).
* fix(enable-banking): clear pending flag and prevent stale re-import after auto-claim
When a booked transaction claims a pending entry via the amount/date heuristic
(find_pending_transaction), two bugs caused the entry to remain incorrectly pending
and the old pending transaction to reappear on subsequent syncs.
Bug 1: The extra["enable_banking"]["pending"] flag was never cleared on the claimed
entry. For simple booked transactions with nil extra the deep-merge path is skipped
entirely, so the pending badge persisted forever.
Bug 2: After the claim the old pending external_id (e.g. PDNG_123) stayed in the
stored raw_transactions_payload. The importer's C4 filter only removes pending
entries whose transaction_id matches a BOOK id — Enable Banking issues completely
different ids for pending vs booked transactions — so PDNG_123 was never pruned.
On the next sync find_or_initialize_by(PDNG_123) couldn't find the claimed entry
(now keyed as BOOK_456) and created a fresh pending duplicate with no category.
Fix: on claim, explicitly clear all providers' pending keys from extra in-memory,
and store the displaced pending external_id in extra["auto_claimed_pending_ids"].
The Processor now queries this field alongside manual_merge to build the excluded_ids
set, so the stale pending data is skipped on every future sync.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* fix(enable-banking): preserve pending date when claiming transactions
When a pending transaction is claimed by a booked transaction, the
original pending date is now preserved instead of being overwritten
by the booked transaction's date. This ensures historical accuracy
for transactions that were originally recorded on a different date.
---------
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
* feat(mobile): add mass delete for chats
Long-press any chat to enter selection mode, tap items to select/deselect,
use Select All to toggle all, then delete with a single confirmation.
Swipe-to-delete continues to work outside selection mode.
* fix(mobile): address PR review comments on mass delete
- Wrap each deleteChat call in its own try-catch so a single network
failure doesn't abort the entire Future.wait operation
- Add null-safe casting for deletedCount and failedIds in provider
- Fix misleading error snackbar copy ("Some chats could not be deleted"
implied partial failure; provider only returns false on total failure)
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
---------
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
OidcIdentity#sync_user_attributes! runs on every SSO sign-in and
overwrote user.first_name / user.last_name with whatever the IdP sent,
because the precedence was `auth.info.* || user.*` — the IdP always
won when it supplied a value. A user who edited their first name to
"Adam" inside Sure had it reset to the IdP value "Ben" on the next
login, while the last name only "stuck" when the IdP happened not to
return a last_name (#1103).
Swap the precedence to `user.* || auth.info.*` so the IdP fills only
when Sure has nothing on file (first link or admin-blanked field).
Edits inside Sure are then authoritative for every subsequent login.
The audit copy on the OidcIdentity record itself is unchanged, so the
IdP-reported name is still available for debugging.
Closes#1103.
Co-authored-by: plind-junior <plind-junior@users.noreply.github.com>
* make default of opening_balance_date_label is TODAY
* feat(i18n): add multi-language support for opening balance label
- Use `t("valuations.show.opening_balance")` for all opening balance display (list and detail views)
- Add or update `opening_balance` translation in all major languages under `config/locales/views/valuations/`
- Now "Opening balance" will be localized in all supported languages
* revert -2.years
* Update config/locales/views/valuations/es.yml
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Signed-off-by: Juan José Mata <jjmata@jjmata.com>
* Update config/locales/views/valuations/pt-BR.yml
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Signed-off-by: Juan José Mata <jjmata@jjmata.com>
* Fix indentation for opening_balance in ro.yml
Signed-off-by: Juan José Mata <jjmata@jjmata.com>
* Fix indentation for opening_balance in Turkish locale
Signed-off-by: Juan José Mata <jjmata@jjmata.com>
* Update zh-TW.yml
Signed-off-by: Juan José Mata <jjmata@jjmata.com>
---------
Signed-off-by: Juan José Mata <jjmata@jjmata.com>
Co-authored-by: Juan José Mata <jjmata@jjmata.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
DS::Dialog#close_button called I18n.t("common.close") but no
`common.close` key exists in any locale file, so every modal rendered
the literal string "Translation missing: en.common.close" as both the
`title` and `aria-label` of the X close button — visible to screen
readers and as a hover tooltip.
Switch to `ds.dialog.close` to mirror the existing `ds.alert.*`
namespace under config/locales/views/components/*.yml, and add the
English string. Other locales fall back to English (fallbacks=true in
config/application.rb) until translated.
Closes#1763.
Co-authored-by: plind-junior <plind-junior@users.noreply.github.com>
* feat(mobile): add suggested questions to empty chat screen
- New constants file (lib/constants/suggested_questions.dart) for the 4
suggested question chips, kept separate from screen logic with a clear
l10n upgrade path noted in comments
- Empty chat screen now shows a personalised greeting and tappable
OutlinedButton chips; tapping one pre-fills and sends the message
- Optimistic message insertion in ChatProvider.sendMessage so the user
message and typing indicator appear instantly on tap, with rollback on
failure
- Full AI response revealed only once polling detects stable content
(2 consecutive polls with no growth), preventing partial responses
from flashing on screen
- fetchChat stops any in-progress polling before fetching so a manual
refresh always shows the authoritative server response
- Fixed updateChatTitle silently wiping messages when the title-update
API response omits the messages array
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* fix(mobile): address PR review comments
- Extract _rollbackOptimisticMessage helper to eliminate duplicated
rollback logic in sendMessage failure and catch branches
- Replace raw 'Error: ${e.toString()}' user-facing strings with a
generic message; retain technical details via debugPrint in each
catch block
- Replace inline ternary in updateChatTitle with explicit if/else for
readability while preserving message-preservation behaviour
- Fix non-reactive AuthProvider read inside Consumer<ChatProvider>
builder (listen: false → listen: true) so greeting updates when
user's firstName changes
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
---------
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
* fix(binance): support CRYPTO: prefix and USD stablecoins
Holdings processors (CoinStats, Coinbase, Kraken, SimpleFIN, Lunchflow,
Binance) store crypto securities with a "CRYPTO:" prefix, but
Provider::BinancePublic#parse_ticker only accepted Binance-search-style
tickers like "BTCUSD". As a result, every fetched price for tickers
like CRYPTO:USDT, CRYPTO:USDC, CRYPTO:SOL, CRYPTO:TRUMP, CRYPTO:KAITO
failed with "Unsupported Binance ticker".
- Strip the CRYPTO: prefix in parse_ticker.
- Short-circuit USD-pegged stablecoins (USDT, USDC, BUSD, DAI, FDUSD,
TUSD, USDP, PYUSD) to a synthetic flat 1.0 USD price. Binance has no
self-pair (USDTUSDT is invalid), and the few stablecoin/USDT pairs
that do exist hover at ~1.0 with sub-cent noise.
- Default prefixed bare base assets (CRYPTO:SOL etc.) to the …USDT
pair (USD). Only when prefixed, so unprefixed garbage like BTCBNB /
BTCGBP still returns nil and the existing rejection tests still pass.
- fetch_security_info returns links: nil for stablecoins rather than a
broken /trade/ URL.
Closes#1441.
* fix(binance): strip CRYPTO: prefix in search_securities
Security::Resolver calls search_provider with the raw holdings-processor
symbol (CRYPTO:SOL, CRYPTO:USDT) before any price fetch. Without prefix
handling here, first-time crypto imports never resolve to an online
Binance security and the new stablecoin/prefix paths in parse_ticker
were unreachable for that flow.
- Strip CRYPTO: from the search query.
- Short-circuit USD stablecoins to a synthetic search result (no
exchangeInfo call, no Binance self-pair to find).
- Teach parse_ticker the "{stablecoin}USD" form produced by the
synthetic result so price fetches route to stablecoin_prices.
---------
Co-authored-by: plind-junior <plind-junior@users.noreply.github.com>
User-facing doc gains explicit 1xM (one goal, multiple accounts),
N goals on shared accounts, and overlap (NxM) sections, plus the
reallocation flow and "why is this goal behind?" diagnostic.
Mechanics doc is rewritten against the actual code on the branch:
file:line citations for current state, accurate corrections to the
prior draft (Sure uses Account AASM status not archived_at, no
Account#balance_at method, balance history via Balance::ChartSeriesBuilder
CTE, Transaction::TRANSFER_KINDS for pace exclusion, advisory-lock
pattern lifted from IdentifyRecurringTransactionsJob, partial-unique
index precedent from entries[external_id, source]), concrete migration
plan with seven steps, surface-by-surface STAY/CHANGE/DELETE verdict
on every component, view, and Stimulus controller, day-one
instrumentation events, and four pre-launch user tests.
User-facing doc and mechanics companion converged on an account-linked
model with a pledge layer. Surfaces the pledge-with-7-day-match mechanic,
proportional-to-remaining-need split default, "Reserved beyond balance"
framing, in-chart pending segment, two-clock rate limit, archive-in-place
account handling, months-of-runway for open-ended goals, and the
pre-launch user tests + day-one pledge instrumentation.
The user-facing note focuses on how a Goal's balance gets computed
and what changes for the user. The mechanics doc covers the schema,
pro-rata math, pace window, and the operational details engineers
need but most readers don't.
Captures the open question on the data model behind Goals
(account-linked vs free-form ledger vs tag-based) for community
review before PR #1757 merges.
* Constrain Lunchflow base URL to trusted endpoint
Prevent SSRF by ignoring user-provided Lunchflow base_url values unless they match the canonical Lunchflow HTTPS endpoint. Add model tests covering invalid host/scheme and valid canonicalization behavior.
* Linter
* Scope SnapTrade orphan cleanup to current family
Restrict orphaned user listing and deletion to SnapTrade user IDs that belong to the current family namespace. Add model tests to prevent cross-family enumeration/deletion regressions.
* Update test/models/snaptrade_item_test.rb
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Signed-off-by: Juan José Mata <jjmata@jjmata.com>
* test: fix snaptrade orphaned users assertion
* style: fix snaptrade test array spacing
---------
Signed-off-by: Juan José Mata <jjmata@jjmata.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: KiloClaw <kiloclaw@openclaw.ai>
Refs #895, discussion #1224.
Adds a "Mark as recurring" entry point on the transfer detail drawer
that creates a `RecurringTransaction` carrying both source and
destination accounts. The recurring index, settings toggle
(`recurring_transactions_disabled`), and projected upcoming feed all
light up automatically once the data shape is there.
Schema:
* `destination_account_id` nullable FK to accounts. `on_delete: :cascade`
matches #20251030172500's precedent for accounts FKs. The existing
`account_id` FK is widened to cascade in the same migration so
Family destruction with a recurring transfer doesn't FK-violate.
* Two predicate-partitioned partial unique indexes per shape:
non-transfer rows (`destination_account_id IS NULL`, original
5-column shape preserved) and transfer rows (6-column shape
including the destination). Postgres treats NULLs as distinct in
unique indexes, so widening would have broken non-transfer dedupe.
* Two CHECK constraints enforcing transfer invariants in PostgreSQL:
`chk_recurring_txns_transfer_requires_source` (destination implies
source) and `chk_recurring_txns_transfer_distinct_accounts`
(destination cannot equal source). Per CLAUDE.md "Enforce null
checks, unique indexes, and simple validations in the database
schema for PostgreSQL".
* `Account` gains an `inbound_recurring_transfers` inverse so the
destroy chain reaches both ends.
Controller / behaviour:
* `transfers#mark_as_recurring` mirrors `transactions#mark_as_recurring`:
i18n flashes (4 new keys: transfer_marked_as_recurring,
transfer_already_exists, transfer_creation_failed,
transfer_feature_disabled), `respond_to format.html`,
`redirect_back_or_to transactions_path`, server-side gate on
`recurring_transactions_disabled?`, and rescue both `RecordInvalid`
and `RecordNotUnique` for the race window between the dedupe
`find_by` and `create_from_transfer`. The `StandardError` rescue
now logs the exception (class, message, transfer/family/user ids)
before surfacing the generic flash so production failures aren't
context-less.
* `RecurringTransaction.accessible_by(user)` now requires
destination_account_id (when present) to be in the user's
accessible set, so a recurring transfer never leaks to a user
without access to BOTH endpoints.
* Model validation gains a `destination_account.blank?` branch in
`transfer_endpoints_consistent` so a dangling
`destination_account_id` (referenced row destroyed) surfaces as a
normal validation error instead of an FK exception on save.
* `Identifier` filter for transfer-kind transactions moved into SQL.
UI:
* Recurring index table and projected feed render transfer rows with
the existing letter-avatar and the row's `name` field
("Transfer to {destination}"). No special pill or icon -- every row
in `/recurring_transactions` is recurring by definition. Amount
column on transfers uses `text-secondary` (muted-but-live) instead
of the income/expense colour, since transfers are zero-net for the
family.
Out of scope (called out in the PR body):
* Auto-creation of future Transfer rows on a schedule
(discussion #1224's primary ask). Behaviour change vs the
current projection-only model.
* Auto-identification of recurring transfer pairs in `Identifier`.
* Frequency model richer than `expected_day_of_month`.
* `Cleaner` for recurring transfers (issue #1590 tracks this).
Tests:
* `RecurringTransaction#transfer?` predicate (with / without
destination).
* `transfer_endpoints_consistent`: rejects same source and
destination, rejects dangling destination_account_id, rejects
cross-family destination.
* `RecurringTransaction.create_from_transfer` happy path;
multi-currency variant stores source-side currency.
* `projected_entry` exposes source / destination on transfer rows.
* `Identifier` skips transfer-kind transactions; creates a pattern
from expense halves while ignoring co-resident transfer halves.
* Destroying the destination account cascades to inbound recurring
transfers (FK + AR association).
* Unique partial index still de-duplicates non-transfer rows after
the destination_account_id widening.
* `transfers#mark_as_recurring` happy path, idempotent on second
call, rejected when `recurring_transactions_disabled`.
Suite: 3261 / 0 / 0 / 24 on the latest upstream/main. Lint clean.
Brakeman clean.
Signed-off-by: Guillem Arias Fauste <gariasf@proton.me>
* fix(enable-banking): handle transactions missing transaction_id and entry_reference
Some ASPSPs omit both transaction_id and entry_reference from their transaction payloads, which is valid per the PSD2/Berlin Group spec. Previously, every such transaction raised an ArgumentError and was silently dropped during sync.
compute_external_id now falls back to a deterministic MD5 fingerprint (prefixed enable_banking_content_) derived from date, amount, currency, direction, counterparty, and remittance info. This fingerprint is stable across re-syncs, so duplicate imports are still correctly prevented. An ArgumentError is only raised for truly empty/unidentifiable payloads.
The importer is updated in three places to use compute_external_id
consistently: the pending pre-filter (before combining with booked),
the C4 stored-pending cleanup, and the new_transactions dedup. This means ID-less pending entries are now also removed when their settled booked counterpart arrives.
Tests cover compute_external_id directly (all 5 cases), end-to-end
fingerprint import, idempotency, and importer storage/dedup behaviour for ID-less transactions including the pending→booked settlement path.
* fix(enable-banking): implement dual-strategy matching for transaction settlement
When a stored pending row had only entry_reference (no transaction_id) and
the settled BOOK row arrived with a new transaction_id, compute_external_id
produced different fingerprints for each side (enable_banking_<ref> vs
enable_banking_<txn_id>). The fingerprint-only comparison introduced in the
previous commit never matched, leaving the stale pending entry in
raw_transactions_payload. Both rows were then imported as separate visible
transactions.
Restore a book_entry_refs set alongside book_fingerprints in both the
pending pre-filter and the C4 stored-pending cleanup. A pending entry is
now removed when either its fingerprint or its entry_reference matches a
booked counterpart — covering same-ID settlement, content-fingerprint
settlement, and the entry_reference cross-match settlement path.
Also updates the ArgumentError message in external_id to accurately
reflect that transaction_id, entry_reference, and content fingerprint
are all accepted identifiers, and aligns build_transaction_content_key
to use transaction_date as a fallback (matching compute_external_id).
Adds a regression test that stores a pending-only row and asserts it is removed when the booked counterpart arrives with a new transaction_id.
* fix(design-system): align DS::Alert icon with title
The icon was rendered at size 'sm' (w-4 h-4) and started at the very
top of the flex row (items-start without an offset), which optically
sat above the title's cap when the title was present and slightly
above the message baseline when it wasn't. The hand-rolled alerts
this PR replaced used 'w-5 h-5 mt-0.5' for exactly this reason —
restore the same combination in the component:
- size: sm -> md (w-4/h-4 -> w-5/h-5).
- class adds mt-0.5 so the icon's vertical center lines up with the
bold title's cap-height (and with the body baseline in the title-less
case).
No API change. Visual fix only.
Refs #1731
* fix(design-system): split DS::Alert into title-row + indented body
Replaces the items-start + margin-fudge approach with a two-row
layout that doesn't depend on icon-bounding-box vs text-cap-height
arithmetic:
- Title case: icon and bold title share a flex row with items-center,
so the icon's vertical centre lines up with the title's line. Body
(block content or message) renders below in a separate row, padded
by pl-8 (= icon md width + gap-3) so it indents under the title
text rather than under the icon.
- Block-only case (no title, no message — used by the alpha_vantage
rate-limit alert): keeps the items-start fallback with a small mt-0.5
on the icon so the cap of the first paragraph still sits near the
icon centre.
- Single-line message case: items-center between icon and message, no
fudge needed.
container_classes loses its 'flex items-start gap-3' base since the
outer div is no longer the flex container. Each branch declares its
own flex/items-* combination.
Refs #1731
* fix(design-system): a11y semantics + visual polish on DS::Alert
Builds on the title-row restructure with the items the design / a11y
review surfaced:
- live: keyword (default :none, accepts :status / :polite and
:alert / :assertive) maps to role="status" or role="alert" on the
outer div. Static, page-baked alerts (the migrated callsites in
#1731) keep the default :none and stay role-less. Dynamic surfaces
(flash, validation summaries appearing after a Turbo update) opt
into the live role they need.
- aria-labelledby on the outer div pointing at the title <p> so AT
picks the title as the alert's accessible name when one is set.
- Variant prefix in the title / message via an sr-only span. Screen
reader hears 'Warning: …', 'Error: …', etc.; sighted users see no
change. Variant labels live under ds.alert.variants.* in
config/locales/views/components/en.yml.
- Body text inside titled alerts now defaults to text-secondary
instead of text-primary, so hierarchy reads on weight + colour
rather than weight alone (Refactoring UI: hierarchy needs both).
Single-line message and block-only fallback keep text-primary
since there is no second tier.
- Icon size goes back from md (20px) to sm (16px) — proportionally
closer to text-sm body — and the items-center branches grow
-mt-0.5 to compensate for the cap-centre vs line-centre offset
that flex's items-center alone can't bridge.
- Title weight bumped from font-medium (500) to font-semibold (600)
for clearer prominence against the now-softer body.
No API breakage: existing callers passing only message:/title:/variant:
keep working. The new live: arg defaults to the correct value for
the static migration sites.
Refs #1731
* fix(design-system): drop aria-labelledby when alert has no role; revert body to text-primary
Two corrections after numerical contrast analysis and CodeRabbit feedback:
1. aria-labelledby was being emitted on every titled alert, but the
default live: :none leaves the outer <div> with no role. ARIA spec
only honours the labelling relationship on elements with a host
role, so on a generic <div> the attribute is invalid and
accessibility validators flag it. Now only emitted when aria_role
is set (live: :status or :alert). Static, page-baked callsites
stay role-less and label-less; dynamic callers that opt into a
live role get the proper accessible-name relationship.
2. text-secondary on bg-{variant}/10 in light mode lands at
~4.07-4.25:1 contrast — below WCAG AA's 4.5:1 for normal text.
Reverting the body wrapper to text-primary brings it back to
AAA (~15:1). Loses some of the Refactoring UI body-vs-title
colour hierarchy; the title's font-semibold weight + larger
optical mass against an otherwise plain body still reads as
hierarchy. Single-line message and block-only fallback already
used text-primary, so this just unifies the three branches.
The remaining contrast gap — text-success (green-600) icon on
bg-success/10 light surface at 2.77:1 — is documented in the PR
description; fixing it cleanly needs a token-level bump
(--color-success: green-600 -> green-700 in light mode) which is
out of scope for this PR.
Refs #1731
* fix(settings/providers): use DS::Alert title:+message: instead of inline content_tag
Three callsites added in #1710 passed block-level markup (`<p>`/`<h2>`)
through `message:` via `safe_join + content_tag`. The post-#1731 alert
template wraps `message:` in a `<p>`, which makes nesting a `<p>` or
`<h2>` invalid HTML — browsers auto-close the outer paragraph and the
indented body row collapses.
Each of the three is semantically a title + body pair, so swap them
to the proper `title:` + `message:` API. No new strings — the i18n
keys (`*.no_withdraw_title` / `_body`, `encryption_error.title` /
`.message`) already split that way; the inline assembly was the
artefact.
The encryption-error block loses an explicit `<h2>` wrapper around
the title; DS::Alert's title is a `<p>`. The visual hierarchy and
sr-only variant prefix are unchanged. Worth tracking heading semantics
as a follow-up against DS::Alert (a `heading_level:` arg) rather than
bringing back the manual markup.
* fix(design-system): make :destructive variant alias explicit in DS::Alert locale
Add `destructive: Error` to `ds.alert.variants` and drop the implicit
`:destructive -> :error` aliasing in `DS::Alert#variant_label`. Both the
locale file and the component now self-document the variant set; lookup
is direct, no conditional needed.
Per @jjmata review on #1734.
* Add mobile custom proxy headers
* Clear login placeholders on focus
Email/password fields ship with example values pre-filled. Tapping the
field now clears the placeholder so users don't have to delete it
manually. Skips clearing if the user has already edited the value.
* Push Configuration as a route from Sign in
Opening Configuration from the Sign in screen now uses Navigator.push
instead of toggling a state flag, so Android back returns to Sign in
instead of quitting the app. Saving the URL auto-pops the route.
* Address PR review on custom proxy headers
- Test Connection no longer leaves global ApiConfig headers mutated;
unsaved edits are restored in a finally block after the probe.
- _loadSavedUrl / _loadCustomHeaders wrap storage reads in try/catch and
always finish initialization with sensible defaults.
- Sanitization is now a single CustomProxyHeader.sanitize() reused by
ApiConfig.setCustomProxyHeaders and CustomProxyHeadersService.
- Brief comment on redactedValue explaining the length-obscuring design.
* Harden custom proxy header validation and load path
- validateValue now rejects ASCII control characters (CR/LF/tab/etc.)
to prevent header-injection via crafted values.
- loadHeaders moves the secure-storage read inside the try block so
platform exceptions are caught the same way JSON parse errors are.
Repro: index -> goal show (chart drawn) -> open edit modal in turbo
frame -> pick a new icon -> submit. Server responds with
turbo_stream.action(:redirect, goal_path). Turbo morphs the show
page, wiping the chart container's children, but Stimulus' connect()
isn't re-run on the morphed element so _draw never fires again. The
ResizeObserver doesn't help — the container's box dimensions are
unchanged.
Listen for turbo:render and turbo:frame-load on document and re-draw
when the container's SVG is missing. Cheap idempotent check
(querySelector('svg')) — no-op if the chart is already there.
Listeners cleaned up in disconnect().
Verified: same flow now lands on the show page with the chart fully
rendered (23 SVG children).
Issue: an on-track goal whose projected end is just above the target
showed two right-anchored labels stacked on top of each other —
"Target · $2,400" and the projection-end short value "$2.4K". The
projection dot already conveys "you'll hit the target on time"; the
extra label adds noise.
Now: when willHit AND |projDotY - y(targetAmount)| < 18px, skip the
projection-end label entirely. The colored dot at the target_date
keeps the visual cue.
Also refactor the y-axis label collision check from value-based
(within 5% of yMax) to pixel-based (within 18px of target's y),
matching the projection-end logic. When a y-tick is close to target,
the Target label drops into the y-axis column at that row (short
format) instead of right-edge full format. Either way, no two labels
ever stack within 18 vertical px.
Verified live: Wedding fund (on_track, projection ≈ target) → just
"Target · $2,400" + y-ticks, no "$2.4K". House downpayment (behind) →
"Target · $50,000" + "Short $12.3K" both retained (well separated).
Goals::AvatarComponent had `attr_reader :icon` which shadowed the
global `icon` view helper. Template called `icon(icon, size:, color:)`
which Ruby resolved against the attr-reader (zero-arity), throwing
"wrong number of arguments (given 2, expected 0)" the moment a goal
had a saved icon and the show page tried to render its avatar.
- Drop `:icon` from attr_reader; expose as `icon_name` instead.
- Template uses `helpers.icon(icon_name, ...)` matching the
Goals::StatusPillComponent pattern (other Goals VCs already use
`helpers.icon`).
Reproduced + verified live via Playwright: edit modal → pick an icon
→ save → show page renders the new avatar with the SVG. Same for
create flow (new modal → pick icon → step 2 → submit → show renders).
Avatar letter/icon now uses `--avatar-color` CSS variable + the new
`.goal-avatar` class. Light mode darkens the text to 55% color + 45%
black so pale palette entries (cyan-300, green-300) stay readable on
the 10%-mix tint over white (~4.5:1). Dark mode reverts to the full
brand color via [data-theme="dark"] .goal-avatar override so the text
doesn't disappear against the near-black tinted surface. Verified
live: #805dee renders as a darker oklab in light mode and full
rgb(128,93,238) in dark mode.
Picker popup compacted:
- 80 (320px) wide, max-h-[60vh] overflow-y-auto so it never spills
off-screen.
- Anchored below the avatar + horizontally centered to it (top-full
left-1/2 -translate-x-1/2) so it doesn't drift off to the right
edge of the form on narrow modals.
- Icon grid max-h-40 (160px, ~5 rows) with the in-house `scrollbar`
utility for a thin gray thumb that works in both themes.
- Section headers (Color / Icon) styled `uppercase tracking-wide`
for visual hierarchy.
Verified popup at 320x310px in edit modal, no vertical overflow.
Previously sat next to the name input via `flex items-start gap-3` so
the picker avatar competed with the input for horizontal space. Move
to its own row, centered (`flex justify-center`), positioned just
before the name field. Mirrors the categories form layout where the
avatar is the focal element above the name input.
Same change applied to the edit form: picker comes first, then name.
Stepper step 1 order is now: heading · picker · name · amount/date ·
funding accounts · notes.
These local-only audit screenshots + the report draft were staged
inadvertently in the previous commit. They belong to the development
session, not the feature branch.
User requested replacing the in-house color disclosure with the
categories color+icon popover. Done as a controller extraction so
categories and goals share one Stimulus controller (user's option:
"Extract a shared color_icon_picker_controller.js").
- `git mv` app/javascript/controllers/category_controller.js to
color_icon_picker_controller.js. Categories form + color_avatar
partial updated to use the new identifier (data-controller=
"color-icon-picker", target/action selectors renamed).
- Goal model gains an icon column (migration
20260511190000_add_icon_to_goals.rb) + ICONS = Category.icon_codes
+ inclusion validation. GoalsController permits :icon in
goal_params + goal_update_params.
- Goals::AvatarComponent now renders icon when present (falls back to
first-letter initial), and adopts the Categories tinted-bg + colored
-content style (bg = `color-mix(in oklab, COLOR 10%, transparent)`,
text/icon = COLOR). Matches the picker's live preview so what the
user sees during selection equals the saved state.
- New goals/_color_picker.html.erb mirrors categories/_form's popover:
avatar + pen overlay summary + popup with color row (+ rainbow
custom-hex trigger) + icon grid. Pickr / contrast validation / auto-
adjust all inherited from the shared controller.
- Stepper step 1 layout: drop the inline letter-avatar (data-goal-
stepper-target="avatarPreview") in favour of the picker avatar next
to the name input. Step 1's tail no longer renders a separate color
partial. Edit form passes icons local through.
Verified live: new goal modal renders 11 color radios (10 presets +
custom) + 141 icon radios + pen-summary; categories form still
operational (no console errors) under the renamed controller.
Lower half of the goal detail used to be: (stat row: monthly pace +
total contributions) + (bottom row: contributions list + funding
breakdown card). Two of those four pieces were redundant:
- Total Contributions stat duplicated the count badge that already
sits beside the Contributions heading below.
- Monthly Pace stat repeated the same numbers the catch-up alert
surfaces above and the chart subtitle reads.
Adopt the dashboard Balance Sheet pattern (app/views/pages/dashboard/_
balance_sheet.html.erb) for the funding widget: inline header with
total ("Funding accounts · $13,250"), thin gap-separated segment bar,
color-dot legend with percent, and a bg-container-inset table with the
shared `pages/dashboard/group_weight` 5-stick weight indicator + value
column.
New show.html.erb bottom: just two full-width sections — funding
widget, then chronological contributions list. Both rendered only when
the goal has contributions (matches the empty-state branch added
earlier).
Locale: goals.show.funding_table.{name, weight, value}.
The "Add $1,531.25" CTA used to open the contribution modal with an
empty amount field — label was a hint, not a default. Now passes the
catch-up amount via ?amount= and the contributions controller seeds
@contribution.amount from params. One click brings the user to the
modal already populated.
Adds a secondary text link below the primary CTA: "Or adjust your
target" → opens the edit modal (Turbo frame). Behavioural-econ choice
architecture: gives the rebaseline path explicitly so users who can't
realistically catch up don't feel forced into the contribution.
Trade-off: lets the alert respect autonomy — commit or recalibrate,
both fine. Action paralysis kept low by visual hierarchy (primary
button vs muted text link).
Found the actual Sure pattern in app/views/accounts/_form.html.erb:27-47
("Additional details" section in the account-creation flow):
<details class="group">
<summary class="cursor-pointer text-sm text-secondary hover:text-primary flex items-center gap-1 py-2">
<%= icon "chevron-right", class: "group-open:rotate-90 transition-transform" %>
...
</summary>
<div class="space-y-2 mt-2 pl-4 border-l border-primary">...</div>
</details>
It's an inline expand (no absolute popup), chevron rotates 90° on
open, body indented with a vertical primary-color rule. My previous
partial was an absolute-positioned popover lifted from
categories/_form.html.erb — not what Sure uses for collapsible form
sections.
Rewrite _color_picker.html.erb to match: chevron + color-preview disc
+ "Color" label in the summary; swatches in an inline indented body.
Catch-up body also drops the em-dash. Was:
"You're saving $X/mo today — $Y/mo short of the pace to finish by $date."
Now two sentences:
"Your current pace is $X/mo. You need an extra $Y/mo to finish by $date."
Two short clauses, no compound separator, each conveys a single number.
Frames the gap as "extra" rather than "short", which behavioral-econ
research suggests reads as more attainable.
Alert previous pass led with delta ("Behind by $750/mo") but the user
still had to reconcile that with the $1,000/mo CTA — the relationship
between current pace, gap, and required rate was implicit.
Make every number visible in the sentence:
- Title: "Save $1,000/mo to stay on track" — leads with the action +
required rate. Reduces decision load: the headline is what to do.
- Body: "You're saving $250/mo today — $750/mo short of the pace to
finish by September 11, 2026." — current pace + gap + deadline.
User can now mentally verify: $250 + $750 = $1,000. The catch-up
amount in title + body + CTA is no longer disconnected from the
current pace number; the body is the bridge.
Adds `scrollbar` utility (defined in app/assets/tailwind/application.css
as 4px gray-300 thumb) to the contributions list container. Browser-
default scrollbar was rendering as a thick dark bar in light mode on
some OSes; the in-house utility renders a thin gray thumb consistently
across themes.
- catch_up alert: title now leads with the new info (delta) and body
states the required rate. Was "Save $1,000/mo to catch up" + "Currently
$750/mo behind" — confusingly double-stated. Now "Behind by $750/mo" +
"Save $1,000/mo to stay on track for {date}." Locale keys swap the
%{amount}/%{delta} placement.
- Goals::StatusPillComponent: each variant carries a theme-dark: text
override so the dark-700 text doesn't disappear against the dark-mode
tinted surface. Verified in dark mode: Paused pill text is now
rgb(231,231,231) (gray-200) instead of rgb(54,54,54) (gray-700).
Pre-existing token contrast fix tracked at we-promise/sure#1736 stays
the long-term path; this is the local workaround that doesn't drop
4.5:1 in either theme.
- New goals/_color_picker.html.erb partial: <details> disclosure with
current-color preview in the summary + swatch grid in the popover.
Mirrors the categories form's pen-icon-overlay pattern in spirit
(collapsed by default; user clicks to expand). Both _form_edit and
_form_stepper render the partial; the stepper's hidden color field is
replaced by the visible disclosure.
- Stepper footer: change `justify-between` to `flex items-center` plus
`ml-auto` on the Continue wrapper. Continue now sits right-aligned in
step 1 (where Back is hidden) and stays right in step 2 with Back
taking the left edge.
Four chart fixes in one pass.
1) Browser was rendering the <title> child as a native hover tooltip
that fought with the custom crosshair tooltip. Drop <title>; use
aria-label on the <svg role="img"> instead — same SR accessible name,
no native tooltip side-effect.
2/3) The hover crosshair clamped at today: bisector ran the saved
series, which ends at today, so future hovers stuck the dot at the
last saved point. Now the controller forks:
- Past hover: snap to nearest contribution via bisector.
- Future hover: snap to whole-week intervals along the projection
segment ([today, target_date]) and place the dot at the
interpolated y on the dashed line. Movement steps cleanly week
by week instead of pixel-by-pixel jitter.
4) Tooltip drops the redundant line:
- Past: "<date> · Saved: $X" (no Projected — there isn't one).
- Future: "<date> · Projected: $X" (no Saved — it's the future).
5) Y-axis tick label suppressed when its value falls within 5% of the
target line so "$2.5K" and "Target · $2,400" stop overlapping near
the right edge. Gridline stays; only the y-axis label drops.
Verified live via Playwright on House downpayment goal: <title>
absent, aria-label populated, past tooltip "Feb 10, 2026 · Saved:
$11,750", future tooltip "Nov 29, 2027 · Projected: $32,235",
neighbouring future x snaps to "Dec 13, 2027 · $32,704" (2-week jump
across the snapping boundary).
Direct nav to /goals/new used to render the index page with an empty
modal frame because the entire template was wrapped in DS::Dialog.
The URL was effectively un-shareable.
Branch on turbo_frame_request? — Turbo Frame requests still render
the DS::Dialog wrapper (the existing in-modal flow on the index page
keeps working). Non-frame requests render a standalone page-level
header (h1 + subtitle + icon) followed by the form_stepper partial.
Same Stimulus controller, same data-goal-stepper-modal-subtitle
selector, so the stepper's subtitle update path works identically.
Controller sets @breadcrumbs so the standalone variant gets the
Home > Goals > New goal trail.
Verified both paths via Playwright: direct GET renders standalone
form with h1 "New goal" + no dialog; click-from-index opens the
DS::Dialog with the stepper inside.
