TrestleAdmin
fix ModelName default pluralization
Coverage decreased (-4.1%) to 80.746% when pulling 1aeaf0cfecc7a13f4842c17b847f0abfc712f734 on svoboda-jan:patch-3 into d61193f2b742bd98844958cc0204196fc06a2ffd on TrestleAdmin:master.
Coverage decreased (-4.1%) to 80.746% when pulling 1aeaf0cfecc7a13f4842c17b847f0abfc712f734 on svoboda-jan:patch-3 into d61193f2b742bd98844958cc0204196fc06a2ffd on TrestleAdmin:master.
What issue are you seeing here?
The form of pluralize that is being used here forces the inflector to use the current locale's pluralization rules (otherwise it defaults to English -- see https://api.rubyonrails.org/classes/String.html#method-i-pluralize).
These rules will usually come from the `rails-i18n' gem.
Yes, the reason was to use english locale. At the time default_plural is called, we already know, that translation is'n present and we are pluralizing the class/model name (e.g. BlogPost), this change makes the assumtion, that model names (in code) are more likely in english rather than in the current locale.
To be honest, I'm not 100% convinced which route is the best way to go here.
At the time default_plural is called, we already know, that translation is'n present and we are pluralizing the class/model name (e.g. BlogPost)
Just for clarification, we know the plural translation isn't provided. However a singular translation could have been provided, so we are not necessarily just pluralizing the internal model name.
By way of example, consider a model named Dog and the German locale (for which the translation is Hund [singular] and Hunde [plural]).
If the developer provides both singular and plural options, everything is straight-forward and works as expected:
de:
activerecord:
models:
dog:
one: Hund
other: Hunde
The difference comes when only the singular form is provided:
de:
activerecord:
models:
dog: Hund
In the current version, the ModelName#plural method returns "Hund". In the PR version, this would return "Hunds". Neither is correct although using the English plural form "Hunds" here is arguably less correct.
I think I'd be open to a configuration option default_pluralization_locale defaulting to -> { I18n.locale }. That way it could be overridden on a per-app basis if required. What do you think?