Money paste follow-ups: separator hint and symbol drift test

[BernatSR]

Part of #3395 (follow-ups from #3300).

Problem

Pasting 1,234 into a EUR money field wrote 1234 instead of 1.234: without a hint, the paste parser reads a comma before exactly three digits as a thousands separator. Before #3300 the browser rejected that paste, so this could now be saved by an auto-submit form.

Changes

1. Currency separator hint

• The money field now carries the selected currency's decimal separator (data-money-field-separator-value) and passes it to parseAmountPaste.
• The hint only decides the one ambiguous shape: a single , or . followed by exactly three digits. Other pastes keep the current parsing, so $1,234.56 pasted into a EUR field is still 1234.56.

Paste	EUR before	EUR after	USD
1,234	1234	1.234	1234
1.234	1.234	1234	1.234
$1,234.56	1234.56	1234.56	1234.56

3. Symbol-list drift test

• test/architecture/currency_paste_symbols_test.rb fails when CURRENCY_SYMBOL in parse_amount_paste.js drifts from the letter-free symbols in config/currencies.yml.
• It's a Minitest rather than an .mjs test because CI doesn't run test/javascript.

Question on part 2 (lettered markers)

I've left USD 500, kr 500, R$ 1.234,56 falling through to the browser. The reason is already documented at the top of parse_amount_paste.js. Would you prefer to keep it that way, or ship a server-provided marker list (possibly limited to the selected currency's own symbol and ISO code)? If the status quo is fine, this PR can close the issue.

Validation

• node --test test/javascript (13 new cases for the separator hint)
• bin/rails test: 10,331 runs, 0 failures
• System tests for money-field flows (transactions, transfers, goals, loans, account activity)
• bin/rubocop, erb_lint, npm run lint, bin/brakeman: clean
• Manually pasted the EUR values above into the new transaction form.

Summary by CodeRabbit

• Bug Fixes
  • Pasted amounts with a single separator followed by exactly three digits are interpreted using the selected currency’s decimal separator, helping avoid incorrect readings of ambiguous values.
  • Other pasted amounts, including values with repeated grouping separators or a different number of digits after the separator, continue to be interpreted independently of the currency setting. This preserves existing behavior for values whose separator usage is unambiguous.

[coderabbitai]

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

Important

Review skipped

Review was skipped as selected files did not have any reviewable changes.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: 92edaba9-ba1d-495e-8a4a-df04486a2898

📥 Commits

Reviewing files that changed from the base of the PR and between b5c07a0 and f0a79cd.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: 8ccc3eb0-0dcc-4220-befd-9f67814e5709

📥 Commits

Reviewing files that changed from the base of the PR and between 7799274 and b5c07a0.

📒 Files selected for processing (4)

• app/javascript/controllers/money_field_controller.js
• app/javascript/utils/parse_amount_paste.js
• app/javascript/utils/parse_locale_float.js
• app/views/shared/_money_field.html.erb

🚧 Files skipped from review as they are similar to previous changes (1)

• app/javascript/utils/parse_amount_paste.js

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.

---

📝 Walkthrough

Walkthrough

The money field supplies the selected currency’s decimal separator to pasted-amount parsing. The parser applies the hint only to numbers with one separator followed by exactly three digits. Tests cover hinted and unhinted parsing, other separator patterns, and currency-symbol consistency.

Changes

Currency-aware paste parsing

Layer / File(s)	Summary
Parse ambiguous amounts app/javascript/utils/parse_amount_paste.js, app/javascript/utils/parse_locale_float.js, test/javascript/parse_amount_paste_test.mjs, test/architecture/currency_paste_symbols_test.rb	parseAmountPaste uses the currency separator hint only for a single separator followed by three digits. Tests cover hinted and unhinted ambiguous amounts, other separator patterns, and verify that the parser’s currency-symbol character class matches configured symbols.
Pass the selected currency separator app/views/shared/_money_field.html.erb, app/javascript/controllers/money_field_controller.js	The money-field view exposes the currency separator and ISO code. The controller stores them after currency data loads and passes the separator to parseAmountPaste when the selected currency matches the stored currency or no currency target exists.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Suggested reviewers: jjmata

Merge Risk: ⚪ Minimal · up to b5c07

The selected currency’s separator now guides ambiguous pastes when available. During a currency switch or failed fetch, the existing heuristic remains in use; no new material merge risk is evident.

Security Architecture Review

Security architecture risk: 🔵 Low · up to b5c07

The change improves currency-aware amount interpretation without adding privileges or widening access. No introduced security issue was established. Existing submission behavior during pending currency changes remains, but its final server-side validation was not verified.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

• inferred — The changed behavior affects amount interpretation in forms using the shared money field. The reviewed delta does not give clipboard content new credentials, execution authority, tenant access, or a new submission destination.

Trust Boundaries and Controls

• observed — Clipboard text remains untrusted numeric input. The existing grammar and finite-result checks remain in place, with rejected input falling through to browser handling. These client-side checks do not establish server-side authorization or amount validation.

Resilience and Maintainability Implications

• observed — While a different currency's metadata is pending or its fetch has failed, paste can still use heuristic parsing and the previous input step for precision, then emit submission-triggering events. This intermediate-state behavior predates the PR; the new separator identity guard prevents a stale hint but does not make the entire currency transition atomic.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 5 files. (1 skipped: 1…	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly identifies both main changes: the currency separator hint and the currency-symbol drift test.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

Full details: Docstring Coverage

Explanation

Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 5 files. (1 skipped: 1 unsupported.)

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create a new PR

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[jjmata]

Single-pass review done without the Agent tool (no multi-agent fan-out / subagent verify pass ran) — two findings below, both self-checked against the diff and the surrounding code.

---

Generated by Claude Code

[jjmata]

Minor: ambiguity detection is now duplicated between this file and parse_locale_float.js.

AMBIGUOUS_SEPARATOR (/^[^.,]*[.,]\d{3}$/) re-implements, with a slightly different scope, the same "single separator + exactly three digits" ambiguity check that parseLocaleFloat's heuristic branch already computes independently (lastDot === -1 && digitsAfterComma === 3 in app/javascript/utils/parse_locale_float.js). The two definitions happen to agree today, but nothing ties them together — if either the regex here or the heuristic there is tweaked later (e.g. to also treat 1,23 or multi-group cases as ambiguous), they can silently drift apart, and parseAmountPaste would gate the hint on a different notion of "ambiguous" than parseLocaleFloat uses when no hint is supplied. Not a current bug, but worth consolidating into one shared ambiguity check (or at least a comment cross-referencing the other file) to keep them from diverging.

---

Generated by Claude Code

[BernatSR]

I added cross-reference comments in both files in b5c07a0. I kept the two checks separate because they answer different questions: the heuristic in parseLocaleFloat picks one reading when there's no hint (it only treats a comma before three digits as grouping), while AMBIGUOUS_SEPARATOR marks every shape a hint should settle, including 1.234. Nothing else passes a separator to parseLocaleFloat, so I didn't want to change its behaviour for the other callers.