Convert hardcoded strings to lazy translated strings #2661
Conversation
Fixes ohcnetwork#2482 --- For more details, open the [Copilot Workspace session](https://copilot-workspace.githubnext.com/ohcnetwork/care/issues/2482?shareId=XXXX-XXXX-XXXX-XXXX).
📝 Walkthrough📝 WalkthroughWalkthroughThe pull request focuses on enhancing internationalization across multiple Django models and serializers by replacing hardcoded strings with lazy translated strings. The changes span four key files: Changes
Assessment against linked issues
Poem
Tip CodeRabbit's docstrings feature is now available as part of our Early Access Program! Simply use the command Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![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
🔭 Outside diff range comments (1)
care/facility/api/serializers/patient_consultation.py (1)
Some untranslated strings found in error messages... how unfortunate.
Several error messages in the codebase are not wrapped with the translation function
_(). Here are the key locations that need attention:
care/facility/api/serializers/patient.py:
- "Facility is required to register a patient"
- "Note cannot be empty"
- "This field is required"
care/facility/api/serializers/inventory.py:
- "Item cannot be measured with unit"
- "Item cannot be Null"
- "Item cannot be Changed"
care/facility/api/serializers/prescription.py:
- "Administered Date cannot be in the future"
- "Administered Date cannot be before Prescription Date"
- "Base dosage is required"
care/facility/api/serializers/shifting.py:
- "Shifting Approving Facility is required"
- "Comment cannot be empty"
care/facility/api/serializers/facility_capacity.py:
- "Current capacity cannot be greater than total capacity"
And several others throughout the codebase. These strings should be wrapped with
_()to maintain consistency with the internationalization efforts that are clearly important for this project.🔗 Analysis chain
Line range hint
226-909: Verify complete translation coverage.While the error messages have been translated, let's verify that no untranslated strings remain in the codebase.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for potential untranslated strings in error messages rg -g '*.py' '"[A-Z].*(?<!_)\b(required|invalid|error|must|cannot|should|only|field|value|greater|less|before|after|future|past|specified)\b'Length of output: 482
Script:
#!/bin/bash # Search for potential untranslated strings in error messages with proper regex rg --pcre2 -g '*.py' '"[A-Z][^"]*(?<!_)(required|invalid|error|must|cannot|should|only|field|value|greater|less|before|after|future|past|specified)[^"]*"' # Also check for strings that look like error messages but aren't wrapped in _() rg --pcre2 -g '*.py' '(?<!_\()"[A-Z][^"]*(?:error|invalid|required|must|cannot)[^"]*"'Length of output: 43774
🧹 Nitpick comments (3)
care/facility/models/encounter_symptom.py (1)
22-53: Consider reordering the symptoms list for better maintainabilityThe OTHERS choice (9) is placed at the beginning of the list, which seems a bit counterintuitive. Perhaps we could move it to the end? 🤔
class Symptom(models.IntegerChoices): - OTHERS = 9, gettext_lazy("Others") FEVER = 2, gettext_lazy("Fever") SORE_THROAT = 3, gettext_lazy("Sore Throat") # ... other symptoms ... + OTHERS = 9, gettext_lazy("Others")care/facility/api/serializers/patient_consultation.py (2)
163-163: Consider enhancing help text with more context.While the translations are implemented correctly, the help text could be more descriptive. Perhaps include examples or specify the expected format for diagnoses and symptoms.
- help_text=_("Bulk create diagnoses for the consultation upon creation"), + help_text=_("Bulk create diagnoses for the consultation upon creation. Example: {'diagnosis': 'id', 'is_principal': true}"), - help_text=_("Bulk create symptoms for the consultation upon creation"), + help_text=_("Bulk create symptoms for the consultation upon creation. Example: {'symptom': 'id', 'onset_date': 'YYYY-MM-DD'}"),Also applies to: 170-170
571-571: Maintain consistency in quote usage.There's a subtle inconsistency in the use of quotes. The code uses single quotes for the date format string while the codebase generally prefers double quotes.
- _("This field value must be greater than {date}").format(date=MIN_ENCOUNTER_DATE.strftime('%Y-%m-%d')) + _("This field value must be greater than {date}").format(date=MIN_ENCOUNTER_DATE.strftime("%Y-%m-%d"))🧰 Tools
🪛 Ruff (0.8.2)
571-571: Single quotes found but double quotes preferred
Replace single quotes with double quotes
(Q000)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
care/facility/api/serializers/patient_consultation.py(26 hunks)care/facility/models/encounter_symptom.py(2 hunks)care/facility/models/patient_consultation.py(8 hunks)care/users/models.py(7 hunks)
🧰 Additional context used
🪛 Ruff (0.8.2)
care/users/models.py
314-314: Undefined name default
(F821)
314-314: Boolean positional value in function call
(FBT003)
319-319: Undefined name default
(F821)
care/facility/api/serializers/patient_consultation.py
571-571: Single quotes found but double quotes preferred
Replace single quotes with double quotes
(Q000)
🔇 Additional comments (4)
care/facility/models/encounter_symptom.py (1)
22-53: LGTM: Internationalization changes look good
The conversion of symptom labels to use gettext_lazy is consistent and follows Django's internationalization best practices.
care/users/models.py (1)
33-53: LGTM: Comprehensive internationalization implementation
All user-facing strings have been properly wrapped with translation functions. The changes cover:
- Gender choices
- District choices
- Local body choices
- Type choices
- CSV mapping
- Model verbose names
Also applies to: 71-80, 107-108, 250-250, 338-348, 466-466
care/facility/models/patient_consultation.py (1)
31-35: LGTM: Excellent internationalization coverage
All user-facing strings have been properly internationalized, including:
- Consent types
- Suggestion choices
- Help texts
- Verbose names
- CSV mappings
- Code status types
- Error messages
Also applies to: 40-45, 60-63, 203-203, 209-209, 231-238, 327-331, 370-370, 405-405
care/facility/api/serializers/patient_consultation.py (1)
7-7: LGTM! Following Django's i18n best practices.
The import of gettext_lazy with the conventional _ alias aligns perfectly with Django's internationalization guidelines.
care/users/models.py
Outdated
| @@ -311,12 +311,12 @@ class User(AbstractUser): | |||
| ) | |||
|
|
|||
| verified = models.BooleanField(default=False) | |||
| deleted = models.BooleanField(default=False) | |||
| deleted = models.BooleanField(default(False)) | |||
There was a problem hiding this comment.
Fix incorrect usage of default() function
There are two instances where default() is used incorrectly. This function doesn't exist in Django's model fields.
- deleted = models.BooleanField(default(False))
+ deleted = models.BooleanField(default=False)
- pf_auth = models.TextField(default(None), null=True)
+ pf_auth = models.TextField(default=None, null=True)Also applies to: 319-319
🧰 Tools
🪛 Ruff (0.8.2)
314-314: Undefined name default
(F821)
314-314: Boolean positional value in function call
(FBT003)
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
care/users/models.py (1)
37-50: Consider if district names really need translation.While you've correctly applied the translation wrapper, district names are proper nouns that typically don't get translated. Unless there are official transliterations or translations of these district names in other languages, this might be unnecessary.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
care/users/models.py(6 hunks)
🔇 Additional comments (5)
care/users/models.py (5)
33-33: LGTM! The gender choices are now properly internationalized.
The translation wrapper is correctly applied while maintaining the tuple structure.
71-80: Nice work on the local body translations!
The translation wrapper is properly applied to administrative division types that definitely need localization.
107-108: Well-handled model verbose names.
The translation wrapper is correctly applied to model verbose names, which is essential for admin interface localization.
Also applies to: 466-466
250-250: Good job on type choices translation.
The translation wrapper is correctly applied using a clean dictionary comprehension approach.
338-348: Verify CSV parsing compatibility with translated headers.
While translating the CSV headers is good for UI display, please ensure that the CSV parsing logic doesn't rely on these translated strings as keys.
Fixes #2482
Summary by CodeRabbit
New Features
Bug Fixes
Chores