fix(i18n): complete German core frontend translations#2671
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe PR expands German localization coverage, replaces hardcoded date and UI text with Rails I18n helpers, adds localized period comparison labels, improves accessibility translations, and introduces tests for locale structure, interpolation consistency, and localization bypasses. ChangesLocalization contracts and enforcement
Localized view rendering
German locale coverage
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 57086d0717
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
config/locales/views/loans/de.yml (1)
19-37: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider using a YAML anchor to eliminate duplicated overview keys.
tabs.overview(lines 28-37) is an exact copy ofoverview(lines 19-26) with one additional key (edit_loan_details). If a label changes in one block, the other can easily fall out of sync.♻️ Optional refactor using YAML merge key
overview: &loans_overview interest_rate: Zinssatz monthly_payment: Monatliche Zahlung not_applicable: Nicht zutreffend original_principal: Ursprünglicher Darlehensbetrag remaining_principal: Verbleibender Darlehensbetrag term: Laufzeit type: Typ unknown: Unbekannt tabs: overview: - interest_rate: Zinssatz - monthly_payment: Monatliche Zahlung - not_applicable: Nicht zutreffend - original_principal: Ursprünglicher Darlehensbetrag - remaining_principal: Verbleibender Darlehensbetrag - term: Laufzeit - type: Typ - unknown: Unbekannt + <<: *loans_overview edit_loan_details: Darlehensdetails bearbeiten🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@config/locales/views/loans/de.yml` around lines 19 - 37, Use a YAML anchor for the shared loan label mappings under overview, then merge that anchor into tabs.overview and retain only the additional edit_loan_details key there. Preserve all existing translated labels while eliminating the duplicated keys.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@config/locales/views/accounts/de.yml`:
- Around line 110-111: Update the German confirm_body_html translation to add
periods at both sentence boundaries: between “auswirkt” and “Diese Aktion”, and
between “werden” and “Du müsstest”. Preserve the existing wording and HTML
structure.
In `@config/locales/views/transactions/de.yml`:
- Line 126: Remove the duplicate scalar transactions.merge_duplicate entry near
the button labels, preserving show.merge_duplicate for the button text and the
later merge_duplicate mapping containing success and failure messages. Ensure
only the result-message mapping remains under transactions.merge_duplicate.
---
Nitpick comments:
In `@config/locales/views/loans/de.yml`:
- Around line 19-37: Use a YAML anchor for the shared loan label mappings under
overview, then merge that anchor into tabs.overview and retain only the
additional edit_loan_details key there. Preserve all existing translated labels
while eliminating the duplicated keys.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 5e6aa1e1-f224-4105-8ae4-6703b6c844b6
📒 Files selected for processing (45)
app/models/period.rbapp/views/accounts/_accountable_group.html.erbapp/views/budget_categories/show.html.erbapp/views/budgets/_picker.html.erbapp/views/layouts/shared/_footer.html.erbapp/views/pages/changelog.html.erbapp/views/reports/_budget_performance.html.erbapp/views/reports/_period_picker.html.erbapp/views/reports/index.html.erbapp/views/reports/print.html.erbapp/views/shared/_app_version.html.erbapp/views/splits/edit.html.erbapp/views/splits/new.html.erbapp/views/transactions/convert_to_trade.html.erbapp/views/transactions/show.html.erbapp/views/valuations/_confirmation_contents.html.erbconfig/locales/breadcrumbs/de.ymlconfig/locales/models/category/de.ymlconfig/locales/models/period/de.ymlconfig/locales/models/period/en.ymlconfig/locales/views/accounts/de.ymlconfig/locales/views/budgets/de.ymlconfig/locales/views/categories/de.ymlconfig/locales/views/chats/de.ymlconfig/locales/views/components/de.ymlconfig/locales/views/holdings/de.ymlconfig/locales/views/ibkr_items/de.ymlconfig/locales/views/kraken_items/de.ymlconfig/locales/views/layout/de.ymlconfig/locales/views/layout/en.ymlconfig/locales/views/loans/de.ymlconfig/locales/views/onboardings/de.ymlconfig/locales/views/other_assets/de.ymlconfig/locales/views/pages/de.ymlconfig/locales/views/pages/en.ymlconfig/locales/views/recurring_transactions/de.ymlconfig/locales/views/reports/de.ymlconfig/locales/views/reports/en.ymlconfig/locales/views/shared/de.ymlconfig/locales/views/shared/en.ymlconfig/locales/views/trades/de.ymlconfig/locales/views/transactions/de.ymlconfig/locales/views/users/de.ymltest/i18n/german_core_localization_test.rbtest/models/period_test.rb
- localize custom period comparisons in every supported period locale - fix German punctuation and duplicate transaction translation key - cover comparison-range interpolation across all period locales
Not addressing this optional refactor: the locale files intentionally keep the two translation namespaces explicit and structurally parallel with other locales. Introducing a German-only YAML merge would add parsing/maintenance complexity without changing runtime behavior. |
jjmata
left a comment
There was a problem hiding this comment.
Thanks for the thorough i18n work — good test coverage in test/i18n/german_core_localization_test.rb and consistent use of Minitest (no RSpec/FactoryBot). Found two issues that I think should be fixed before merging:
1. date.formats.month_year doesn't exist for German (or most other locales), so the new l(..., format: :month_year) calls will silently fall back to English month names
This PR replaces hardcoded strftime("%B %Y") with l(date, format: :month_year) in three places:
app/views/budget_categories/show.html.erb:34app/views/reports/_budget_performance.html.erb:7app/views/reports/print.html.erb:27(footer)
But config/locales/defaults/de.yml only defines date.formats.{default,long,short} — there's no month_year key, and this PR doesn't add one. Since config.i18n.fallbacks = true (config/application.rb:28), l(date, format: :month_year) for :de will fall back to en's "%B %Y", rendering English month names for German users — the exact regression this PR is meant to fix, just relocated. I checked all locale files under config/locales/defaults/; only ca, en, fr, ru, and zh-CN define month_year, so this affects de plus ~10 other locales.
Suggest adding date.formats.month_year (and ideally short_month_year, used elsewhere) to de.yml — or the other locale defaults that are missing it — before switching these call sites over.
2. Dropping .downcase in app/views/accounts/_accountable_group.html.erb:59 changes rendering for every locale, not just German
The change:
- text: t("accounts.sidebar.new_account_group", account_group: account_group.accountable_type.singular_display_name.downcase),
+ text: t("accounts.sidebar.new_account_group", account_group: account_group.accountable_type.singular_display_name),singular_display_name resolves through I18n.t("accounts.types.#{name.underscore}"), which returns capitalized strings (e.g. "Credit Card"). Every other locale's new_account_group string embeds %{account_group} assuming a lowercase, mid-sentence noun — e.g. en.yml: "New %{account_group}", fr.yml: "Nouveau %{account_group}", plus es, pl, nl, ca, ro, pt-BR, hu, vi, zh-TW. Only the German string was reworded to compensate ("%{account_group} hinzufügen", noun-first). As-is, this will render "New Credit Card" instead of "New credit card" in English and similarly odd capitalization in the other unaudited locales.
Since this is a shared, non-German-specific view, either restore .downcase and give German its own capitalized-noun phrasing without touching the call site, or audit/update every locale's new_account_group string to match the new capitalization.
Everything else (the bulk YAML additions, the l(...)/t(...) replacements for :long/:short formats, the new Period#localized_comparison_range helper) looks correct and in scope.
Generated by Claude Code
- localize month-year labels without missing named-format fallbacks - preserve lowercase account types outside German - cover German, English, and French rendering regressions
|
@jjmata Thanks for the detailed review — both issues are addressed in
Addressed slightly differently: the three call sites introduced by this PR now use the explicit localized format
Addressed with a locale-controlled capitalization flag. Existing lowercase behavior remains the default through the English fallback, while German explicitly preserves noun capitalization. Regression coverage now asserts Validation: full suite 5,379 runs / 22,163 assertions / 0 failures / 0 errors; focused i18n and affected-controller tests 60 runs / 203 assertions; RuboCop and ERB lint clean. A re-review would be appreciated when convenient. |
|
Periodic review sweep: commit 063905f (pushed after @jjmata's review) addresses both flagged points — localized Generated by Claude Code |
|
Automated review sweep: the two issues jjmata flagged on 07-13 (missing Generated by Claude Code |
|
Some interesting i18n changes here, can you review and share your feedback @gariasf? |
|
The two review rounds (month_year format, the cross-locale Generated by Claude Code |
- localize custom period comparisons in every supported period locale - fix German punctuation and duplicate transaction translation key - cover comparison-range interpolation across all period locales
- localize month-year labels without missing named-format fallbacks - preserve lowercase account types outside German - cover German, English, and French rendering regressions
ac1d3db to
bb31ab2
Compare
jjmata
left a comment
There was a problem hiding this comment.
Can you fix the CI tests that broke as a result of the rebase? 🙏
Summary
German users no longer encounter English fallbacks across the core dashboard, accounts, transactions, budgets, categories, reports, navigation, chat, and loan flows. Period labels, month names, dates, account-group actions, financial terms, and supporting menu text now follow the active locale consistently.
The change also adds a structural regression test for German/English key and interpolation parity, plus focused coverage for localized custom period labels and account-type capitalization. This reduces 397 statically used German fallbacks while keeping locale interpolation contracts aligned.
Validation
bin/rails test: 5,376 runs, 22,140 assertions, 0 failures, 0 errors, 28 existing skipsgit diff --check: cleanNo screenshots are included because validation used synthetic test data only and must not expose details from a private Sure instance.
Post-Deploy Monitoring & Validation
No additional operational monitoring is required. Validate representative core routes with the German locale and watch for missing-translation or interpolation errors; revert this PR if localized rendering regresses.
Summary by CodeRabbit