[chores] Improve i18n coverage for user-facing strings #519#539
[chores] Improve i18n coverage for user-facing strings #519#539CodingWithSaksham wants to merge 3 commits into
Conversation
Fixed missing/broken translation coverage across openwisp_users (admin labels, login template a model validation error that were silently untranslatable) and filled in the German ,Italian, Russian, and Slovenian translations for every newly extracted string. Closes openwisp#519
|
Warning Review limit reached
Next review available in: 50 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThis PR improves i18n coverage across openwisp-users. Hardcoded strings in Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
Poem Caution Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional.
❌ Failed checks (1 error, 1 warning)
✅ Passed checks (6 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
coverage: 98.128% (-0.003%) from 98.131% — CodingWithSaksham:issues/519-improve-i18n-coverage into openwisp:master |
![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 current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@openwisp_users/mixins.py`:
- Around line 63-75: The validation message in the organization-change check is
still built by splicing a translated verb fragment into a sentence, which should
be replaced with full plural-aware translations. Update the message construction
in the mixin method that raises ValidationError to use ngettext with complete
singular and plural sentence templates, and format only the named placeholders
such as count, object_label, and related_object_label. Remove the standalone
verb translation so the entire sentence is translated in context.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: e50f2788-5d0a-49eb-a22b-9991fed86065
📒 Files selected for processing (13)
openwisp_users/accounts/locale/de/LC_MESSAGES/django.poopenwisp_users/accounts/locale/it/LC_MESSAGES/django.poopenwisp_users/accounts/locale/ru/LC_MESSAGES/django.poopenwisp_users/accounts/locale/sl/LC_MESSAGES/django.poopenwisp_users/admin.pyopenwisp_users/api/permissions.pyopenwisp_users/base/models.pyopenwisp_users/locale/de/LC_MESSAGES/django.poopenwisp_users/locale/it/LC_MESSAGES/django.poopenwisp_users/locale/ru/LC_MESSAGES/django.poopenwisp_users/locale/sl/LC_MESSAGES/django.poopenwisp_users/mixins.pyopenwisp_users/templates/admin/openwisp_users/login.html
📜 Review details
⏰ Context from checks skipped due to timeout. (14)
- GitHub Check: Python==3.13 | django~=5.2.0
- GitHub Check: Python==3.12 | django~=4.2.0
- GitHub Check: Python==3.11 | django~=4.2.0
- GitHub Check: Python==3.10 | django~=5.2.0
- GitHub Check: Python==3.10 | django~=5.1.0
- GitHub Check: Python==3.13 | django~=5.1.0
- GitHub Check: Python==3.12 | django~=5.1.0
- GitHub Check: Python==3.11 | django~=5.0.0
- GitHub Check: Python==3.11 | django~=5.1.0
- GitHub Check: Python==3.12 | django~=5.2.0
- GitHub Check: Python==3.10 | django~=4.2.0
- GitHub Check: Python==3.11 | django~=5.2.0
- GitHub Check: Python==3.12 | django~=5.0.0
- GitHub Check: Python==3.10 | django~=5.0.0
🧰 Additional context used
📓 Path-based instructions (1)
**/*.py
📄 CodeRabbit inference engine (Custom checks)
For Django pull requests, ensure all user-facing strings are marked as translatable using the Django i18n framework (django.utils.translation.gettext, gettext_lazy, or ugettext aliases)
**/*.py: Mark user-facing strings for translation with Django i18n helpers in Django code
Place imports at the top of the file; only defer imports when necessary (e.g., Django model imports inside functions or methods where the app registry is not yet ready)
Avoid unnecessary blank lines inside function and method bodies
Write comments and docstrings only when they explain why code is shaped a certain way; place comments before the relevant code block instead of scattering them inside it
Files:
openwisp_users/api/permissions.pyopenwisp_users/mixins.pyopenwisp_users/base/models.pyopenwisp_users/admin.py
🪛 HTMLHint (1.9.2)
openwisp_users/templates/admin/openwisp_users/login.html
[warning] 47-47: No matching [ label ] tag found.
(input-requires-label)
🔇 Additional comments (18)
openwisp_users/locale/de/LC_MESSAGES/django.po (1)
140-148: 📐 Maintainability & Code Quality | ⚡ Quick winFix the plural agreement in this translation.
permission_classesreads as plural here, sowurdeis incorrect. Usewurdenor rephrase the sentence so the admin error reads naturally.Suggested fix
- "permission_classes der View wurde nicht korrekt implementiert. Bitte " + "permission_classes der View wurden nicht korrekt implementiert. Bitte "openwisp_users/locale/it/LC_MESSAGES/django.po (1)
142-150: 📐 Maintainability & Code Quality | ⚡ Quick winFix the verb agreement in this translation.
permission_classesis plural in context, sonon implementatashould benon implementate.Suggested fix
- "permission_classes della view non implementata correttamente. Utilizza una " + "permission_classes della view non implementate correttamente. Utilizza una "openwisp_users/locale/ru/LC_MESSAGES/django.po (1)
141-149: 📐 Maintainability & Code Quality | ⚡ Quick winRephrase this error message for grammar.
permission_classes представления реализовано некорректноis not natural Russian. Please rewrite the sentence so the technical term stays intact but the message reads correctly.Suggested fix
- "permission_classes представления реализовано некорректно. Используйте один " + "Свойство permission_classes у представления реализовано некорректно. Используйте один "openwisp_users/locale/sl/LC_MESSAGES/django.po (1)
134-142: 📐 Maintainability & Code Quality | ⚡ Quick winFix the broken grammar in this translation.
permission_classes poglede ni pravilno implementiranis not grammatical Slovenian. Please rewrite the full sentence before shipping.Suggested fix
- "permission_classes poglede ni pravilno implementiran. Uporabite enega od " + "permission_classes pogleda niso pravilno implementirane. Uporabite enega od "openwisp_users/accounts/locale/de/LC_MESSAGES/django.po (1)
9-18: LGTM!Also applies to: 45-56, 57-73, 74-213
openwisp_users/accounts/locale/it/LC_MESSAGES/django.po (1)
9-16: LGTM!Also applies to: 44-63, 72-80, 122-144, 154-207
openwisp_users/accounts/locale/ru/LC_MESSAGES/django.po (1)
8-19: LGTM!Also applies to: 30-37, 49-102, 112-220
openwisp_users/accounts/locale/sl/LC_MESSAGES/django.po (1)
9-19: LGTM!Also applies to: 51-76, 86-222
openwisp_users/templates/admin/openwisp_users/login.html (3)
44-48: HTMLHint "missing label" warning on the submit input is a pre-existing false positive, not introduced by this change.Submit inputs get their accessible name from the
valueattribute; no<label>association is required here. No action needed for this i18n change.Source: Linters/SAST tools
7-7: LGTM! This mirrors Django's own admin idiom for singular/plural error banners.
23-26: LGTM!blocktrans trimmedcorrectly wraps the full sentence with the{{ username }}variable inline.openwisp_users/admin.py (5)
183-197: LGTM!
266-282: LGTM! This mirrors Django's own admin pattern of usinggettextwithmodel_ngettextfor the pluralized noun while keeping a single sentence template.
399-422: LGTM!
489-509: LGTM!
642-675: LGTM!openwisp_users/base/models.py (1)
492-504: LGTM! Named-placeholder formatting is applied correctly and matches the test expectation for the owner-downgrade error.openwisp_users/api/permissions.py (1)
46-53: LGTM!
3 participants
Checklist
Reference to Existing Issue
Closes #519 .
Description of Changes
Fixed missing/broken translation coverage across
openwisp_users(admin labels, login template, several admin messages and a model validation error that were silently untranslatable) and filled in the German ,Italian, Russian, and Slovenian translations for every newly extracted string.NOTE
I used AI primarily to translate the text into different languages. Most of the translations were verified using Google Translate. However, some translations could not be verified because the strings contained variables and placeholders, and translating them directly with Google Translate produced incorrect or broken translations by altering those variables. In those cases, I relied on the AI generated translations.