Fixing consistency issues with pluralization in UI#1304
Conversation
📝 WalkthroughWalkthroughApplied Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~5 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d11cb8445b
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| <% end %> | ||
| </div> | ||
| <%= tag.p account.short_subtype_label, class: "text-sm text-secondary truncate" %> | ||
| <%= tag.p account.short_subtype_label.singularize, class: "text-sm text-secondary truncate" %> |
There was a problem hiding this comment.
Applying singularize to every subtype label regresses valid labels that are already correct in singular account context, e.g. the depository subtype Savings now renders as Saving. This affects common savings accounts in the account lists (and the same pattern was added in _account.html.erb), so users will see incorrect subtype text instead of the configured label.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/views/accounts/_account.html.erb`:
- Around line 32-34: The view calls account.long_subtype_label.singularize which
mutates an I18n-translated string using English-only rules; stop calling
String#singularize on translated labels (account.long_subtype_label) and instead
expose a model method that uses I18n pluralization (e.g. implement
subtype_label_for or subtype_label in accountable.rb that calls I18n.t with a
count argument and plural keys for the subtype), update locale files to provide
one/other entries for each subtype, and change the view to render the model
method with count: 1 (rather than calling .singularize) so translations are
correctly pluralized per locale.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 7488b8d1-c057-4c5e-b9e3-1082b8307f93
📒 Files selected for processing (3)
app/views/accounts/_account.html.erbapp/views/accounts/_accountable_group.html.erbconfig/locales/views/accounts/en.yml
| <% if account.long_subtype_label %> | ||
| <p class="text-sm text-secondary truncate"><%= account.long_subtype_label %></p> | ||
| <p class="text-sm text-secondary truncate"><%= account.long_subtype_label.singularize %></p> | ||
| <% end %> |
There was a problem hiding this comment.
String#singularize will produce incorrect results for non-English locales.
Ruby's singularize method uses English Inflector rules. Since account.long_subtype_label returns an I18n-translated string (via I18n.t in accountable.rb), calling .singularize on non-English translations like "Cuenta de Inversión" (Spanish) or "Investitionskonto" (German) will produce garbled text.
A more robust approach would be to store both singular and plural forms in the locale files and select the appropriate one at the model level:
# In accountable.rb
def subtype_label_for(subtype, format: :short, count: 1)
I18n.t(
"#{name.underscore.pluralize}.subtypes.#{subtype}.#{format}",
count: count,
default: fallback
)
endThen update locale files to use Rails I18n pluralization:
vehicles:
subtypes:
suv:
short:
one: "SUV"
other: "SUVs"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/views/accounts/_account.html.erb` around lines 32 - 34, The view calls
account.long_subtype_label.singularize which mutates an I18n-translated string
using English-only rules; stop calling String#singularize on translated labels
(account.long_subtype_label) and instead expose a model method that uses I18n
pluralization (e.g. implement subtype_label_for or subtype_label in
accountable.rb that calls I18n.t with a count argument and plural keys for the
subtype), update locale files to provide one/other entries for each subtype, and
change the view to render the model method with count: 1 (rather than calling
.singularize) so translations are correctly pluralized per locale.
|
Comments from the LLMs noted. Making further edits... |
|
Is this something your PR addresses also @UberDudePL? |
|
Yes, in my PL Localization, I was working on pluralization but one step further. As in Polish we are using, one, few, many and other to fully cover all aspects of proper grammar. But I was nit touching en.yml files to separate one and other as per example with SUV/SUVs. I can work on this if you want |
|
If @CYBRXT doesn't have time it would be great to get some help here, yes! Thank yiou @UberDudePL. |
4 participants
Closes #1303.
From the testing I've done on my dev environment, the issue can be fixed by pluralizing the category type names in config/locales/views/accounts/en.yml, and by singularizing any displayed subtype labels for the sidebar and Settings/General/Accounts by adding .singularize to the app/views/accounts/_account.html.erb and app/views/accounts/_accountable_group.html.erb files.
The format of all the asset/liability category types should now all be plural category with singular subtype label, as shown in this screenshot from the accounts sidebar:
And this one from Settings/General/Accounts:
Summary by CodeRabbit
Release Notes