i18n: Core features (Part 1/4) #667
Conversation
📝 WalkthroughWalkthroughThis PR comprehensively introduces internationalization (i18n) across the application by replacing hardcoded English strings with Rails translation keys ( Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
|
Related Documentation Checked 1 published document(s) in 1 knowledge base(s). No updates required. How did I do? Any feedback? |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
config/locales/views/accounts/en.yml (1)
62-79: Duplicate translation keys inactivitysection.There are duplicate keys defined under
accounts.show.activity:
newappears at lines 62 and 73new_balanceappears at lines 63 and 74new_transactionappears at lines 64 and 75In YAML, duplicate keys at the same level cause the second definition to silently override the first. While this doesn't break functionality here (values are identical), it creates maintenance confusion and could lead to bugs if values diverge later.
Additionally,
search_placeholder(line 65) andsearch.placeholder(line 79) serve the same purpose with different key structures.🔧 Suggested fix: Remove duplicate keys
activity: - new: New - new_balance: New balance - new_transaction: New transaction - search_placeholder: Search entries by name amount: Amount balance: Balance confirmed: Confirmed date: Date entries: entries entry: entry filter: Filter new: New new_balance: New balance new_transaction: New transaction no_entries: No entries found pending: Pending - search: - placeholder: Search entries by name + search_placeholder: Search entries by name status: Status title: Activityapp/controllers/holdings_controller.rb (1)
36-48: Add the missing locale key for the flash message.The code uses
t(".cannot_delete")but this key does not exist inconfig/locales/views/holdings/en.yml. Theholdings.destroysection currently only contains thesuccesskey. Add thecannot_deletetranslation to the locale file:destroy: success: Holding deleted cannot_delete: [translation needed]Without this entry, the application will display a missing translation error to users instead of the intended message.
🤖 Fix all issues with AI agents
In `@app/controllers/categories_controller.rb`:
- Around line 59-62: Add the missing i18n key categories.destroy_all.all_deleted
referenced by the destroy_all action: when Current.family.categories.destroy_all
completes the redirect uses t(".all_deleted") (lazy lookup resolves to
categories.destroy_all.all_deleted), so add that key with the appropriate
English message to your views/categories locale YAML (matching the style of
existing keys like categories.destroy.success).
In `@app/controllers/family_merchants_controller.rb`:
- Line 5: The breadcrumbs keys used in the controller (breadcrumbs.home and
breadcrumbs.merchants) are missing from the English locale; open the English
locale file for the shared views and add a breadcrumbs section with home: "Home"
and merchants: "Merchants" (en: breadcrumbs: home: Home merchants: Merchants)
and ensure any other locale files also include the same keys so i18n coverage
matches the controller usage.
In `@app/views/valuations/_confirmation_contents.html.erb`:
- Line 25: Replace the inline action_verb.capitalize usage with a locale-aware
translation: stop calling .capitalize and instead move the entire sentence (or
at least the action words) into I18n translation keys so locale files supply the
properly capitalized form; for example create translation keys like
"valuations.confirmation.<action>" or "valuations.actions.<action_verb>" and use
I18n.t("...").interpolate(action-specific values) from the view (referencing
action_verb and the partial _confirmation_contents) so capitalization is
controlled per locale rather than using action_verb.capitalize.
In `@app/views/valuations/confirm_create.html.erb`:
- Line 13: Hardcoded action_verb: "set" in confirm_create.html.erb should be
replaced with an i18n lookup to avoid injecting an English string into
translated copy; find the hash or locals where action_verb is set (action_verb:
"set") and change it to use the Rails i18n helper (e.g.
t("valuations.action_verbs.set") or I18n.t(...)) and ensure the corresponding
translation key (valuations.action_verbs.set) exists in your locale files,
mirroring the change made in confirm_update.html.erb.
In `@app/views/valuations/confirm_update.html.erb`:
- Line 12: The partial is passing a hardcoded action_verb: "update" which gets
interpolated into t("valuations.confirmation_contents.investment_intro", action:
action_verb, ...) breaking i18n; change it to pass a translated key instead
(e.g., use t("valuations.confirmation_contents.action_update") or select a
locale key based on the action), or refactor the translation into separate keys
like valuations.confirmation_contents.investment_intro_update and
investment_intro_set and call the appropriate key from the view; add the
corresponding keys to the locale files (e.g.,
confirmation_contents.action_update/action_set or the full intro per-action
keys) and update the view to pass the translated string or correct key name so
no raw English word is interpolated.
In `@config/locales/views/family_merchants/en.yml`:
- Around line 1-6: The translations for the family_merchants namespace are split
across two files: move the keys family_merchant.edit and family_merchant.delete
into the existing family_merchants subsection in the merchants locale file
(merge them under the same en -> family_merchants -> family_merchant hierarchy
so all actions and view keys live together), remove this separate
family_merchants locale file, and ensure the YAML nesting and keys match the
existing create.success and destroy.success entries so the lookup paths remain
consistent.
In `@config/locales/views/oidc_accounts/en.yml`:
- Around line 9-17: Replace the hardcoded user-facing strings in the
link.html.erb view with i18n lookups: swap the literal verify identity paragraph
with t("oidc_accounts.link.verify_identity_description_html", provider:
`@pending_auth`["provider"], email: `@pending_auth`["email"]) and replace the "no
account found" markup with t("oidc_accounts.link.no_account_found_html", email:
`@pending_auth`["email"], provider: `@pending_auth`["provider"]); ensure you use the
_html keys so Rails will render safe HTML while interpolating
`@pending_auth`["provider"] and `@pending_auth`["email"] to preserve escaping.
🧹 Nitpick comments (5)
app/views/accounts/new/_container.html.erb (1)
23-24: LGTM!All i18n keys are correctly defined and the translations exist in the locale file. The implementation properly localizes UI labels for keyboard navigation hints.
Optional: Consider using Rails' shorthand
.keynotation (e.g.,t(".previous")instead oft("accounts.new.container.previous")) for consistency with other partials like_activity.html.erband reduced verbosity. The view path automatically resolves toaccounts.new.container.*.Also applies to: 32-32, 38-38, 48-49
app/views/entries/_selection_bar.html.erb (1)
10-10: Consider using a relative translation key for consistency.In Rails partials, relative keys resolve based on the partial's path. Since this partial is at
app/views/entries/_selection_bar.html.erb, you could uset(".delete")which would resolve toentries.selection_bar.delete.This would be more consistent with how other views in this PR use relative keys, though the current full-path approach works correctly.
♻️ Suggested change
- <button type="button" data-bulk-select-scope-param="bulk_delete" data-action="bulk-select#submitBulkRequest" class="p-1.5 group/delete hover:bg-inverse flex items-center justify-center rounded-md" title="<%= t("entries.selection_bar.delete") %>"> + <button type="button" data-bulk-select-scope-param="bulk_delete" data-action="bulk-select#submitBulkRequest" class="p-1.5 group/delete hover:bg-inverse flex items-center justify-center rounded-md" title="<%= t(".delete") %>">app/views/holdings/new.html.erb (1)
1-1: Consider using a relative translation key for consistency.Since this is
holdings/new.html.erb, Rails will automatically resolvet(".coming_soon")toholdings.new.coming_soon. Using relative keys is a common convention in this codebase and reduces duplication.♻️ Suggested change
-<p><%= t("holdings.new.coming_soon") %></p> +<p><%= t(".coming_soon") %></p>app/views/budget_categories/_allocation_progress.erb (1)
12-12: Consider using relative translation keys for brevity.Since this partial is
budget_categories/_allocation_progress.erb, Rails will resolve relative keys liket(".over_100_set")tobudget_categories.allocation_progress.over_100_set. The fully qualified keys work but are more verbose than necessary.♻️ Suggested changes
- <p class="text-primary text-sm"><%= t("budget_categories.allocation_progress.over_100_set") %></p> + <p class="text-primary text-sm"><%= t(".over_100_set") %></p>- <%= t("budget_categories.allocation_progress.percent_set", percent: number_to_percentage(budget.allocated_percent, precision: 0)) %> + <%= t(".percent_set", percent: number_to_percentage(budget.allocated_percent, precision: 0)) %>- <%= t("budget_categories.allocation_progress.budget_exceeded") %> <span class="text-red-500"><%= format_money(budget.available_to_allocate_money.abs) %></span> + <%= t(".budget_exceeded") %> <span class="text-red-500"><%= format_money(budget.available_to_allocate_money.abs) %></span>- <span class="text-secondary"><%= t("budget_categories.allocation_progress.left_to_allocate") %></span> + <span class="text-secondary"><%= t(".left_to_allocate") %></span>Also applies to: 15-15, 37-37, 41-41
app/views/budgets/_budgeted_summary.html.erb (1)
5-57: Consider using relative translation keys for consistency.The i18n implementation is correct, but this partial uses absolute paths (
t("budgets.budgeted_summary.expected_income")) while other views in this PR use relative keys (t(".title")). Since Rails resolves relative keys for partials to{controller}.{partial_name}.{key}, relative keys would work here and be more concise.♻️ Suggested refactor to use relative keys
- <h3 class="text-sm text-secondary mb-2"><%= t("budgets.budgeted_summary.expected_income") %></h3> + <h3 class="text-sm text-secondary mb-2"><%= t(".expected_income") %></h3>- <p class="text-secondary"><%= t("budgets.budgeted_summary.amount_earned", amount: format_money(budget.actual_income_money)) %></p> + <p class="text-secondary"><%= t(".amount_earned", amount: format_money(budget.actual_income_money)) %></p>Apply similarly to other
t("budgets.budgeted_summary.*")calls in this file.
|
|
||
| def index | ||
| @breadcrumbs = [ [ "Home", root_path ], [ "Merchants", nil ] ] | ||
| @breadcrumbs = [ [ :"breadcrumbs.home", root_path ], [ :"breadcrumbs.merchants", nil ] ] |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, check the controller file to understand the context
cat -n app/controllers/family_merchants_controller.rb | head -20Repository: we-promise/sure
Length of output: 174
🏁 Script executed:
# Search for breadcrumb translations in locale files
rg -n "breadcrumbs:" config/locales/ -A 3 -B 1Repository: we-promise/sure
Length of output: 333
🏁 Script executed:
# Search for family_merchants translations (for flash messages)
rg -n "family_merchants:" config/locales/ -A 5 -B 1Repository: we-promise/sure
Length of output: 5568
🏁 Script executed:
# Search for breadcrumbs.merchants or merchants under breadcrumbs in all locales
rg -n "breadcrumbs:" config/locales/ -A 20 | grep -i merchantsRepository: we-promise/sure
Length of output: 120
🏁 Script executed:
# Check English locale for breadcrumbs
rg -n "breadcrumbs:" config/locales/views/shared/en.yml -A 10Repository: we-promise/sure
Length of output: 41
🏁 Script executed:
# Search for all breadcrumb keys defined
fd "shared.*\.yml" config/locales/ -x rg -l "breadcrumbs:" {}Repository: we-promise/sure
Length of output: 41
🏁 Script executed:
# Search for all files in config/locales that contain breadcrumbs
rg -l "breadcrumbs:" config/locales/Repository: we-promise/sure
Length of output: 93
🏁 Script executed:
# Check what locale files exist in shared
fd "." config/locales/views/shared/ -type f | head -20Repository: we-promise/sure
Length of output: 229
🏁 Script executed:
# Search for breadcrumbs in all English locale files
rg -n "breadcrumbs:" config/locales/ -B 2 | grep -A 3 "en.yml"Repository: we-promise/sure
Length of output: 41
🏁 Script executed:
# List all files in config/locales/views/shared
ls -la config/locales/views/shared/Repository: we-promise/sure
Length of output: 808
🏁 Script executed:
# View the French shared locale file to see what breadcrumbs are defined
cat config/locales/views/shared/fr.ymlRepository: we-promise/sure
Length of output: 682
🏁 Script executed:
# Search for any English locale files with breadcrumbs
find config/locales -name "*.en.yml" -o -name "*en*.yml" | xargs grep -l "breadcrumbs:" 2>/dev/null || echo "No English breadcrumbs found"Repository: we-promise/sure
Length of output: 87
🏁 Script executed:
# View the English shared locale file
cat config/locales/views/shared/en.ymlRepository: we-promise/sure
Length of output: 378
🏁 Script executed:
# Check if there are any other locale files that might have breadcrumbs
rg -n "breadcrumbs:" config/locales/ -B 2 -A 10Repository: we-promise/sure
Length of output: 806
Add breadcrumb translations to the English locale file.
The breadcrumbs.home and breadcrumbs.merchants keys are used in the code but only defined in the French locale (config/locales/views/shared/fr.yml). The English locale (config/locales/views/shared/en.yml) lacks the entire breadcrumbs: section. Add the missing breadcrumb keys to all locale files to ensure proper i18n coverage across languages.
Missing breadcrumbs in English locale
en:
breadcrumbs:
home: Home
merchants: Merchants
🤖 Prompt for AI Agents
In `@app/controllers/family_merchants_controller.rb` at line 5, The breadcrumbs
keys used in the controller (breadcrumbs.home and breadcrumbs.merchants) are
missing from the English locale; open the English locale file for the shared
views and add a breadcrumbs section with home: "Home" and merchants: "Merchants"
(en: breadcrumbs: home: Home merchants: Merchants) and ensure any other locale
files also include the same keys so i18n coverage matches the controller usage.
| </div> | ||
| </div> | ||
| <% else %> | ||
| <p><%= action_verb.capitalize %> |
There was a problem hiding this comment.
action_verb.capitalize may break for non-English locales.
Calling .capitalize on an interpolated string assumes English capitalization rules. Some languages have different capitalization behavior or none at all. Consider moving the entire sentence into the translation with the action already capitalized, or use separate translation keys for different actions.
🤖 Prompt for AI Agents
In `@app/views/valuations/_confirmation_contents.html.erb` at line 25, Replace the
inline action_verb.capitalize usage with a locale-aware translation: stop
calling .capitalize and instead move the entire sentence (or at least the action
words) into I18n translation keys so locale files supply the properly
capitalized form; for example create translation keys like
"valuations.confirmation.<action>" or "valuations.actions.<action_verb>" and use
I18n.t("...").interpolate(action-specific values) from the view (referencing
action_verb and the partial _confirmation_contents) so capitalization is
controlled per locale rather than using action_verb.capitalize.
| --- | ||
| en: | ||
| family_merchants: | ||
| family_merchant: | ||
| edit: Edit | ||
| delete: Delete |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for family_merchants translation keys across all locale files
rg -n "family_merchants:" config/locales/ -A 20Repository: we-promise/sure
Length of output: 17674
Consolidate family_merchants translations into a single locale file.
The family_merchants namespace is already defined in config/locales/views/merchants/en.yml with controller actions (create.success, destroy.success) and view sections (edit, form, index). Creating a separate config/locales/views/family_merchants/en.yml file splits translations for the same feature across two locations, violating the hierarchical organization guideline. Merge the partial-level keys (family_merchant.edit, family_merchant.delete) into the existing config/locales/views/merchants/en.yml file under the appropriate family_merchants subsection to maintain a unified structure.
🤖 Prompt for AI Agents
In `@config/locales/views/family_merchants/en.yml` around lines 1 - 6, The
translations for the family_merchants namespace are split across two files: move
the keys family_merchant.edit and family_merchant.delete into the existing
family_merchants subsection in the merchants locale file (merge them under the
same en -> family_merchants -> family_merchant hierarchy so all actions and view
keys live together), remove this separate family_merchants locale file, and
ensure the YAML nesting and keys match the existing create.success and
destroy.success entries so the lookup paths remain consistent.
| verify_identity_description_html: To link your %{provider} account<span> (%{email})</span>, please verify your identity by entering your password. | ||
| email: Email | ||
| email_placeholder: Enter your email | ||
| password: Password | ||
| password_placeholder: Enter your password | ||
| security_note: This helps ensure that only you can link external accounts to your profile. | ||
| link_account: Link Account | ||
| create_new_account: Create New Account | ||
| no_account_found_html: No account found with the email <strong>%{email}</strong>. Click below to create a new account using your %{provider} identity. |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -n --type=erb --type=rb -C3 'verify_identity_description_html|no_account_found_html'Repository: we-promise/sure
Length of output: 85
🏁 Script executed:
#!/bin/bash
rg -n 'verify_identity_description_html|no_account_found_html' app/views/Repository: we-promise/sure
Length of output: 41
🏁 Script executed:
#!/bin/bash
# Search entire repo for the keys
rg -n 'verify_identity_description_html|no_account_found_html' .
echo "---"
# Also check the full context of the locale file
head -30 config/locales/views/oidc_accounts/en.ymlRepository: we-promise/sure
Length of output: 1449
🏁 Script executed:
#!/bin/bash
# Find OIDC-related view files
fd -e erb . app/views/ | grep -i oidc
echo "---"
# Also search for views that might reference these namespaces
find app/views -name "*oidc*" -o -name "*account*" | head -20Repository: we-promise/sure
Length of output: 1044
🏁 Script executed:
#!/bin/bash
cat -n app/views/oidc_accounts/link.html.erbRepository: we-promise/sure
Length of output: 3271
Update the view to use the i18n keys instead of hardcoding user-facing strings.
The locale file defines verify_identity_description_html and no_account_found_html, but app/views/oidc_accounts/link.html.erb hardcodes these strings directly in the template (lines 7–11 and 37–41) instead of using t() calls. Per the project's i18n guidelines, all user-facing strings must use the localization helper.
Update the view to use:
t("oidc_accounts.link.verify_identity_description_html", provider:@pending_auth["provider"], email:@pending_auth["email"])t("oidc_accounts.link.no_account_found_html", email:@pending_auth["email"], provider:@pending_auth["provider"])
Rails will render _html keys as safe HTML while escaping the interpolation values, which prevents XSS while preserving intended HTML structure.
🤖 Prompt for AI Agents
In `@config/locales/views/oidc_accounts/en.yml` around lines 9 - 17, Replace the
hardcoded user-facing strings in the link.html.erb view with i18n lookups: swap
the literal verify identity paragraph with
t("oidc_accounts.link.verify_identity_description_html", provider:
`@pending_auth`["provider"], email: `@pending_auth`["email"]) and replace the "no
account found" markup with t("oidc_accounts.link.no_account_found_html", email:
`@pending_auth`["email"], provider: `@pending_auth`["provider"]); ensure you use the
_html keys so Rails will render safe HTML while interpolating
`@pending_auth`["provider"] and `@pending_auth`["email"] to preserve escaping.
|
Thanks for all this work, @foXaCe ... I've been trying to set up some Claude Code scripts that help keep localizations updated and sends new "proposed strings" to contributors when new functionality is added. Would be be interested in working on a workflow like that? BTW, still see some CodeRabbit PR review comments that need action even on this first one ( #667 ) - can you review? |
|
Thanks @jjmata! Yes, that would definitely make translations easier to maintain. I'd be interested in working on a Claude Code workflow for that. I'll review the pending CodeRabbit comments now. |
- Remove duplicate translation keys in accounts/en.yml activity section - Add missing holdings.destroy.cannot_delete key - Add missing categories.destroy_all.all_deleted key - Add missing breadcrumbs.home and breadcrumbs.merchants keys - Replace hardcoded action_verb strings with i18n keys in valuations - Merge family_merchants locale into merchants locale file
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
config/locales/views/merchants/en.yml (1)
38-39: Typo: "effect" should be "affect".The word "effect" (noun/verb meaning to bring about) should be "affect" (verb meaning to influence).
📝 Proposed fix
confirm_body: Are you sure you want to delete this merchant? Removing this merchant - will unlink all associated transactions and may effect your reporting. + will unlink all associated transactions and may affect your reporting.
🤖 Fix all issues with AI agents
In `@app/views/valuations/_confirmation_contents.html.erb`:
- Line 49: The conditional is inverted: the view uses is_update to choose the
wrong translation keys; update the template so when is_update is true it renders
t("valuations.confirmation_contents.recalculation_update") and when false it
renders t("valuations.confirmation_contents.recalculation_change") — locate the
ERB line that references is_update and the t(...) keys and swap the branches so
the action verb and recalculation message are semantically aligned.
🧹 Nitpick comments (2)
config/locales/views/accounts/en.yml (1)
59-60: Consider consolidating duplicatecomplete_setuptranslations.This key has the same value as
accounts.account.complete_setupon line 10. If both views should always display identical text, consider referencing a single key to reduce maintenance overhead. However, separate keys are acceptable if the contexts may diverge in the future.config/locales/views/categories/en.yml (1)
19-22: Combine the split message into a single interpolated translation key.Lines 21–22 split one sentence across two keys, which makes word order and punctuation maintenance difficult for translators. Refactor to use a single
poor_contrastkey with%{auto_adjust}interpolation:♻️ Suggested update
- poor_contrast: Poor contrast, choose darker color or - auto_adjust: auto-adjust. + poor_contrast: "Poor contrast, choose darker color or %{auto_adjust}." + auto_adjust: auto-adjustUpdate the view to pass the auto_adjust label into the interpolation:
t(".poor_contrast", auto_adjust: t(".auto_adjust")).
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
app/views/valuations/_confirmation_contents.html.erbapp/views/valuations/confirm_create.html.erbapp/views/valuations/confirm_update.html.erbconfig/locales/views/accounts/en.ymlconfig/locales/views/categories/en.ymlconfig/locales/views/holdings/en.ymlconfig/locales/views/merchants/en.ymlconfig/locales/views/shared/en.ymlconfig/locales/views/valuations/en.yml
🚧 Files skipped from review as they are similar to previous changes (1)
- config/locales/views/holdings/en.yml
🧰 Additional context used
📓 Path-based instructions (17)
{app/**/*.{erb,html},app/javascript/**/*.js}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Prefer Hotwire-first approach: use Turbo + Stimulus for reactive UI without heavy JavaScript
Files:
app/views/valuations/confirm_create.html.erbapp/views/valuations/confirm_update.html.erbapp/views/valuations/_confirmation_contents.html.erb
app/views/**/*.erb
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
app/views/**/*.erb: Always use theiconhelper for icons in views, never uselucide_icondirectly
Use Tailwind design tokens (e.g., text-primary, bg-container, border-secondary) instead of raw color names
Pass data to Stimulus controllers via data-*-value attributes instead of inline JavaScript
Prefer semantic HTML elements (dialog, details, summary) over JavaScript components
Use Turbo Frames for page sections instead of client-side solutions
Use server-side formatting for currencies, numbers, and dates instead of client-side
app/views/**/*.erb: Avoid heavy logic in ERB views; prefer helpers and components instead
Ensure ERB views passerb-lintchecks as configured in.erb_lint.yml
Files:
app/views/valuations/confirm_create.html.erbapp/views/valuations/confirm_update.html.erbapp/views/valuations/_confirmation_contents.html.erb
**/*.{css,erb}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use Tailwind CSS v4.x with custom design system for styling
Files:
app/views/valuations/confirm_create.html.erbapp/views/valuations/confirm_update.html.erbapp/views/valuations/_confirmation_contents.html.erb
**/*.{rb,erb}
📄 CodeRabbit inference engine (.cursor/rules/general-rules.mdc)
**/*.{rb,erb}: UseCurrent.userfor accessing the current user. Do NOT usecurrent_user
UseCurrent.familyfor accessing the current family. Do NOT usecurrent_family
Files:
app/views/valuations/confirm_create.html.erbapp/views/valuations/confirm_update.html.erbapp/views/valuations/_confirmation_contents.html.erb
**/*.html.erb
📄 CodeRabbit inference engine (.cursor/rules/project-conventions.mdc)
**/*.html.erb: Prefer native HTML elements over JavaScript-based components. Use semantic HTML: for modals,for disclosures
Leverage Turbo frames to break up pages instead of JavaScript-driven client-side solutions
Use theiconhelper from application_helper.rb for icons. Never uselucide_iconhelper directly
Files:
app/views/valuations/confirm_create.html.erbapp/views/valuations/confirm_update.html.erbapp/views/valuations/_confirmation_contents.html.erb
**/*.{rb,js,erb}
📄 CodeRabbit inference engine (.cursor/rules/project-conventions.mdc)
Format currencies, numbers, dates, and other values server-side, then pass to Stimulus controllers for display only
Files:
app/views/valuations/confirm_create.html.erbapp/views/valuations/confirm_update.html.erbapp/views/valuations/_confirmation_contents.html.erb
**/*.{rb,html.erb}
📄 CodeRabbit inference engine (.cursor/rules/project-conventions.mdc)
Use ActiveRecord validations for complex validations and business logic. Simple validations may be mirrored in ActiveRecord for form error handling convenience, but prioritize client-side form validation when possible
Files:
app/views/valuations/confirm_create.html.erbapp/views/valuations/confirm_update.html.erbapp/views/valuations/_confirmation_contents.html.erb
{app/views/**,app/helpers/**,app/javascript/controllers/**}
📄 CodeRabbit inference engine (.cursor/rules/ui-ux-design-guidelines.mdc)
{app/views/**,app/helpers/**,app/javascript/controllers/**}: Reference maybe-design-system.css for base primitives, functional tokens, and component tokens before writing styles
Prefer using functional design system tokens (e.g., text-primary, bg-container, border-primary) from maybe-design-system.css instead of raw Tailwind color values
Files:
app/views/valuations/confirm_create.html.erbapp/views/valuations/confirm_update.html.erbapp/views/valuations/_confirmation_contents.html.erb
{app/views/**,app/helpers/**}
📄 CodeRabbit inference engine (.cursor/rules/ui-ux-design-guidelines.mdc)
Always generate semantic HTML
Files:
app/views/valuations/confirm_create.html.erbapp/views/valuations/confirm_update.html.erbapp/views/valuations/_confirmation_contents.html.erb
**/*.{erb,html.erb}
📄 CodeRabbit inference engine (.cursor/rules/view_conventions.mdc)
**/*.{erb,html.erb}: Use ViewComponents when: element has complex logic or styling patterns, will be reused across multiple views/contexts, needs structured styling with variants/sizes, requires interactive behavior or Stimulus controllers, has configurable slots or complex APIs, or needs accessibility features or ARIA support
Use Partials when: element is primarily static HTML with minimal logic, used in only one or few specific contexts, is simple template content, doesn't need variants/sizes/complex configuration, or is more about content organization than reusable functionality
Prefer components over partials: if a component is available in app/components, use it; if not, look for a partial; if neither exists, decide between component or partial based on complexity and reusability criteria
Keep domain logic out of views: compute values like button classes, conditional logic, and data transformations in the component file, not the template file
Always use the declarative approach when integrating Stimulus controllers in views: the ERB template should declare what happens using data-* attributes, and the Stimulus controller should respond
Component controllers in app/components/ should only be used within their component templates; global controllers in app/javascript/controllers/ can be used across any view
Pass data from Rails to Stimulus using data-*-value attributes, not inline JavaScript
Files:
app/views/valuations/confirm_create.html.erbapp/views/valuations/confirm_update.html.erbapp/views/valuations/_confirmation_contents.html.erb
**/*.erb
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.erb: In view templates, use native HTML elements preferred over JS components: use<dialog>for modals,<details><summary>for disclosures
Leverage Turbo frames for page sections over client-side solutions in views
Always use server-side formatting for currencies, numbers, and dates in views
Always useiconhelper inapplication_helper.rb. NEVER uselucide_icondirectly in templates.
Always generate semantic HTML in view templates
Keep domain logic OUT of view templates. Logic belongs in component files, not template files.
Files:
app/views/valuations/confirm_create.html.erbapp/views/valuations/confirm_update.html.erbapp/views/valuations/_confirmation_contents.html.erb
**/*.{erb,js,ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{erb,js,ts,tsx}: Use query params for state management over localStorage/sessions in views
In TailwindCSS, always referenceapp/assets/tailwind/maybe-design-system.cssfor primitives and tokens
Use functional tokens defined in design system (e.g.,text-primaryinstead oftext-white,bg-containerinstead ofbg-white)
Pass data to Stimulus controllers viadata-*-valueattributes, not inline JavaScript
Files:
app/views/valuations/confirm_create.html.erbapp/views/valuations/confirm_update.html.erbapp/views/valuations/_confirmation_contents.html.erb
**/*.{erb,rb}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{erb,rb}: All user-facing strings must use localization (i18n). Always uset()helper. Update locale files for each new or changed element.
Use i18n interpolation for dynamic content:t("users.greeting", name: user.name)
Use Rails pluralization in i18n:t("transactions.count", count:@transactions.count)
Files:
app/views/valuations/confirm_create.html.erbapp/views/valuations/confirm_update.html.erbapp/views/valuations/_confirmation_contents.html.erb
**/*.{js,ts,tsx,erb}
📄 CodeRabbit inference engine (CLAUDE.md)
In Stimulus controllers, use Declarative Actions (HTML declares what happens). Example:
data-action="click->toggle#toggle"
Files:
app/views/valuations/confirm_create.html.erbapp/views/valuations/confirm_update.html.erbapp/views/valuations/_confirmation_contents.html.erb
config/**
📄 CodeRabbit inference engine (AGENTS.md)
Place configuration files in
config/directory
Files:
config/locales/views/categories/en.ymlconfig/locales/views/accounts/en.ymlconfig/locales/views/merchants/en.ymlconfig/locales/views/shared/en.ymlconfig/locales/views/valuations/en.yml
config/locales/**/*.yml
📄 CodeRabbit inference engine (CLAUDE.md)
Organize i18n keys hierarchically by feature:
accounts.index.title,transactions.form.amount_label
Files:
config/locales/views/categories/en.ymlconfig/locales/views/accounts/en.ymlconfig/locales/views/merchants/en.ymlconfig/locales/views/shared/en.ymlconfig/locales/views/valuations/en.yml
app/views/**/_*.html.erb
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use partials only for simple, static HTML with minimal logic in specific contexts
app/views/**/_*.html.erb: Use underscore prefix for partial files (e.g., _trend_change.html.erb, _form_errors.html.erb, _sync_indicator.html.erb)
Place context-specific partials in relevant controller view directory (e.g., accounts/_account_sidebar_tabs.html.erb)
Files:
app/views/valuations/_confirmation_contents.html.erb
🧠 Learnings (17)
📓 Common learnings
Learnt from: jjmata
Repo: we-promise/sure PR: 189
File: app/views/password_resets/new.html.erb:14-14
Timestamp: 2025-10-07T09:51:02.265Z
Learning: In the we-promise/sure repository, use i18n methods (like `t()` or `I18n.t`) for all user-facing strings to maintain proper internationalization infrastructure. While translations should be provided, extensive localization work is not a priority at this stage of development.
Learnt from: CR
Repo: we-promise/sure PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-10T19:11:44.566Z
Learning: Applies to config/locales/**/*.yml : Organize i18n keys hierarchically by feature: `accounts.index.title`, `transactions.form.amount_label`
Learnt from: jjmata
Repo: we-promise/sure PR: 173
File: app/mailers/email_confirmation_mailer.rb:9-9
Timestamp: 2025-10-01T11:40:36.711Z
Learning: In the we-promise/sure repository, i18n (internationalization) is used throughout the application for translations. When making branding configurable (e.g., product names), it's appropriate to add parameters like `product: product_name` to existing `t()` translation calls rather than replacing them with hardcoded strings.
Learnt from: CR
Repo: we-promise/sure PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-10T19:11:44.566Z
Learning: Applies to **/*.{erb,rb} : All user-facing strings must use localization (i18n). Always use `t()` helper. Update locale files for each new or changed element.
Learnt from: CR
Repo: we-promise/sure PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-19T22:25:57.508Z
Learning: Commit messages should use imperative subject line ≤ 72 characters (e.g., 'Add account balance validation'). Include rationale in body and reference issues (`#123`)
Learnt from: CR
Repo: we-promise/sure PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-10T19:11:44.566Z
Learning: Applies to **/*.{erb,rb} : Use i18n interpolation for dynamic content: `t("users.greeting", name: user.name)`
Learnt from: CR
Repo: we-promise/sure PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-10T19:11:44.566Z
Learning: Applies to **/*.{erb,rb} : Use Rails pluralization in i18n: `t("transactions.count", count: transactions.count)`
Learnt from: normanalie
Repo: we-promise/sure PR: 479
File: config/locales/models/address/fr.yml:1-11
Timestamp: 2025-12-20T23:22:25.282Z
Learning: In the we-promise/sure repository, address model translations use a custom i18n structure: {locale}.address.attributes (e.g., fr.address.attributes.country) instead of the standard Rails activerecord.attributes.address namespace. This custom structure is consistent across all locale files (en.yml, fr.yml, nb.yml, tr.yml) in config/locales/models/address/.
Learnt from: CR
Repo: we-promise/sure PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-19T22:25:57.508Z
Learning: Pull requests should include clear description, linked issues, screenshots for UI changes, and migration notes if applicable. Ensure CI passes, tests are added/updated, and rubocop/Biome checks are clean
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .cursor/rules/project-conventions.mdc:0-0
Timestamp: 2025-11-24T16:56:13.406Z
Learning: Applies to **/*.{rb,js,erb} : Format currencies, numbers, dates, and other values server-side, then pass to Stimulus controllers for display only
📚 Learning: 2026-01-10T19:11:44.566Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-10T19:11:44.566Z
Learning: Applies to **/*.{erb,rb} : All user-facing strings must use localization (i18n). Always use `t()` helper. Update locale files for each new or changed element.
Applied to files:
app/views/valuations/confirm_create.html.erbconfig/locales/views/categories/en.ymlconfig/locales/views/accounts/en.ymlapp/views/valuations/confirm_update.html.erbapp/views/valuations/_confirmation_contents.html.erbconfig/locales/views/valuations/en.yml
📚 Learning: 2026-01-10T19:11:44.566Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-10T19:11:44.566Z
Learning: Applies to **/*.{erb,rb} : Use Rails pluralization in i18n: `t("transactions.count", count: transactions.count)`
Applied to files:
app/views/valuations/confirm_create.html.erbapp/views/valuations/confirm_update.html.erbapp/views/valuations/_confirmation_contents.html.erb
📚 Learning: 2025-10-07T09:51:02.265Z
Learnt from: jjmata
Repo: we-promise/sure PR: 189
File: app/views/password_resets/new.html.erb:14-14
Timestamp: 2025-10-07T09:51:02.265Z
Learning: In the we-promise/sure repository, use i18n methods (like `t()` or `I18n.t`) for all user-facing strings to maintain proper internationalization infrastructure. While translations should be provided, extensive localization work is not a priority at this stage of development.
Applied to files:
app/views/valuations/confirm_create.html.erbapp/views/valuations/confirm_update.html.erbapp/views/valuations/_confirmation_contents.html.erb
📚 Learning: 2025-10-01T11:40:36.711Z
Learnt from: jjmata
Repo: we-promise/sure PR: 173
File: app/mailers/email_confirmation_mailer.rb:9-9
Timestamp: 2025-10-01T11:40:36.711Z
Learning: In the we-promise/sure repository, i18n (internationalization) is used throughout the application for translations. When making branding configurable (e.g., product names), it's appropriate to add parameters like `product: product_name` to existing `t()` translation calls rather than replacing them with hardcoded strings.
Applied to files:
app/views/valuations/confirm_create.html.erbapp/views/valuations/confirm_update.html.erbapp/views/valuations/_confirmation_contents.html.erb
📚 Learning: 2025-11-24T16:54:59.198Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-24T16:54:59.198Z
Learning: Applies to app/views/**/*.erb : Prefer semantic HTML elements (dialog, details, summary) over JavaScript components
Applied to files:
app/views/valuations/confirm_create.html.erbapp/views/valuations/confirm_update.html.erb
📚 Learning: 2025-11-24T16:56:13.406Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .cursor/rules/project-conventions.mdc:0-0
Timestamp: 2025-11-24T16:56:13.406Z
Learning: Applies to **/*.html.erb : Prefer native HTML elements over JavaScript-based components. Use semantic HTML: <dialog> for modals, <details><summary> for disclosures
Applied to files:
app/views/valuations/confirm_create.html.erbapp/views/valuations/confirm_update.html.erb
📚 Learning: 2026-01-10T19:11:44.566Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-10T19:11:44.566Z
Learning: Applies to **/*.erb : In view templates, use native HTML elements preferred over JS components: use `<dialog>` for modals, `<details><summary>` for disclosures
Applied to files:
app/views/valuations/confirm_create.html.erbapp/views/valuations/confirm_update.html.erb
📚 Learning: 2026-01-10T19:11:44.566Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-10T19:11:44.566Z
Learning: Applies to config/locales/**/*.yml : Organize i18n keys hierarchically by feature: `accounts.index.title`, `transactions.form.amount_label`
Applied to files:
config/locales/views/categories/en.ymlconfig/locales/views/accounts/en.ymlconfig/locales/views/merchants/en.ymlconfig/locales/views/shared/en.ymlapp/views/valuations/_confirmation_contents.html.erbconfig/locales/views/valuations/en.yml
📚 Learning: 2026-01-09T12:26:07.143Z
Learnt from: jjmata
Repo: we-promise/sure PR: 593
File: app/controllers/pages_controller.rb:200-203
Timestamp: 2026-01-09T12:26:07.143Z
Learning: When checking if a category is "Uncategorized Investments" in app/controllers/pages_controller.rb (or elsewhere), avoid comparing against the hard-coded string ct.category.name != "Uncategorized Investments" because it will break when localization is implemented. Instead, use a model-level predicate like ct.category.uncategorized_investment? or compare object identity like ct.category != Category.uncategorized_investments.
Applied to files:
config/locales/views/categories/en.ymlapp/views/valuations/confirm_update.html.erbapp/views/valuations/_confirmation_contents.html.erb
📚 Learning: 2025-11-14T14:22:37.076Z
Learnt from: jjmata
Repo: we-promise/sure PR: 326
File: app/models/category_import.rb:10-10
Timestamp: 2025-11-14T14:22:37.076Z
Learning: In the CategoryImport model (app/models/category_import.rb), the import process implements a "last write wins" strategy where importing a category.csv file completely overrides existing category structures, including parent relationships. Setting category.parent = nil in the first pass is intentional behavior to ensure the CSV becomes the source of truth.
Applied to files:
config/locales/views/categories/en.yml
📚 Learning: 2025-12-18T07:48:15.175Z
Learnt from: jjmata
Repo: we-promise/sure PR: 464
File: config/locales/views/simplefin_items/zh-CN.yml:36-36
Timestamp: 2025-12-18T07:48:15.175Z
Learning: Enforce the exact branding: always use 'SimpleFIN' (FIN in all caps) and never 'SimpleFin' when referring to the Simple Financial Interchange protocol or SimpleFIN Bridge service across all locale files under config/locales/views. This applies to code, documentation, and translations. Update zh-CN.yml and other locale files as needed; perform a project-wide search/replace to fix inconsistent references and add a review check to catch any future deviations.
Applied to files:
config/locales/views/categories/en.ymlconfig/locales/views/accounts/en.ymlconfig/locales/views/merchants/en.ymlconfig/locales/views/shared/en.ymlconfig/locales/views/valuations/en.yml
📚 Learning: 2025-12-18T08:32:32.373Z
Learnt from: jjmata
Repo: we-promise/sure PR: 464
File: config/locales/views/simplefin_items/zh-CN.yml:36-36
Timestamp: 2025-12-18T08:32:32.373Z
Learning: In YAML locale files, ensure vendor branding (SimpleFIN) appears only in end-user visible strings (UI text, translations, user-facing messages). Internal keys such as logs, class names, and variable identifiers should not include the vendor branding and can use simpler naming like SimpleFin or simplefin. Apply this broadly to all locale files under config/locales.
Applied to files:
config/locales/views/categories/en.ymlconfig/locales/views/accounts/en.ymlconfig/locales/views/merchants/en.ymlconfig/locales/views/shared/en.ymlconfig/locales/views/valuations/en.yml
📚 Learning: 2025-12-20T23:22:25.282Z
Learnt from: normanalie
Repo: we-promise/sure PR: 479
File: config/locales/models/address/fr.yml:1-11
Timestamp: 2025-12-20T23:22:25.282Z
Learning: In the we-promise/sure repository, address model translations use a custom i18n structure: {locale}.address.attributes (e.g., fr.address.attributes.country) instead of the standard Rails activerecord.attributes.address namespace. This custom structure is consistent across all locale files (en.yml, fr.yml, nb.yml, tr.yml) in config/locales/models/address/.
Applied to files:
config/locales/views/accounts/en.ymlconfig/locales/views/shared/en.yml
📚 Learning: 2026-01-10T19:11:44.566Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-10T19:11:44.566Z
Learning: Applies to **/*.{erb,rb} : Use i18n interpolation for dynamic content: `t("users.greeting", name: user.name)`
Applied to files:
app/views/valuations/confirm_update.html.erbapp/views/valuations/_confirmation_contents.html.erb
📚 Learning: 2025-11-24T16:57:20.318Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .cursor/rules/view_conventions.mdc:0-0
Timestamp: 2025-11-24T16:57:20.318Z
Learning: Applies to app/views/**/_*.html.erb : Use underscore prefix for partial files (e.g., _trend_change.html.erb, _form_errors.html.erb, _sync_indicator.html.erb)
Applied to files:
app/views/valuations/confirm_update.html.erb
📚 Learning: 2025-12-19T22:25:57.508Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-19T22:25:57.508Z
Learning: Commit messages should use imperative subject line ≤ 72 characters (e.g., 'Add account balance validation'). Include rationale in body and reference issues (`#123`)
Applied to files:
app/views/valuations/confirm_update.html.erb
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: ci / test
🔇 Additional comments (15)
config/locales/views/merchants/en.yml (1)
12-14: LGTM!The new
family_merchantsection witheditanddeletekeys follows the Rails convention for partial translations and aligns with the hierarchical i18n organization pattern (family_merchants.family_merchant.edit). As per coding guidelines, this correctly organizes keys by feature.config/locales/views/accounts/en.yml (5)
9-10: LGTM!The new translation keys are well-named and appropriately nested under
accounts.account. The lowercase style fordeletion_in_progresssuits a status indicator, whilecomplete_setupuses proper title case for a button label.
43-49: LGTM!The container keyboard navigation hints are well-structured under
accounts.new.container, following the hierarchical organization guideline. The keys are concise and appropriate for UI navigation labels.
74-74: LGTM!Flattening
search.placeholdertosearch_placeholderis appropriate when there's a single search-related key. The naming is clear and follows conventions.
80-83: LGTM!The new chart labels are well-organized under
accounts.show.chart. The distinction betweenno_datahere anddata_not_availableunderaccounts.chartis appropriate given the different messages and contexts.
93-94: LGTM!Renaming
importtoimport_transactionsimproves clarity and avoids potential keyword conflicts. Thedelete_accountkey follows the established menu item naming pattern.config/locales/views/valuations/en.yml (1)
31-58: Well-structured i18n keys for valuations.The new locale keys are properly organized hierarchically by feature, following the project's i18n conventions. The
action_verbsgroup correctly addresses previous review feedback about hardcoded strings.app/views/valuations/confirm_update.html.erb (1)
2-15: Previous review feedback addressed — action verb is now localized.The hardcoded
action_verb: "update"has been replaced witht("valuations.action_verbs.update"), correctly resolving the i18n interpolation issue flagged in the previous review. All user-facing strings now use thet()helper.app/views/valuations/confirm_create.html.erb (1)
2-16: Previous review feedback addressed — action verb is now localized.The hardcoded
action_verb: "set"has been replaced witht("valuations.action_verbs.set"), correctly resolving the i18n issue flagged in the previous review. All strings properly use thet()helper.app/views/valuations/_confirmation_contents.html.erb (2)
8-23: Investment branch properly localized.The investment intro uses correct i18n interpolation with
actionanddateparameters. Labels for monetary values are properly translated while preserving the formatted amounts.
25-46: Previous review feedback addressed — no more.capitalizeon action_verb.The non-investment branch now uses
action_verbdirectly without calling.capitalize, addressing the locale-awareness concern from the previous review. The account type labels are properly localized.config/locales/views/shared/en.yml (1)
3-5: The breadcrumb key structure is correct as-is. The family_merchants_controller.rb already references:"breadcrumbs.home"and:"breadcrumbs.merchants"(root-level keys), which match the keys in the locale files. Both en.yml and fr.yml have breadcrumbs consistently at the root level, not nested undershared, and this is the intended design. Moving them undershared:as suggested in the original review would break the existing code.Likely an incorrect or invalid review comment.
config/locales/views/categories/en.yml (3)
13-14: Nice addition for bulk-delete feedback.Clear success message and consistent placement under
categories.destroy_all.
34-34: Good coverage for the delete-all label.The
categories.index.delete_allkey looks consistent with surrounding entries.
48-50: Dropdown copy additions look good.Keys are clear and map cleanly to the new dropdown UI text.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| <% end %> | ||
|
|
||
| <p>All future transactions and balances will be recalculated based on this <%= is_update ? "change" : "update" %>.</p> | ||
| <p><%= is_update ? t("valuations.confirmation_contents.recalculation_change") : t("valuations.confirmation_contents.recalculation_update") %></p> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's look at the file and its context
cat -n app/views/valuations/_confirmation_contents.html.erb | head -60Repository: we-promise/sure
Length of output: 3029
🏁 Script executed:
# Check where is_update is being set from - look for callers of this partial
rg "_confirmation_contents" --type erb -A 2 -B 2Repository: we-promise/sure
Length of output: 41
🏁 Script executed:
# Check the locale files for the messages
fd -type f -name "*.yml" -path "*/config/locales/*" | xargs rg -l "valuations"Repository: we-promise/sure
Length of output: 2301
🏁 Script executed:
# Read the valuations locale file to see the actual messages
cat config/locales/views/valuations/en.ymlRepository: we-promise/sure
Length of output: 1884
🏁 Script executed:
# Find all views that use this partial and how they pass is_update
rg "_confirmation_contents" app/views/ -A 3 -B 3Repository: we-promise/sure
Length of output: 41
🏁 Script executed:
# Check the valuations controller to understand is_update logic
cat -n app/controllers/valuations_controller.rbRepository: we-promise/sure
Length of output: 3294
🏁 Script executed:
# Check the confirm_create view to see how is_update is passed
cat -n app/views/valuations/confirm_create.html.erbRepository: we-promise/sure
Length of output: 839
🏁 Script executed:
# Check the confirm_update view to see how is_update is passed
cat -n app/views/valuations/confirm_update.html.erbRepository: we-promise/sure
Length of output: 843
Conditional logic is inverted for recalculation messages.
When is_update is true (update action), the code displays recalculation_change ("based on this change"), but should display recalculation_update ("based on this update"). Similarly, when is_update is false (create action with action verb "set"), it should display recalculation_change, not recalculation_update. The action verb and the recalculation message should be semantically aligned.
Suggested fix
- <p><%= is_update ? t("valuations.confirmation_contents.recalculation_change") : t("valuations.confirmation_contents.recalculation_update") %></p>
+ <p><%= is_update ? t("valuations.confirmation_contents.recalculation_update") : t("valuations.confirmation_contents.recalculation_change") %></p>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <p><%= is_update ? t("valuations.confirmation_contents.recalculation_change") : t("valuations.confirmation_contents.recalculation_update") %></p> | |
| <p><%= is_update ? t("valuations.confirmation_contents.recalculation_update") : t("valuations.confirmation_contents.recalculation_change") %></p> |
🤖 Prompt for AI Agents
In `@app/views/valuations/_confirmation_contents.html.erb` at line 49, The
conditional is inverted: the view uses is_update to choose the wrong translation
keys; update the template so when is_update is true it renders
t("valuations.confirmation_contents.recalculation_update") and when false it
renders t("valuations.confirmation_contents.recalculation_change") — locate the
ERB line that references is_update and the t(...) keys and swap the branches so
the action verb and recalculation message are semantically aligned.
|
@jjmata I've addressed all the CodeRabbit review comments: Fixes applied:
All CI checks are now passing ✅ PRs #668, #669, #670 have been rebased on this branch and are ready to be merged in order. |
|
Note about CI on PRs #668, #669, #670: These PRs show failing tests - this is expected behavior. Here's why: The 4 PRs form a dependency chain: Each PR adds i18n keys that are referenced in code added by subsequent PRs. When GitHub CI runs tests on #668, it tests against Once merged in order, each subsequent PR will pass:
This is a standard approach for splitting large i18n changes into reviewable chunks while avoiding a single massive PR. |
|
@jjmata This is Part 1/4 of the i18n effort to extract hardcoded strings. The full series is:
Each PR builds on the previous one (dependency chain). Questions for you:
Happy to adjust the approach based on your feedback. Let me know! |
|
Not ignoring your i18n PRs, @foXaCe ... just trying to find the right time to land them in |
|
Thinking we might release |
|
Hi, yeah, I know—it's a good idea. There's also lokalise.com |
2 participants
Summary
Test plan
bin/rails testSummary by CodeRabbit
New Features
Refactor
✏️ Tip: You can customize this high-level summary in your review settings.