feat(i18n): add Italian locale (it) #396
Conversation
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughAdds Italian locale files, replaces many hard-coded UI strings and breadcrumb labels with I18n lookups, updates supported locales, refactors Period to use I18n for labels and adds "last_month", tweaks bulk-select UI text, and updates .gitignore. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~40 minutes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
app/views/settings/_settings_nav.html.erb (1)
88-88: Localize the mobile "Back" link.The desktop "Back" link on line 51 has been localized, but the mobile navigation on line 88 still uses a hardcoded "Back" string. This creates an incomplete i18n implementation where Italian users on mobile devices would still see English text.
Apply this diff to complete the localization:
- text: "Back", + text: t("settings.settings_nav_link_large.previous", default: "Back"),app/controllers/settings/profiles_controller.rb (1)
32-34: Localize flash messages for consistency.Lines 32 and 34 contain hard-coded English strings, while lines 16 and 24 properly use i18n for flash messages. For consistency and to support Italian locale, these should also be localized.
Apply this diff:
if @user.destroy # Also destroy the invitation associated with this user for this family Current.family.invitations.find_by(email: @user.email)&.destroy - flash[:notice] = "Member removed successfully." + flash[:notice] = t("settings.profiles.destroy.success") else - flash[:alert] = "Failed to remove member." + flash[:alert] = t("settings.profiles.destroy.failure") endBased on coding guidelines requiring all user-facing strings to use localization.
app/views/budget_categories/show.html.erb (2)
34-60: Complete the i18n migration for remaining hardcoded strings.Several user-facing strings in the Overview section remain hardcoded and should be localized per coding guidelines:
- Line 34:
"%b %Y spending"- Line 43:
"Status"- Lines 48, 54, 60:
"overspent","left"(appears twice)As per coding guidelines, all user-facing strings must use the
t()helper. Add these keys toconfig/locales/views/budget_categories/[locale].ymland update the view accordingly.
134-136: Localize the empty state message.The "No transactions found" message should be localized to support the Italian locale being added in this PR.
As per coding guidelines, all user-facing strings must use the
t()helper.Apply this diff:
- <p class="text-secondary text-sm mb-4"> - No transactions found for this budget period. - </p> + <p class="text-secondary text-sm mb-4"> + <%= t('budget_categories.labels.no_transactions', default: "No transactions found for this budget period.") %> + </p>
🟡 Minor comments (15)
config/locales/views/cryptos/it.yml-7-7 (1)
7-7: Fix typo in Italian translation."Insersici" contains an extra 's' and should be "Inserisci" (the imperative form of the Italian verb "inserire").
Apply this diff:
- title: Insersici il saldo del conto + title: Inserisci il saldo del contoconfig/locales/views/settings/securities/it.yml-6-10 (1)
6-10: Inconsistent capitalization in Italian translations.Line 10 uses "Autenticazione a Due Fattori" with a capitalized "Due", while line 6 uses lowercase "due". For consistency and proper Italian grammar, standardize to lowercase: "Autenticazione a due fattori" (only the first word capitalized in titles).
Apply this diff:
- mfa_title: Autenticazione a Due Fattori + mfa_title: Autenticazione a due fattoriconfig/locales/views/pages/it.yml-17-39 (1)
17-39: Fix a few Italian typos and minor wording issues in dashboard copyThere are some small spelling/grammar glitches in user-facing strings:
no_data_title:"flusso di casssa"→"flusso di cassa","period"→"periodo".no_data_description:"questo period"→"questo periodo".add_transaction:"trasazione"→"transazione".add_accounts:"Aggiunti i tuoi"→"Aggiungi i tuoi".Suggested patch:
balance_sheet: asset: Attività liability: Passività no_items: "Ancora nessun %{name}" - add_accounts: "Aggiunti i tuoi %{name} per visualizzare un riepilogo completo" + add_accounts: "Aggiungi i tuoi %{name} per visualizzare un riepilogo completo" name: Nome value: Valore weight: Peso cashflow_sankey: - title: "Flusso di cassa" - no_data_title: "Nessun dato per il flusso di casssa per questo period" - no_data_description: "Aggiungi transazioni per visualizzare i dati del flusso di cassa oppure estendi il periodo" - add_transaction: "Aggiungi trasazione" + title: "Flusso di cassa" + no_data_title: "Nessun dato per il flusso di cassa per questo periodo" + no_data_description: "Aggiungi transazioni per visualizzare i dati del flusso di cassa oppure estendi il periodo" + add_transaction: "Aggiungi transazione"These keep the intent while polishing the Italian shown in the UI.
config/locales/models/user/it.yml-20-20 (1)
20-20: Standardize capitalization in error message.The error message on line 20 should start with a capital letter for consistency. Italian error messages typically begin with uppercase.
- invalid_file_size: la dimensione del file deve essere inferiore a %{max_megabytes}MB + invalid_file_size: La dimensione del file deve essere inferiore a %{max_megabytes}MBconfig/locales/views/depositories/it.yml-10-10 (1)
10-10: Fix apparent Italian typo.Line 10 contains "Insersici il saldo del conto" which appears to be a typo. The correct Italian imperative form should be "Inserisci il saldo del conto".
- title: Insersici il saldo del conto + title: Inserisci il saldo del contoconfig/locales/doorkeeper.it.yml-12-19 (1)
12-19: Incomplete translations: Error messages remain in English.The ActiveRecord error messages for
doorkeeper/applicationattributes are still in English rather than Italian. While the PR objectives acknowledge incomplete translations, these user-facing error messages should be prioritized for translation.Examples needing translation:
- "cannot contain a fragment." → "non può contenere un frammento."
- "must be a valid URI." → "deve essere un URI valido."
- "must specify a scheme." → "deve specificare uno schema."
- etc.
config/locales/views/shared/it.yml-16-16 (1)
16-16: Fix Italian translation: "merchants" should be "Esercenti" or "Commercianti", not "Membri""Membri" means "Members" in Italian and is incorrect for "merchants". Use "Esercenti" for business/financial/payment contexts (e.g., merchant services, merchant accounts) or "Commercianti" for general trading/shopkeeper contexts. Given the shared views locale, "Esercenti" is likely the appropriate choice.
config/locales/views/holdings/it.yml-25-25 (1)
25-25: Fix typo: "Canella" should be "Cancella".There's a typo in the Italian translation where "Canella" should be "Cancella" (delete).
Apply this diff to fix the typo:
- delete: Canella + delete: Cancellaconfig/locales/views/simplefin_items/it.yml-25-25 (1)
25-25: Fix typo: "tokenn" should be "token".There's a typo in the Italian translation where "tokenn" appears with a double 'n'.
Apply this diff to fix the typo:
- token_compromised: Il tokenn di setup potrebbe essere compromesso, scaduto o già utilizzato. Creane uno nuovo. + token_compromised: Il token di setup potrebbe essere compromesso, scaduto o già utilizzato. Creane uno nuovo.config/locales/views/reports/it.yml-16-16 (1)
16-16: Correct typo inshowing_periodItalian stringSmall typo in the reports heading:
- showing_period: "Stai visulizzando i dati dal %{start} al %{end}" + showing_period: "Stai visualizzando i dati dal %{start} al %{end}"Placeholders remain unchanged and correct.
config/locales/views/accounts/it.yml-14-101 (1)
14-101: Fix mixed-language and grammar issues in Italian account view translationsThere are a few user-facing language glitches worth correcting:
- Line 14:
successis partially in English.- Line 51: wrong past participle agreement (
trovatavsmovimento).- Line 99: wrong article (
Tutte i movimenti).Suggested patch:
@@ destroy: - success: "Conto %{type} scheduled for deletion" + success: "Conto %{type} programmato per l'eliminazione" @@ activity: @@ - no_entries: Nessun movimento trovata + no_entries: Nessun movimento trovato @@ warning_transactions_kept: Tutte i movimenti e i saldi esistenti saranno conservati. + warning_transactions_kept: Tutti i movimenti e i saldi esistenti saranno conservati.This keeps semantics while fixing language quality.
Committable suggestion skipped: line range outside the PR's diff.
config/locales/views/transactions/it.yml-100-100 (1)
100-100: Incorrect translation: "membri" should be "commercianti".The translation "Modifica membri" means "Edit members" rather than "Edit merchants". The correct Italian word for merchants is "commercianti" or "esercenti".
Apply this diff:
- edit_merchants: Modifica membri + edit_merchants: Modifica commercianticonfig/locales/views/imports/it.yml-65-65 (1)
65-65: Remove trailing space.Line 65 has a trailing space after "Ripristino".
Apply this diff:
- reverting: Ripristino + reverting: Ripristinoconfig/locales/views/imports/it.yml-59-59 (1)
59-59: Fix typo: "Comletata" should be "Completata".The word is missing the 'p'.
Apply this diff:
- complete: Comletata + complete: Completataconfig/locales/views/category/deletions/it.yml-10-10 (1)
10-10: Fix typo: "essegnarere" should be "assegnare".The verb is misspelled. It should be "assegnare" (to assign).
Apply this diff:
- delete_and_recategorize: Elimina "%{category_name}" e essegnarere una nuova categoria + delete_and_recategorize: Elimina "%{category_name}" e assegnare una nuova categoria
🧹 Nitpick comments (10)
config/locales/views/mfa/it.yml (1)
5-6: Remove redundant key prefixes within nested sections.Keys
backup_codes_descriptionandbackup_codes_titleduplicate thebackup_codes:prefix. Since they're already nested, they should be nameddescriptionandtitleinstead to avoid redundancy.Apply this diff to fix the redundant key names:
mfa: backup_codes: - backup_codes_description: Ogni codice può essere utilizzato una sola volta. Conserva questi codici in un luogo sicuro. - backup_codes_title: I tuoi codici di backup + description: Ogni codice può essere utilizzato una sola volta. Conserva questi codici in un luogo sicuro. + title: I tuoi codici di backup continue: Torna alle impostazioni di sicurezzaThis aligns with the hierarchical i18n key pattern used in the rest of the file and the codebase. Based on coding guidelines, keys should follow the feature-based hierarchy without repeating the parent key name.
config/locales/views/imports/en.yml (1)
39-50: Consider using interpolation for product name.The branding update from "Maybe's" to "Sure's" is noted. However, based on the project's learnings about making branding configurable, consider using interpolation for these strings to make the product name configurable.
For example, line 39 could become:
- account_mapping_description: Assign all of your imported file's accounts to - Sure's existing accounts. You can also add new accounts or leave them - uncategorized. + account_mapping_description: Assign all of your imported file's accounts to + %{product_name}'s existing accounts. You can also add new accounts or leave them + uncategorized.Apply similar changes to lines 43, 46, and 49. This would allow the product name to be configured dynamically via the
t()helper with aproduct_nameparameter.Based on learnings about making branding configurable in this repository.
config/locales/doorkeeper.it.yml (2)
1-1: Address TODO: Complete Italian translations.The file contains a TODO comment indicating incomplete translations. As noted in the PR objectives, this is acknowledged, but it should be tracked for follow-up work.
Do you want me to open a tracking issue for completing the Doorkeeper Italian translations?
32-37: Incomplete translations: Help text remains in English.The form help text for OAuth applications is still in English and should be translated to Italian for consistency.
app/javascript/controllers/bulk_select_controller.js (1)
30-33: Selection bar text now better supports i18n; consider localizing the “Edit” headerThe updated selection bar text:
this.selectionBarTextTarget.innerText = `${count} ${this._pluralizedResourceName()}`;correctly relies on
singularLabel/pluralLabelvalues, which can be provided from the server as localized strings—this is an improvement for i18n.As a follow-up, the bulk edit drawer header still hardcodes
"Edit":headingTextEl.innerText = `Edit ${ this.selectedIdsValue.length } ${this._pluralizedResourceName()}`;To fully localize this, consider passing a translated prefix via a Stimulus value:
static values = { singularLabel: String, pluralLabel: String, + editLabel: String, selectedIds: { type: Array, default: [] }, }; bulkEditDrawerHeaderTargetConnected(element) { const headingTextEl = element.querySelector("h2"); - headingTextEl.innerText = `Edit ${ - this.selectedIdsValue.length - } ${this._pluralizedResourceName()}`; + headingTextEl.innerText = `${this.editLabelValue} ${ + this.selectedIdsValue.length + } ${this._pluralizedResourceName()}`; }Then set
data-bulk-select-edit-label-valuefrom the translated string in the view.Also applies to: 132-135
config/locales/views/layout/en.yml (1)
13-18: Consider nesting navigation under layouts for consistency.The new
navigationkeys are at the top level, while existing keys in this file followlayouts.auth.*andlayouts.shared.footer.*structure. For better organization and consistency with hierarchical i18n conventions, consider nesting aslayouts.navigation.*instead.app/views/transactions/_summary.html.erb (1)
4-4: Consider using plural form for consistency.The key
total_transactionis singular, but it represents multiple transactions (showingtotals.count). Consider renaming tototal_transactionsfor better semantic clarity.app/controllers/imports_controller.rb (1)
17-20: Breadcrumb label i18n looks good; consider following up on remaining stringsUsing
t("shared.breadcrumbs.import_export", default: "Import/Export")for the breadcrumb label is aligned with the i18n conventions and the rest of this PR. For consistency, consider a follow-up to also move the"Home"label and the various notices/alerts in this controller into locale files so all user-facing strings are centralized.Based on learnings, i18n should eventually cover all user-visible text, but incremental cleanup in separate PRs is acceptable.
app/views/layouts/application.html.erb (1)
1-7: Navigation item localization is aligned with layout localesUsing
t("navigation.*")for the nav item names is consistent with the new layout locale files and keeps navigation fully i18n-able. As a later cleanup, you might also extract remaining layout strings (e.g., “Free trial”, “Upgrade”) into locales for complete coverage, but the current changes are solid.app/views/transactions/index.html.erb (1)
50-51: Refactor bulk select labels to avoid translation concatenation.Concatenating separate translations (
transaction+selected) is problematic for i18n because different languages may require different word orders or phrasing. For example, some languages might say "X selected transactions" while others say "transactions selected: X".Create unified translation keys that handle the complete phrase with interpolation:
- data-bulk-select-singular-label-value="<%= t(".transaction", default: "transaction", count: 1 ) %> <%= t("transactions.selection_bar.selected", default: "selected", count: 1 ) %>" - data-bulk-select-plural-label-value="<%= t(".transaction", default: "transactions", count: 2 ) %> <%= t("transactions.selection_bar.selected", default: "selected", count: 2 ) %>" + data-bulk-select-singular-label-value="<%= t("transactions.selection_bar.count_selected", count: 1, default: "transaction selected") %>" + data-bulk-select-plural-label-value="<%= t("transactions.selection_bar.count_selected", count: 2, default: "transactions selected") %>"Then update the locale files to use a single key with proper pluralization:
transactions: selection_bar: count_selected: one: "%{count} movimento selezionato" other: "%{count} movimenti selezionati"
|
There's a few conflicts and review comments here, @Startype-design ... can you take a look before @alessiocappa does a final review? 🙏 |
alessiocappa
left a comment
alessiocappa
left a comment
There was a problem hiding this comment.
@Startype-design, thanks a lot for the PR. As an Italian, it’s fantastic to see the app translated into my native language! 😃
It’s definitely a great starting point, even though some translations are still missing as you mentioned in the PR description. I’ve left a few comments for incorrect translations and believe some points highlighted by CodeRabbit are worth checking out, such as files with incorrect locale keys which could be problematic.
If you can double-check the comments and fix those issues, we can merge the PR and refine it in future ones.
Another aspect that feels slightly off is the labels in the groups section of the homepage:
I think they should be plural instead of singular (e.g. Conti, Investimenti, Immobili, etc.) but I’m not sure if it’s something already manageable.
| @@ -0,0 +1,155 @@ | |||
| it: # TODO : translate | |||
There was a problem hiding this comment.
Lot of missing translations here. Probably it's better to translate them if we want to keep the file.
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/models/balance_sheet/classification_group.rb (1)
10-10: Remove unused instance variable assignment.Line 10 assigns
@name = name, but the@nameinstance variable is never used elsewhere in the class. Thenamemethod (lines 15-19) doesn't reference it and is called directly each time. This assignment is dead code.Apply this diff to remove the unused assignment:
def initialize(classification:, currency:, accounts:) @classification = normalize_classification!(classification) - @name = name @currency = currency @accounts = accounts end
🧹 Nitpick comments (3)
app/views/accounts/_accountable_group.html.erb (1)
54-57: Preferthelper, add safer fallback, and avoid forcing.downcaseon localized labels
- In views we generally prefer the
t(...)helper overI18n.t(...)for user-facing strings.account_group.accountable_type.nameassumes.accountable_typeresponds to.name; usingto_s.underscoreis safer and works whether it’s a String or class.- If
accounts.types.#{group_type}is missing, the user will see a raw key or error; adding adefault:fallback (e.g.,account_group.name) makes this more robust.- Forcing
.downcaseon a translated label can break abbreviations/proper nouns (“ETF” → “etf”, etc.); casing is usually better controlled directly in the translation text.A small refactor that keeps the behavior but tightens things up:
- <% - group_type = account_group.accountable_type.name.underscore - account_group_name = I18n.t("accounts.types.#{group_type}", count: 1) - %> + <% + group_type = account_group.accountable_type.to_s.underscore + account_group_name = t("accounts.types.#{group_type}", + count: 1, + default: account_group.name) + %> @@ - text: t("accounts.sidebar.new_account_group", account_group: account_group_name.downcase), + text: t("accounts.sidebar.new_account_group", account_group: account_group_name),This keeps everything i18n-driven while avoiding case mangling in the view and giving you a safe fallback if a type translation is missing. As per coding guidelines, …
Also applies to: 60-60
app/controllers/settings/providers_controller.rb (2)
8-12: Breadcrumb i18n key change looks good; consider localizing “Home” as wellSwitching to
t("shared.breadcrumbs.providers", default: "Bank Sync Providers")resolves the earlier conflict with the profile breadcrumb key and aligns with the i18n work in this PR. Minor follow-up: the"Home"label is still hard-coded; consider using an i18n key (e.g.t("shared.breadcrumbs.home")) or the shared breadcrumbable mechanism for consistency, and ensure the newshared.breadcrumbs.providers(and home) keys exist in bothenanditlocales.
64-75: Localize remaining user-facing notices and alertsThe success, no-op, failure, and authorization messages are still hard-coded English strings. To fully align with the repository’s i18n guidelines, consider moving them into locale files, for example:
- redirect_to settings_providers_path, notice: "Provider settings updated successfully" + redirect_to settings_providers_path, notice: t("settings.providers.update.success") - redirect_to settings_providers_path, notice: "No changes were made" + redirect_to settings_providers_path, notice: t("settings.providers.update.no_changes") - flash.now[:alert] = "Failed to update provider settings: #{error.message}" + flash.now[:alert] = t("settings.providers.update.failure", message: error.message) - redirect_to settings_providers_path, alert: "Not authorized" unless Current.user.admin? + redirect_to settings_providers_path, alert: t("settings.providers.not_authorized") unless Current.user.admin?and then defining the corresponding keys under
settings.providersinen.ymlandit.yml.Also applies to: 94-96
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (21)
.gitignore(1 hunks)app/controllers/concerns/breadcrumbable.rb(1 hunks)app/controllers/settings/providers_controller.rb(1 hunks)app/models/balance_sheet/classification_group.rb(2 hunks)app/views/accounts/_accountable_group.html.erb(1 hunks)config/locales/mailers/invitation_mailer/it.yml(1 hunks)config/locales/views/accounts/it.yml(1 hunks)config/locales/views/email_confirmation_mailer/it.yml(1 hunks)config/locales/views/entries/it.yml(1 hunks)config/locales/views/family_exports/it.yml(1 hunks)config/locales/views/holdings/it.yml(1 hunks)config/locales/views/imports/en.yml(1 hunks)config/locales/views/imports/it.yml(1 hunks)config/locales/views/merchants/it.yml(1 hunks)config/locales/views/other_assets/it.yml(1 hunks)config/locales/views/reports/it.yml(1 hunks)config/locales/views/settings/it.yml(1 hunks)config/locales/views/shared/it.yml(1 hunks)config/locales/views/tag/deletions/it.yml(1 hunks)config/locales/views/tags/it.yml(1 hunks)config/locales/views/transactions/it.yml(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (13)
- config/locales/views/family_exports/it.yml
- config/locales/mailers/invitation_mailer/it.yml
- config/locales/views/accounts/it.yml
- config/locales/views/imports/it.yml
- config/locales/views/settings/it.yml
- config/locales/views/tag/deletions/it.yml
- config/locales/views/merchants/it.yml
- app/controllers/concerns/breadcrumbable.rb
- config/locales/views/reports/it.yml
- config/locales/views/shared/it.yml
- config/locales/views/transactions/it.yml
- config/locales/views/entries/it.yml
- config/locales/views/other_assets/it.yml
🧰 Additional context used
📓 Path-based instructions (25)
config/locales/**/*.yml
📄 CodeRabbit inference engine (CLAUDE.md)
Use hierarchical i18n keys by feature:
accounts.index.title,transactions.form.amount_label. Uset()helper for all user-facing strings with interpolation for dynamic content.
Files:
config/locales/views/holdings/it.ymlconfig/locales/views/tags/it.ymlconfig/locales/views/imports/en.ymlconfig/locales/views/email_confirmation_mailer/it.yml
config/**/*.{rb,yml,yaml}
📄 CodeRabbit inference engine (AGENTS.md)
Configuration files should be placed in
config/directory, with environment examples in.env.local.exampleand.env.test.example
Files:
config/locales/views/holdings/it.ymlconfig/locales/views/tags/it.ymlconfig/locales/views/imports/en.ymlconfig/locales/views/email_confirmation_mailer/it.yml
**/*.rb
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.rb: Application supports two modes: 'managed' and 'self_hosted' via Rails.application.config.app_mode
Use Current.user and Current.family instead of current_user / current_family for authentication context
Optimize database queries with proper indexes to prevent N+1 queries using includes/joins
**/*.rb: Use Rails pluralization in i18n:t("transactions.count", count: @transactions.count).
Runbin/rubocop -f github -afor Ruby linting with auto-correct before pull requests.
**/*.rb: Ruby code should use 2-space indent,snake_casefor methods and variables, andCamelCasefor classes and modules, following Rails conventions for folders and file names
Prefer environment variables over hard-coded values for configuration
Files:
app/models/balance_sheet/classification_group.rbapp/controllers/settings/providers_controller.rb
app/models/**/*.rb
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
app/models/**/*.rb: Keep business logic in models using skinny controllers and fat models pattern
Store all monetary values in base currency (user's primary currency) and use Money objects for conversion
Use ActiveRecord validations for form convenience and complex business logic validations
Use Rails concerns and POROs for organizing business logic instead of services folder
app/models/**/*.rb: All monetary values stored in base currency (user's primary currency). UseMoneyobjects for currency conversion and formatting.
Place business logic inapp/models/folder. Avoid creatingapp/services/folder. Use Rails concerns and POROs for organization.
Models should answer questions about themselves using instance methods: useaccount.balance_seriesnotAccountSeries.new(account).call.
Files:
app/models/balance_sheet/classification_group.rb
**/*.{rb,erb,html.erb}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{rb,erb,html.erb}: UseCurrent.userfor the current user context in controllers and views. Do NOT usecurrent_user.
UseCurrent.familyfor the current family context. Do NOT usecurrent_family.
Files:
app/models/balance_sheet/classification_group.rbapp/views/accounts/_accountable_group.html.erbapp/controllers/settings/providers_controller.rb
**/*.{erb,html.erb,rb}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{erb,html.erb,rb}: All user-facing strings must use localization (i18n) via thet()helper. Update locale files for each new or changed element.
Use server-side formatting for currencies, numbers, and dates. Do not format in JavaScript.
Files:
app/models/balance_sheet/classification_group.rbapp/views/accounts/_accountable_group.html.erbapp/controllers/settings/providers_controller.rb
{app/models/**/*.rb,db/migrate/**/*.rb}
📄 CodeRabbit inference engine (CLAUDE.md)
Simple validations (null checks, unique indexes) should be in the database. ActiveRecord validations for form convenience (prefer client-side when possible). Complex validations and business logic in ActiveRecord.
Files:
app/models/balance_sheet/classification_group.rb
**/*.{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/models/balance_sheet/classification_group.rbapp/views/accounts/_accountable_group.html.erbapp/controllers/settings/providers_controller.rb
**/*.{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/models/balance_sheet/classification_group.rbapp/views/accounts/_accountable_group.html.erbapp/controllers/settings/providers_controller.rb
**/*.{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/models/balance_sheet/classification_group.rbapp/views/accounts/_accountable_group.html.erbapp/controllers/settings/providers_controller.rb
app/**/*.{rb,js,ts,jsx,tsx,css,scss}
📄 CodeRabbit inference engine (AGENTS.md)
Code should be organized in
app/directory (Rails MVC, services, jobs, mailers, components), with JS inapp/javascript/, and styles/assets inapp/assets/(Tailwind, images, fonts)
Files:
app/models/balance_sheet/classification_group.rbapp/controllers/settings/providers_controller.rb
app/**/*.rb
📄 CodeRabbit inference engine (AGENTS.md)
Run
bin/brakemansecurity scan before major PRs to check for static analysis of common Rails issues
Files:
app/models/balance_sheet/classification_group.rbapp/controllers/settings/providers_controller.rb
{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/accounts/_accountable_group.html.erb
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/accounts/_accountable_group.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-sideViews should use ERB and avoid heavy logic; prefer helpers and components instead (ERB checked by
erb-lintper.erb_lint.yml)
Files:
app/views/accounts/_accountable_group.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/accounts/_accountable_group.html.erb
**/*.{erb,html,vue,jsx,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{erb,html,vue,jsx,tsx}: Use Tailwind CSS v4.x with custom design system defined inapp/assets/tailwind/maybe-design-system.css. Always use functional tokens (e.g.,text-primarynottext-white).
Prefer semantic HTML elements over JavaScript components (e.g., use<dialog>for modals,<details><summary>for disclosures).
Files:
app/views/accounts/_accountable_group.html.erb
**/*.{erb,html.erb}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{erb,html.erb}: Useiconhelper for icons, never uselucide_icondirectly.
Leverage Turbo frames for page sections over client-side solutions.
Pass data to Stimulus controllers viadata-*-valueattributes, not inline JavaScript.
**/*.{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/accounts/_accountable_group.html.erb
app/views/**/_*.{erb,html.erb}
📄 CodeRabbit inference engine (CLAUDE.md)
Use Partials when element is primarily static HTML with minimal logic, used in only one or few contexts, is simple template content, or doesn't need variants/sizes/complex configuration.
Files:
app/views/accounts/_accountable_group.html.erb
{app/components/**/*.html.erb,app/views/**/*.{erb,html.erb}}
📄 CodeRabbit inference engine (CLAUDE.md)
Keep domain logic out of view templates. Logic belongs in component files, not template files.
Files:
app/views/accounts/_accountable_group.html.erb
**/*.erb
📄 CodeRabbit inference engine (CLAUDE.md)
Run
bundle exec erb_lint ./app/**/*.erb -afor ERB linting with auto-correct before pull requests.
Files:
app/views/accounts/_accountable_group.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/accounts/_accountable_group.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/accounts/_accountable_group.html.erb
{app/views/**,app/helpers/**}
📄 CodeRabbit inference engine (.cursor/rules/ui-ux-design-guidelines.mdc)
Always generate semantic HTML
Files:
app/views/accounts/_accountable_group.html.erb
app/controllers/**/*.rb
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use Rails strong parameters and CSRF protection throughout the application
app/controllers/**/*.rb: Use strong parameters and CSRF protection throughout the application.
Use query params for state over localStorage/sessions.
Files:
app/controllers/settings/providers_controller.rb
🧠 Learnings (11)
📓 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: 2025-11-24T16:55:43.069Z
Learning: Applies to **/*.{erb,html.erb,rb} : All user-facing strings must use localization (i18n) via the `t()` helper. Update locale files for each new or changed element.
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.
📚 Learning: 2025-11-24T16:55:43.069Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:55:43.069Z
Learning: Applies to **/*.{erb,html.erb,rb} : All user-facing strings must use localization (i18n) via the `t()` helper. Update locale files for each new or changed element.
Applied to files:
config/locales/views/holdings/it.ymlapp/models/balance_sheet/classification_group.rbapp/views/accounts/_accountable_group.html.erbapp/controllers/settings/providers_controller.rbconfig/locales/views/tags/it.ymlconfig/locales/views/imports/en.ymlconfig/locales/views/email_confirmation_mailer/it.yml
📚 Learning: 2025-11-24T16:55:43.069Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:55:43.069Z
Learning: Applies to config/locales/**/*.yml : Use hierarchical i18n keys by feature: `accounts.index.title`, `transactions.form.amount_label`. Use `t()` helper for all user-facing strings with interpolation for dynamic content.
Applied to files:
config/locales/views/holdings/it.ymlapp/models/balance_sheet/classification_group.rbapp/views/accounts/_accountable_group.html.erbapp/controllers/settings/providers_controller.rbconfig/locales/views/tags/it.ymlconfig/locales/views/imports/en.ymlconfig/locales/views/email_confirmation_mailer/it.yml
📚 Learning: 2025-11-24T16:55:43.069Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:55:43.069Z
Learning: Applies to **/*.rb : Use Rails pluralization in i18n: `t("transactions.count", count: transactions.count)`.
Applied to files:
app/models/balance_sheet/classification_group.rbapp/controllers/settings/providers_controller.rb
📚 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/controllers/settings/providers_controller.rbconfig/locales/views/imports/en.ymlconfig/locales/views/email_confirmation_mailer/it.yml
📚 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/controllers/settings/providers_controller.rbconfig/locales/views/imports/en.yml
📚 Learning: 2025-11-24T16:55:51.094Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .cursor/rules/cursor_rules.mdc:0-0
Timestamp: 2025-11-24T16:55:51.094Z
Learning: Applies to .cursor/rules/*.mdc : Cursor rule files must have the required frontmatter structure with 'description', 'globs', and 'alwaysApply' fields, followed by markdown content with main points in bold and sub-points with details
Applied to files:
.gitignore
📚 Learning: 2025-11-24T16:55:51.094Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .cursor/rules/cursor_rules.mdc:0-0
Timestamp: 2025-11-24T16:55:51.094Z
Learning: Applies to .cursor/rules/*.mdc : Use file reference syntax `[filename](mdc:path/to/file)` for rule references and `[filename](mdc:path/to/file)` for code references in documentation
Applied to files:
.gitignore
📚 Learning: 2025-11-24T16:55:51.094Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .cursor/rules/cursor_rules.mdc:0-0
Timestamp: 2025-11-24T16:55:51.094Z
Learning: Applies to .cursor/rules/*.mdc : Use language-specific code blocks with ✅ DO examples for good patterns and ❌ DON'T examples for anti-patterns
Applied to files:
.gitignore
📚 Learning: 2025-11-24T16:55:51.094Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .cursor/rules/cursor_rules.mdc:0-0
Timestamp: 2025-11-24T16:55:51.094Z
Learning: Cursor rule content should start with high-level overview, include specific actionable requirements, show examples of correct implementation, reference existing code, and keep rules DRY by referencing other rules
Applied to files:
.gitignore
📚 Learning: 2025-11-24T16:55:51.094Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .cursor/rules/cursor_rules.mdc:0-0
Timestamp: 2025-11-24T16:55:51.094Z
Learning: Use bullet points for clarity in Cursor rules, keep descriptions concise, include both DO and DON'T examples, reference actual code over theoretical examples, and use consistent formatting across rules
Applied to files:
.gitignore
🧬 Code graph analysis (1)
app/models/balance_sheet/classification_group.rb (1)
app/models/balance_sheet/account_group.rb (1)
classification(51-53)
🔇 Additional comments (8)
.gitignore (2)
50-50: Good addition for VS Code workspace files.Adding
*.code-workspaceto the ignore rules is appropriate; VS Code workspace configuration is user-specific and should not be committed.
72-73: Inconsistency between summary and actual changes.The AI summary states that
compose.ymlandplaid_test_accounts/were removed in this PR, but both entries appear in the provided code without change markers, indicating they remain unchanged. Please clarify whether these entries were intended to be removed.config/locales/views/holdings/it.yml (1)
1-35: Hierarchical i18n structure is well-organized and follows conventions.The file correctly implements Italian translations with proper YAML structure and hierarchical keys (
holdings.index.average_cost,holdings.show.delete_title, etc.). Interpolation keys are properly formatted (%{qty},%{security},%{price}).The past review comment regarding a typo ("Canella" vs. "Cancella") appears to have been resolved—line 25 correctly shows
delete: Cancellaand line 26 appropriately usescancellerà(future tense: "will delete").config/locales/views/tags/it.yml (1)
1-23: Verify capitalization consistency with English locale and other Italian locale files.The Italian locale file maintains consistent capitalization of "Tag" throughout all instances. To ensure this aligns with the English equivalent (
config/locales/views/tags/en.yml) and maintains consistency with other Italian locale files, please verify the capitalization choice against those references. The file structure, hierarchical key organization, and interpolation syntax are correct per the coding guidelines.config/locales/views/email_confirmation_mailer/it.yml (1)
1-10: ✅ Namespace issue resolved—Italian locale now correctly configured.The critical issue from the previous review has been fixed. Line 2 now correctly uses the
it:namespace, ensuring these translations are loaded under the Italian locale rather than English. The hierarchical key structure (email_confirmation_mailer.confirmation_email.*) follows best practices, and the interpolation variables (%{hours},%{product_name}) are correctly formatted.Verify that the corresponding English locale file (
config/locales/views/email_confirmation_mailer/en.yml) exists with matching keys to maintain i18n structure consistency across locales, as noted in the PR objectives:#!/bin/bash # Verify English locale file exists with matching keys if [ -f "config/locales/views/email_confirmation_mailer/en.yml" ]; then echo "✓ EN locale file exists" echo "Comparing keys between EN and IT..." # Extract keys from both files en_keys=$(rg -o '^\s+[a-z_]+:' "config/locales/views/email_confirmation_mailer/en.yml" | sed 's/^\s*//;s/:$//' | sort) it_keys=$(rg -o '^\s+[a-z_]+:' "config/locales/views/email_confirmation_mailer/it.yml" | sed 's/^\s*//;s/:$//' | sort) if [ "$en_keys" = "$it_keys" ]; then echo "✓ Keys match between EN and IT locales" else echo "⚠ Key mismatch detected between EN and IT locales" echo "EN keys: $en_keys" echo "IT keys: $it_keys" fi else echo "⚠ EN locale file not found at config/locales/views/email_confirmation_mailer/en.yml" ficonfig/locales/views/imports/en.yml (1)
37-51: Ensure Italian locale and parameter passing are consistent with placeholder usage.The English locale correctly uses
%{product_name}as a dynamic placeholder for branding across four description keys (lines 39, 43, 46, 49), aligning with the hierarchical i18n structure required by the coding guidelines. However, two items require verification:
- The corresponding Italian locale file (
config/locales/views/imports/it.yml) must mirror this structure with%{product_name}placeholders preserved in all four translations.- The view/controller code invoking these translation keys must pass the
product_nameparameter to thet()helper (e.g.,t('import.shows.account_mapping_description', product_name: @product_name)).app/models/balance_sheet/classification_group.rb (2)
16-18: Ensure i18n keys for classifications are defined in locale files.The i18n implementation includes a proper fallback, but verify that the keys
pages.dashboard.balance_sheet.assetandpages.dashboard.balance_sheet.liabilityexist inconfig/locales/locale files to avoid relying on the default titleize fallback.
36-45: Verify locale keys exist for all account types.The i18n implementation correctly uses
count: 2to force plural form for account type names, which is appropriate for group labels. The fallback toaccountable.display_nameis sensible. Ensure that all accountable types have correspondingaccounts.types.{type}keys defined in the locale configuration files.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
app/controllers/reports_controller.rb(1 hunks)app/models/period.rb(2 hunks)config/locales/models/en.yml(1 hunks)config/locales/models/it.yml(1 hunks)config/locales/views/pages/en.yml(1 hunks)config/locales/views/pages/it.yml(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- config/locales/models/it.yml
- config/locales/views/pages/en.yml
- config/locales/views/pages/it.yml
- app/controllers/reports_controller.rb
🧰 Additional context used
📓 Path-based instructions (12)
config/locales/**/*.yml
📄 CodeRabbit inference engine (CLAUDE.md)
Use hierarchical i18n keys by feature:
accounts.index.title,transactions.form.amount_label. Uset()helper for all user-facing strings with interpolation for dynamic content.
Files:
config/locales/models/en.yml
config/**/*.{rb,yml,yaml}
📄 CodeRabbit inference engine (AGENTS.md)
Configuration files should be placed in
config/directory, with environment examples in.env.local.exampleand.env.test.example
Files:
config/locales/models/en.yml
**/*.rb
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.rb: Application supports two modes: 'managed' and 'self_hosted' via Rails.application.config.app_mode
Use Current.user and Current.family instead of current_user / current_family for authentication context
Optimize database queries with proper indexes to prevent N+1 queries using includes/joins
**/*.rb: Use Rails pluralization in i18n:t("transactions.count", count: @transactions.count).
Runbin/rubocop -f github -afor Ruby linting with auto-correct before pull requests.
**/*.rb: Ruby code should use 2-space indent,snake_casefor methods and variables, andCamelCasefor classes and modules, following Rails conventions for folders and file names
Prefer environment variables over hard-coded values for configuration
Files:
app/models/period.rb
app/models/**/*.rb
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
app/models/**/*.rb: Keep business logic in models using skinny controllers and fat models pattern
Store all monetary values in base currency (user's primary currency) and use Money objects for conversion
Use ActiveRecord validations for form convenience and complex business logic validations
Use Rails concerns and POROs for organizing business logic instead of services folder
app/models/**/*.rb: All monetary values stored in base currency (user's primary currency). UseMoneyobjects for currency conversion and formatting.
Place business logic inapp/models/folder. Avoid creatingapp/services/folder. Use Rails concerns and POROs for organization.
Models should answer questions about themselves using instance methods: useaccount.balance_seriesnotAccountSeries.new(account).call.
Files:
app/models/period.rb
**/*.{rb,erb,html.erb}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{rb,erb,html.erb}: UseCurrent.userfor the current user context in controllers and views. Do NOT usecurrent_user.
UseCurrent.familyfor the current family context. Do NOT usecurrent_family.
Files:
app/models/period.rb
**/*.{erb,html.erb,rb}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{erb,html.erb,rb}: All user-facing strings must use localization (i18n) via thet()helper. Update locale files for each new or changed element.
Use server-side formatting for currencies, numbers, and dates. Do not format in JavaScript.
Files:
app/models/period.rb
{app/models/**/*.rb,db/migrate/**/*.rb}
📄 CodeRabbit inference engine (CLAUDE.md)
Simple validations (null checks, unique indexes) should be in the database. ActiveRecord validations for form convenience (prefer client-side when possible). Complex validations and business logic in ActiveRecord.
Files:
app/models/period.rb
**/*.{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/models/period.rb
**/*.{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/models/period.rb
**/*.{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/models/period.rb
app/**/*.{rb,js,ts,jsx,tsx,css,scss}
📄 CodeRabbit inference engine (AGENTS.md)
Code should be organized in
app/directory (Rails MVC, services, jobs, mailers, components), with JS inapp/javascript/, and styles/assets inapp/assets/(Tailwind, images, fonts)
Files:
app/models/period.rb
app/**/*.rb
📄 CodeRabbit inference engine (AGENTS.md)
Run
bin/brakemansecurity scan before major PRs to check for static analysis of common Rails issues
Files:
app/models/period.rb
🧠 Learnings (2)
📓 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: 2025-11-24T16:55:43.069Z
Learning: Applies to **/*.{erb,html.erb,rb} : All user-facing strings must use localization (i18n) via the `t()` helper. Update locale files for each new or changed element.
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.
📚 Learning: 2025-11-24T16:55:43.069Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:55:43.069Z
Learning: Applies to **/*.{erb,html.erb,rb} : All user-facing strings must use localization (i18n) via the `t()` helper. Update locale files for each new or changed element.
Applied to files:
config/locales/models/en.ymlapp/models/period.rb
🔇 Additional comments (4)
config/locales/models/en.yml (1)
1-51: LGTM! Well-structured locale file for period translations.The locale file follows proper hierarchical i18n structure and all 12 period keys from the model have corresponding translations. The English labels are clear, consistent, and follow standard conventions (short labels like "1D", "WTD", "MTD", etc.). This aligns with the coding guideline requiring all user-facing strings to use i18n.
app/models/period.rb (3)
14-56: Excellent refactoring to separate concerns.The PERIODS hash now contains only business logic (date ranges), while presentation labels have been moved to locale files. This follows Rails best practices and the coding guideline requiring all user-facing strings to use i18n. The date calculations for each period are correct, including the newly added "last_month" period.
125-136: I18n implementation is correct with appropriate defaults.The label methods properly use I18n lookups with sensible defaults for custom periods. The
comparison_labelcorrectly falls back to a formatted date range when no translation is found. This implementation follows Rails i18n conventions and satisfies the coding guideline requiring i18n for all user-facing strings.Note: Once the minor issue in
i18n_scopeis addressed, these methods will produce cleaner i18n keys.
12-57: Verify period key consistency across locale files.Ensure that all period keys defined in the PERIODS hash have corresponding entries in both the English and Italian locale files (config/locales/models/en.yml and config/locales/models/it.yml).
#!/bin/bash # Description: Verify all PERIODS keys have corresponding locale entries in both en.yml and it.yml echo "=== Period keys defined in Period model ===" rg -A1 'PERIODS = \{' app/models/period.rb | rg '^\s*"(\w+)"\s*=>' -o -r '$1' | sort echo "" echo "=== Period keys in English locale file ===" rg '^\s+(\w+):$' config/locales/models/en.yml -r '$1' | grep -v '^periods$' | sort echo "" echo "=== Period keys in Italian locale file ===" if [ -f config/locales/models/it.yml ]; then rg '^\s+(\w+):$' config/locales/models/it.yml -r '$1' | grep -v '^periods$' | grep -v '^it$' | sort else echo "Italian locale file not found" fi echo "" echo "=== Checking for complete structure in locale files ===" echo "Each period should have: label_short, label, comparison_label" for locale_file in config/locales/models/en.yml config/locales/models/it.yml; do if [ -f "$locale_file" ]; then echo "Checking $locale_file..." rg -A3 '^\s+\w+:$' "$locale_file" | rg 'label_short:|label:|comparison_label:' || echo " Missing required keys" fi done
| def i18n_scope | ||
| "periods.#{key}" | ||
| end |
There was a problem hiding this comment.
Guard against nil key for cleaner i18n lookups.
When key is nil (for custom periods), this method returns "periods.", which creates malformed i18n keys like "periods..label" (double dot). While the defaults in the label methods prevent this from breaking functionality, it's not clean and could cause confusion or warnings.
Consider adding a guard:
def i18n_scope
- "periods.#{key}"
+ key ? "periods.#{key}" : nil
endThen update the label methods to skip i18n lookup when i18n_scope is nil:
def label
- I18n.t("#{i18n_scope}.label", default: "Custom Period")
+ return "Custom Period" unless i18n_scope
+ I18n.t("#{i18n_scope}.label", default: "Custom Period")
end(Apply similar changes to label_short and comparison_label.)
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In app/models/period.rb around lines 120–122, i18n_scope currently builds
"periods.#{key}" which produces "periods." when key is nil; change i18n_scope to
return nil (or false) when key is nil/blank instead of "periods.", and update
label, label_short, and comparison_label to skip the I18n lookup when i18n_scope
is nil (use the fallback/default string directly) so no malformed keys like
"periods..label" are generated.
|
@jjmata and @alessiocappa I updated the translation and fixed conflicts and errors reported by CodeRabbit. |
Signed-off-by: Juan José Mata <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/models/period.rb (1)
12-39: Remove duplicatedlast_monthhash key and leftover label metadata; align PERIODS with I18n-based labelsThe
PERIODShash defines"last_month"twice (Lines 25-27 and 28-33). In Ruby, the last definition wins, so the first one is effectively dead code, which is confusing and easy to mis-edit later.Additionally,
"last_month"and"last_30_days"still carrylabel_short,label, andcomparison_labelfields, but the public label methods now always useI18n.tand ignore these fields. That makes this metadata misleading and out of sync with the new i18n-based design.I recommend:
- Keeping a single
"last_month"entry.- Dropping the per-key label metadata from
"last_month"and"last_30_days"so PERIODS only encodes date ranges and all text comes from locales.For example:
- "current_month" => { - date_range: -> { [ Date.current.beginning_of_month, Date.current ] } - }, - "last_month" => { - date_range: -> { [ 1.month.ago.beginning_of_month.to_date, 1.month.ago.end_of_month.to_date ] } - }, - "last_month" => { - date_range: -> { [ 1.month.ago.beginning_of_month.to_date, 1.month.ago.end_of_month.to_date ] }, - label_short: "LM", - label: "Last Month", - comparison_label: "vs. last month" - }, - "last_30_days" => { - date_range: -> { [ 30.days.ago.to_date, Date.current ] }, - label_short: "30D", - label: "Last 30 Days", - comparison_label: "vs. last 30 days" - }, + "current_month" => { + date_range: -> { [ Date.current.beginning_of_month, Date.current ] } + }, + "last_month" => { + date_range: -> { [ 1.month.ago.beginning_of_month.to_date, 1.month.ago.end_of_month.to_date ] } + }, + "last_30_days" => { + date_range: -> { [ 30.days.ago.to_date, Date.current ] } + },
♻️ Duplicate comments (1)
app/models/period.rb (1)
129-145: Guardi18n_scopefor custom periods to avoid malformed translation keysFor custom periods,
keyis nil, soi18n_scopebecomes"periods."and the label lookups resolve against"periods..label","periods..label_short", etc. While thedefault:options mask this at runtime, it produces malformed I18n keys and can generate confusing missing-translation noise.It would be cleaner to return
nilfromi18n_scopewhenkeyis nil and skip the I18n lookup in that case:- def i18n_scope - "periods.#{key}" - end - - def label - I18n.t("#{i18n_scope}.label", default: "Custom Period") - end - - def label_short - I18n.t("#{i18n_scope}.label_short", default: "Custom") - end - - def comparison_label - I18n.t( - "#{i18n_scope}.comparison_label", - default: "#{start_date.strftime(@date_format)} to #{end_date.strftime(@date_format)}" - ) - end + def i18n_scope + key ? "periods.#{key}" : nil + end + + def label + return "Custom Period" unless i18n_scope + I18n.t("#{i18n_scope}.label", default: "Custom Period") + end + + def label_short + return "Custom" unless i18n_scope + I18n.t("#{i18n_scope}.label_short", default: "Custom") + end + + def comparison_label + return "#{start_date.strftime(@date_format)} to #{end_date.strftime(@date_format)}" unless i18n_scope + I18n.t( + "#{i18n_scope}.comparison_label", + default: "#{start_date.strftime(@date_format)} to #{end_date.strftime(@date_format)}" + ) + endThis keeps custom periods using the explicit Ruby defaults and avoids malformed i18n keys.
🧹 Nitpick comments (1)
app/models/period.rb (1)
121-127: Localizeintervaland validation error messages
interval(Lines 121-127) returns"1 week"/"1 day", andmust_be_valid_date_range(Lines 153-162) builds English error messages inline. Given the repo’s guideline that all user-facing strings in*.rbmust use i18n, these should eventually move into locale files and useI18n.tinstead of hard-coded text.Suggested direction (sketch):
- def interval - if days > 366 - "1 week" - else - "1 day" - end - end + def interval + if days > 366 + I18n.t("periods.interval.week", default: "1 week") + else + I18n.t("periods.interval.day", default: "1 day") + end + endAnd similarly for validation messages:
- unless start_date.is_a?(Date) && end_date.is_a?(Date) - errors.add(:start_date, "must be a valid date, got #{start_date.inspect}") - errors.add(:end_date, "must be a valid date, got #{end_date.inspect}") + unless start_date.is_a?(Date) && end_date.is_a?(Date) + errors.add(:start_date, I18n.t("periods.errors.invalid_start_date", default: "must be a valid date")) + errors.add(:end_date, I18n.t("periods.errors.invalid_end_date", default: "must be a valid date")) end - - errors.add(:start_date, "must be before end date") if start_date > end_date + errors.add(:start_date, I18n.t("periods.errors.start_after_end", default: "must be before end date")) if start_date > end_dateThis can be a follow-up cleanup, but it will make this model consistent with the i18n work done in this PR.
Also applies to: 153-162
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
app/controllers/family_merchants_controller.rb(1 hunks)app/controllers/reports_controller.rb(1 hunks)app/controllers/settings/providers_controller.rb(1 hunks)app/models/period.rb(3 hunks)app/views/pages/dashboard/_balance_sheet.html.erb(1 hunks)config/locales/views/imports/en.yml(1 hunks)config/locales/views/pages/en.yml(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (6)
- app/controllers/settings/providers_controller.rb
- config/locales/views/imports/en.yml
- app/controllers/reports_controller.rb
- config/locales/views/pages/en.yml
- app/views/pages/dashboard/_balance_sheet.html.erb
- app/controllers/family_merchants_controller.rb
🧰 Additional context used
📓 Path-based instructions (10)
**/*.rb
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.rb: Application supports two modes: 'managed' and 'self_hosted' via Rails.application.config.app_mode
Use Current.user and Current.family instead of current_user / current_family for authentication context
Optimize database queries with proper indexes to prevent N+1 queries using includes/joins
**/*.rb: Use Rails pluralization in i18n:t("transactions.count", count: @transactions.count).
Runbin/rubocop -f github -afor Ruby linting with auto-correct before pull requests.
**/*.rb: Ruby code should use 2-space indent,snake_casefor methods and variables, andCamelCasefor classes and modules, following Rails conventions for folders and file names
Prefer environment variables over hard-coded values for configuration
Files:
app/models/period.rb
app/models/**/*.rb
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
app/models/**/*.rb: Keep business logic in models using skinny controllers and fat models pattern
Store all monetary values in base currency (user's primary currency) and use Money objects for conversion
Use ActiveRecord validations for form convenience and complex business logic validations
Use Rails concerns and POROs for organizing business logic instead of services folder
app/models/**/*.rb: All monetary values stored in base currency (user's primary currency). UseMoneyobjects for currency conversion and formatting.
Place business logic inapp/models/folder. Avoid creatingapp/services/folder. Use Rails concerns and POROs for organization.
Models should answer questions about themselves using instance methods: useaccount.balance_seriesnotAccountSeries.new(account).call.
Files:
app/models/period.rb
**/*.{rb,erb,html.erb}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{rb,erb,html.erb}: UseCurrent.userfor the current user context in controllers and views. Do NOT usecurrent_user.
UseCurrent.familyfor the current family context. Do NOT usecurrent_family.
Files:
app/models/period.rb
**/*.{erb,html.erb,rb}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{erb,html.erb,rb}: All user-facing strings must use localization (i18n) via thet()helper. Update locale files for each new or changed element.
Use server-side formatting for currencies, numbers, and dates. Do not format in JavaScript.
Files:
app/models/period.rb
{app/models/**/*.rb,db/migrate/**/*.rb}
📄 CodeRabbit inference engine (CLAUDE.md)
Simple validations (null checks, unique indexes) should be in the database. ActiveRecord validations for form convenience (prefer client-side when possible). Complex validations and business logic in ActiveRecord.
Files:
app/models/period.rb
**/*.{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/models/period.rb
**/*.{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/models/period.rb
**/*.{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/models/period.rb
app/**/*.{rb,js,ts,jsx,tsx,css,scss}
📄 CodeRabbit inference engine (AGENTS.md)
Code should be organized in
app/directory (Rails MVC, services, jobs, mailers, components), with JS inapp/javascript/, and styles/assets inapp/assets/(Tailwind, images, fonts)
Files:
app/models/period.rb
app/**/*.rb
📄 CodeRabbit inference engine (AGENTS.md)
Run
bin/brakemansecurity scan before major PRs to check for static analysis of common Rails issues
Files:
app/models/period.rb
🧠 Learnings (3)
📓 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: 2025-11-24T16:55:43.069Z
Learning: Applies to **/*.{erb,html.erb,rb} : All user-facing strings must use localization (i18n) via the `t()` helper. Update locale files for each new or changed element.
Learnt from: CR
Repo: we-promise/sure PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:55:43.069Z
Learning: Applies to config/locales/**/*.yml : Use hierarchical i18n keys by feature: `accounts.index.title`, `transactions.form.amount_label`. Use `t()` helper for all user-facing strings with interpolation for dynamic content.
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.
📚 Learning: 2025-11-24T16:55:43.069Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:55:43.069Z
Learning: Applies to **/*.{erb,html.erb,rb} : All user-facing strings must use localization (i18n) via the `t()` helper. Update locale files for each new or changed element.
Applied to files:
app/models/period.rb
📚 Learning: 2025-11-24T16:55:43.069Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:55:43.069Z
Learning: Applies to config/locales/**/*.yml : Use hierarchical i18n keys by feature: `accounts.index.title`, `transactions.form.amount_label`. Use `t()` helper for all user-facing strings with interpolation for dynamic content.
Applied to files:
app/models/period.rb
🧬 Code graph analysis (1)
app/models/period.rb (4)
app/controllers/accounts_controller.rb (1)
family(189-191)app/models/family.rb (1)
oldest_entry_date(98-100)app/models/account.rb (1)
start_date(222-225)app/models/market_data_importer.rb (1)
end_date(126-128)
⏰ 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). (2)
- GitHub Check: ci / test
- GitHub Check: Cursor Bugbot
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
This PR is being reviewed by Cursor Bugbot
Details
You are on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle.
To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.
| date_range: -> { [ Date.current.beginning_of_month, Date.current ] } | ||
| }, | ||
| "last_month" => { | ||
| date_range: -> { [ 1.month.ago.beginning_of_month.to_date, 1.month.ago.end_of_month.to_date ] } |
There was a problem hiding this comment.
Bug: Duplicate "last_month" key in PERIODS hash
The PERIODS hash contains two entries with the key "last_month". Ruby hashes don't support duplicate keys, so the second entry will silently overwrite the first. This appears to be an incomplete refactoring where a new entry was added (lines 24-27) but the old entry (lines 28-33 with inline labels) wasn't removed. The second duplicate still contains hardcoded labels that should have been removed as part of the i18n migration.
| @@ -0,0 +1,8 @@ | |||
| --- | |||
| en: | |||
There was a problem hiding this comment.
Bug: Wrong locale key in Italian invitation mailer file
The Italian locale file config/locales/views/invitation_mailer/it.yml has en: as its root key instead of it:. This means the Italian translations for the invitation mailer will never be loaded when the locale is set to Italian - they'll be treated as English translations instead, causing users to see Italian text even when using English locale.
Signed-off-by: Yari <[email protected]>
3 participants
This PR introduces the initial Italian locale (it) and improves the overall i18n structure of the app.
What’s included
These new keys were added to the EN locale and mirrored in the IT file to keep the i18n structure consistent.
Notes / Known Issues
This appears to be related to the order in which breadcrumbs are initialized, as some keys resolve before the locale is fully loaded.
Next steps
Summary by CodeRabbit
New Features
Refactor
Chores
Style
✏️ Tip: You can customize this high-level summary in your review settings.