Skip to content

fix(i18n): move Questrade dialog strings into en.yml instead of inline defaults#2790

Open
tryeverything24 wants to merge 1 commit into
we-promise:mainfrom
tryeverything24:fix/questrade-locale-defaults
Open

fix(i18n): move Questrade dialog strings into en.yml instead of inline defaults#2790
tryeverything24 wants to merge 1 commit into
we-promise:mainfrom
tryeverything24:fix/questrade-locale-defaults

Conversation

@tryeverything24

@tryeverything24 tryeverything24 commented Jul 25, 2026

Copy link
Copy Markdown

PR body — we-promise/sure — account: tryeverything24

Branch: fix/questrade-locale-defaults (in WSL clone /root/sure, commit 0e489bf9, based on main @ 40b18e84)
Title: fix(i18n): stop hardcoding Questrade view strings via t() defaults


Summary

Addresses the Rule 5 finding from DS Drift Patrol #2635 (i18n bypass via hardcoded default: strings; same pattern previously flagged in #2157).

The Questrade views shipped with 43 t(".key", default: "English string") calls across four templates:

  • app/views/questrade_items/_questrade_item.html.erb (14)
  • app/views/questrade_items/setup_accounts.html.erb (11)
  • app/views/questrade_items/select_existing_account.html.erb (8)
  • app/views/settings/providers/_questrade_panel.html.erb (10)

Hardcoded default: fallbacks silently mask a missing or renamed key instead of surfacing it, and they hide English copy from translators / i18n-tasks. Worse, 17 of these keys were never added to en.yml at all (the setup_accounts and select_existing_account dialog copy, plus the provider panel's eyebrow, setup steps, help link, and disconnect strings) — those views only rendered correctly because of the inline fallbacks.

What changed

  • Added the 17 missing keys to config/locales/views/questrade_items/en.yml, using the exact strings the views render today, following the file's existing structure and quoting style.
  • Dropped every default: argument from the four Questrade templates. Keys that already existed keep resolving to their current locale values.

No user-visible copy changes in any locale: English strings are byte-identical, and other locales fall back to English exactly as before via config.i18n.fallbacks (config/application.rb:28). The new keys are now visible to i18n-tasks and translators (fr/pl/tr questrade files can pick them up).

Test plan

  • bin/rails test test/controllers/questrade_items_controller_test.rb — 17 runs, 0 failures (these tests render the touched setup/link dialogs).
  • Verified every t() key referenced by the four templates resolves against the loaded en tree (55 keys checked, 0 missing).
  • bundle exec erb_lint on the four templates — no offenses.
  • bin/rails test — full unit/integration suite green.

Checklist for opening: base = main, allow edits from maintainers ON.

Summary by CodeRabbit

  • New Features
    • Expanded the Questrade setup experience with clearer step-by-step guidance, account type options, historical data settings, and sync start-date information.
    • Added Link, Create, Done, and Cancel actions throughout account setup and account linking dialogs.
    • Added improved setup, linking, syncing, error, and disconnect messaging.
  • Documentation
    • Added detailed Questrade setup instructions, including token-generation guidance and help information.

The Questrade views shipped with ~40 `t(".key", default: "English
string")` calls. Hardcoded `default:` fallbacks silently mask a
missing or renamed key instead of surfacing it, defeating the
missing-translation safety net, and they hide English copy from
translators — this exact pattern was flagged by DS Drift Patrol
(we-promise#2635, previously we-promise#2157).

17 of those keys were never added to
config/locales/views/questrade_items/en.yml (the setup_accounts and
select_existing_account dialogs plus the provider panel's eyebrow,
steps, help link, and disconnect strings), so those views only worked
because of the inline fallbacks.

- Add the missing keys to en.yml with the exact strings the views
  render today, so no user-visible copy changes.
- Drop every `default:` argument from the four Questrade views
  (43 call sites). Keys that already existed keep resolving to their
  current locale values, so rendered output is unchanged there too.

Other locales fall back to the English strings exactly as before via
`config.i18n.fallbacks`, and the new keys are now visible to
i18n-tasks and translators.

Addresses the finding in we-promise#2635.
@coderabbitai

coderabbitai Bot commented Jul 25, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: c8fcc081-a998-4e82-9e5d-09327be78ed3

📥 Commits

Reviewing files that changed from the base of the PR and between 40b18e8 and 0e489bf.

📒 Files selected for processing (5)
  • app/views/questrade_items/_questrade_item.html.erb
  • app/views/questrade_items/select_existing_account.html.erb
  • app/views/questrade_items/setup_accounts.html.erb
  • app/views/settings/providers/_questrade_panel.html.erb
  • config/locales/views/questrade_items/en.yml

📝 Walkthrough

Walkthrough

Questrade views now rely on dedicated I18n keys without inline fallback text. English translations were expanded for provider setup, account linking, account setup, sync guidance, statuses, buttons, and account types. The existing-account flow also renders Link submit buttons.

Changes

Questrade localization

Layer / File(s) Summary
Provider setup translations and panel wiring
config/locales/views/questrade_items/en.yml, app/views/settings/providers/_questrade_panel.html.erb
Adds setup and disconnect translations and updates the provider panel to use translation-only copy.
Existing account linking
config/locales/views/questrade_items/en.yml, app/views/questrade_items/select_existing_account.html.erb
Restructures account-linking translations and adds per-account Link submit buttons.
Account setup and status copy
app/views/questrade_items/_questrade_item.html.erb, app/views/questrade_items/setup_accounts.html.erb, config/locales/views/questrade_items/en.yml
Removes inline fallbacks from status and setup views and adds translations for setup states, buttons, sync guidance, and account types.

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

Possibly related issues

Possibly related PRs

Suggested reviewers: jjmata

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main change: moving Questrade translation strings from inline defaults into en.yml.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

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

jjmata commented Jul 26, 2026

Copy link
Copy Markdown
Collaborator

Reviewed against the i18n conventions in CLAUDE.md.

Looks correct overall:

  • All t(...) calls in the four touched views (_questrade_item.html.erb, select_existing_account.html.erb, setup_accounts.html.erb, _questrade_panel.html.erb) had their inline default: fallbacks removed — no leftovers found in these files.
  • Every new key referenced by the views (panel.eyebrow, panel.help_text, panel.disconnect(_confirm), panel.steps.s1_htmls6, select_existing_account.dialog_title/page_title/linking_label/none_available/buttons.cancel/buttons.link, setup_accounts.dialog_title/page_title/import_as/all_linked/types.*/buttons.cancel/create/done) is present in config/locales/views/questrade_items/en.yml with text matching the removed default exactly, so behavior is unchanged.
  • Keys are nested hierarchically under the existing questrade_items.<view> namespaces, consistent with the file's existing structure.

Minor nit — key naming inconsistency: the new buttons: sub-hash (buttons.cancel, buttons.link, buttons.create, buttons.done) duplicates existing flat keys already sitting in the same namespaces (select_existing_account.cancel/cancel_button/link_button, setup_accounts.cancel/create_accounts). It also diverges from the suffix convention other providers use in this same file tree (e.g. indexa_capital_items/snaptrade_items use cancel_button, link_button, done_button, create_button rather than a nested buttons: hash). Not a blocker, but worth consolidating in a follow-up so there aren't 3-4 keys meaning "Cancel" in the same namespace.

Other locales: fr.yml, pl.yml, and tr.yml exist for questrade_items but don't have these new keys yet. That looks consistent with how this repo handles translations (separate PRs add non-English locales after English lands, per #2697/#2737), so English-only here seems acceptable rather than a gap.


Generated by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants