String resources cleanup #3181
String resources cleanup #3181
Conversation
📝 WalkthroughWalkthroughThis set of changes focuses on renaming, consolidating, and removing string resources across the application—primarily those related to PersonalId (formerly ConnectId) authentication features, camera permissions, and error messages. Many string resources with generic or legacy keys have been deleted and replaced with new keys prefixed by "personalid_". Corresponding updates were made in layout XML files and Java classes to reference the new string resource keys. Additionally, large numbers of localized string entries in several languages were removed or similarly renamed to match the new conventions. No functional logic or control flow was changed in the Java code; all updates are limited to text resource management and UI string references. Sequence Diagram(s)No sequence diagrams are generated for these changes, as all modifications are limited to string resource references and do not affect the application's control flow or introduce new interactive features. ✨ Finishing Touches
🧪 Generate Unit Tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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: 3
🧹 Nitpick comments (2)
app/src/org/commcare/activities/LoginActivity.java (1)
531-533: Use new string resource for failed login message
Switched the error message key topersonalid_failed_to_login_with_connectid. After merging, remove the oldfailed_to_login_with_connectidentry from your string resources to avoid dead code.app/res/layout/screen_personalid_name.xml (1)
37-37: Prefix subtitle with PersonalId namespace
Changed the subtitle TextView to use@string/personalid_name_fragment_subtitle. Don't forget to delete the oldname_fragment_subtitleresource if it's no longer used.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (20)
app/res/layout/fragment_recovery_code.xml(2 hunks)app/res/layout/screen_login.xml(2 hunks)app/res/layout/screen_personalid_name.xml(2 hunks)app/res/layout/screen_personalid_photo_capture.xml(3 hunks)app/res/values-es/strings.xml(0 hunks)app/res/values-fr/strings.xml(1 hunks)app/res/values-lt/strings.xml(0 hunks)app/res/values-no/strings.xml(0 hunks)app/res/values-pt/strings.xml(1 hunks)app/res/values-sw/strings.xml(1 hunks)app/res/values-ti/strings.xml(1 hunks)app/res/values/strings.xml(1 hunks)app/src/org/commcare/activities/LoginActivity.java(1 hunks)app/src/org/commcare/fragments/MicroImageActivity.java(1 hunks)app/src/org/commcare/fragments/personalId/PersonalIdBackupCodeFragment.java(2 hunks)app/src/org/commcare/fragments/personalId/PersonalIdBiometricConfigFragment.java(2 hunks)app/src/org/commcare/fragments/personalId/PersonalIdPhoneFragment.java(2 hunks)app/src/org/commcare/fragments/personalId/PersonalIdPhoneVerificationFragment.java(1 hunks)app/src/org/commcare/fragments/personalId/PersonalIdPhotoCaptureFragment.java(1 hunks)app/src/org/commcare/utils/BiometricsHelper.java(1 hunks)
💤 Files with no reviewable changes (3)
- app/res/values-lt/strings.xml
- app/res/values-es/strings.xml
- app/res/values-no/strings.xml
🧰 Additional context used
🧬 Code Graph Analysis (1)
app/src/org/commcare/utils/BiometricsHelper.java (1)
app/src/org/commcare/android/database/connect/models/PersonalIdSessionData.kt (2)
requiredLock(9-38)PIN(30-32)
🔇 Additional comments (25)
app/res/layout/screen_login.xml (2)
136-136: Update login_via_connect text to new PersonalId string
Ensures thelogin_via_connectTextView now references the newly prefixedpersonalid_login_via_connect. Confirm that this resource is defined and translated for all supported locales.
278-278: No-op closing tag change
The closing</LinearLayout>was updated but carries no functional change.app/res/layout/screen_personalid_photo_capture.xml (4)
21-29: Rename title string to PersonalId variant
Updated the@+id/titleTextView to use@string/personalid_photo_capture_title. Confirm that this key exists and is localized.
31-39: Rename subtitle string to PersonalId variant
Switched the@+id/subtitleTextView to@string/personalid_photo_capture_subtitle. Ensure translations are in sync.
52-60: Rename take-photo button text
Changed the take-photo button to@string/personalid_photo_capture_take_photo_button. Verify that it's defined across all language packs.
60-67: Rename save-photo button text
Updated the save-photo button to@string/personalid_photo_capture_save_photo_button. Check that the disabled/enabled states still make sense with the new resource.app/src/org/commcare/fragments/MicroImageActivity.java (1)
115-116: Switch to namespaced permission rationale strings
The permission rationale dialog now correctly usespersonalid_camera_permission_titleandpersonalid_camera_permission_msgto align with the PersonalId resource prefix.app/src/org/commcare/fragments/personalId/PersonalIdBiometricConfigFragment.java (2)
35-35: Trivial cleanup of an empty line/import removal; no action needed.
277-277: Use updated PersonalId-specific error title
The security configuration failure dialog now referencespersonalid_configuration_process_failed_title, matching the new resource namespace.app/res/layout/fragment_recovery_code.xml (1)
188-188: Update topersonalid_namespaced string resource
The "Not me" TextView now points to@string/personalid_this_is_not_me, ensuring consistency with PersonalId branding.app/src/org/commcare/fragments/personalId/PersonalIdPhoneFragment.java (2)
227-227: Use namespaced subtitle string for configuration failure
Switched topersonalid_configuration_process_failed_subtitlefor the failure message to keep resource keys consistent.
256-256: Use namespaced title string for configuration failure dialog
Updated topersonalid_configuration_process_failed_titlein the navigation action, aligning with the refactoring.app/src/org/commcare/fragments/personalId/PersonalIdPhoneVerificationFragment.java (1)
108-111: Switch OTP error messages topersonalid_resources
Updated the OTP failure cases (INVALID_CREDENTIAL,TOO_MANY_REQUESTS,MISSING_ACTIVITY,VERIFICATION_FAILED) to use the newpersonalid_prefixed string keys.app/src/org/commcare/utils/BiometricsHelper.java (3)
237-237: Consistent string resource usage in default switch
The newyieldcall usesR.string.personalid_configuration_process_failed_server_msg, aligning with the PersonalId prefix refactor.
248-248: String resource update approved for PIN hardware error
Switched to the newpersonalid_configuration_process_failed_security_subtitlekey for PIN errors.
252-252: String resource update approved for biometric hardware error
Switched to the newpersonalid_configuration_process_failed_security_subtitlekey for biometric errors.app/src/org/commcare/fragments/personalId/PersonalIdBackupCodeFragment.java (2)
89-89: Welcome back message updated to new resource
UsingR.string.personalid_welcome_back_msgwith the correct prefix ensures consistency with the PersonalId namespace.
182-183: Recovery failure dialog strings updated correctly
The dialog now referencespersonalid_recovery_failed_titleandpersonalid_recovery_failed_message, matching the renamed resources.app/res/values-sw/strings.xml (1)
429-433: Add new Swahili OTP error strings
Introducespersonalid_-prefixed OTP error and status messages for Swahili locale.app/res/values-fr/strings.xml (1)
422-426: Add new French OTP error strings
Introducespersonalid_-prefixed OTP error and status messages for French locale.app/res/values-ti/strings.xml (1)
406-411: Approved: New PersonalID OTP strings added
The newly introducedpersonalid_prefixed keys for OTP errors are consistent with the pattern in other locales.app/res/values-pt/strings.xml (1)
420-425: Approved: Localized PersonalID OTP error strings
The addedpersonalid_keys and their Portuguese translations align with the default and other language files.app/res/values/strings.xml (3)
588-588: Approved: Generic PersonalID error string
Addingpersonalid_generic_errorgives a clear fallback message—good improvement.
608-621: Approved: Remaining PersonalID string additions
The otherpersonalid_keys for camera permissions, capture actions, and OTP verification align with naming conventions. Confirm that code correctly handles%sand%dplaceholders.
595-607:⚠️ Potential issueFix formatting and quoting issues
personalid_login_via_connectcontains an explicit\n; remove it or verify it's rendered correctly.personalid_photo_upload_failurevalue is wrapped in extraneous quotes—remove leading/trailing"to avoid literal quotes in UI.- Strings containing HTML tags or escape sequences (
<u>…</u>,\n) should declareformatted="false"or wrap content in CDATA.Suggested diff:
- <string name="personalid_photo_upload_failure">"Failed to save photo. Please check your internet and try again. "</string> + <string name="personalid_photo_upload_failure" formatted="false">Failed to save photo. Please check your internet and try again.</string>- <string name="personalid_login_via_connect">Ready to login!\n</string> + <string name="personalid_login_via_connect" formatted="false">Ready to login!</string>- <string name="personalid_this_is_not_me"><u>This is not me</u></string> + <string name="personalid_this_is_not_me" formatted="false"><![CDATA[<u>This is not me</u>]]></string>Likely an incorrect or invalid review comment.
app/src/org/commcare/fragments/personalId/PersonalIdPhotoCaptureFragment.java
Show resolved
Hide resolved
| case VERIFICATION_FAILED -> getString(R.string.personalid_otp_verification_failed); | ||
| default -> | ||
| getString(R.string.otp_verification_failed) + errorMessage != null ? errorMessage : "Unknown error"; | ||
| getString(R.string.personalid_otp_verification_failed_generic) + errorMessage != null ? errorMessage : "Unknown error"; |
There was a problem hiding this comment.
Fix operator precedence in default OTP error handling
The concatenation with the ternary is parsed incorrectly, causing userMessage to be null. Parenthesize the conditional:
- getString(R.string.personalid_otp_verification_failed_generic) + errorMessage != null ? errorMessage : "Unknown error"
+ getString(R.string.personalid_otp_verification_failed_generic) + (errorMessage != null ? errorMessage : "Unknown error");📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| getString(R.string.personalid_otp_verification_failed_generic) + errorMessage != null ? errorMessage : "Unknown error"; | |
| getString(R.string.personalid_otp_verification_failed_generic) | |
| + (errorMessage != null ? errorMessage : "Unknown error"); |
🤖 Prompt for AI Agents
In
app/src/org/commcare/fragments/personalId/PersonalIdPhoneVerificationFragment.java
at line 113, the concatenation with the ternary operator is parsed incorrectly
due to operator precedence, causing userMessage to be null. Fix this by adding
parentheses around the ternary expression to ensure it evaluates correctly
before concatenation.
|
|
||
| private static void crashWithInvalidSecurityTypeException(Activity activity, String requiredLock) { | ||
| new RuntimeException(activity.getString(R.string.configuration_process_failed_server_msg, requiredLock)); | ||
| new RuntimeException(activity.getString(R.string.personalid_configuration_process_failed_server_msg, requiredLock)); |
There was a problem hiding this comment.
Missing throw on RuntimeException
The helper instantiates a RuntimeException but never throws it. Change to throw new RuntimeException(...) to actually crash on invalid security type.
🤖 Prompt for AI Agents
In app/src/org/commcare/utils/BiometricsHelper.java at line 243, the code
creates a new RuntimeException but does not throw it, so the exception is never
triggered. Fix this by adding the `throw` keyword before `new
RuntimeException(...)` to ensure the exception is properly thrown and the
program crashes on invalid security type.
|
Did a separate PR for CR comments above - #3182 |
Product Description
Remove unused strings
rename to use
personalId_prefixReview by Commit.
Doing this on connect beta first in order to correctly identify unused strings with connect code
Will duplicate this to
commcare_2.57once merged.Labels and Review