Improved wrong backup code error message #3471
Conversation
…l be locked instead of creating a new one.
OrangeAndGreen
requested review from
Jignesh-dimagi,
conroy-ricketts and
shubham1g5
📝 WalkthroughWalkthroughThis pull request updates the localization strings for Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (3 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 |
![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
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
app/res/values-es/strings.xmlapp/res/values-fr/strings.xmlapp/res/values-hi/strings.xmlapp/res/values-pt/strings.xmlapp/res/values-sw/strings.xmlapp/res/values-ti/strings.xmlapp/res/values/strings.xml
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: OrangeAndGreen
Repo: dimagi/commcare-android PR: 3108
File: app/src/org/commcare/activities/StandardHomeActivityUIController.java:0-0
Timestamp: 2025-06-20T13:21:20.908Z
Learning: User OrangeAndGreen prefers to handle code issues in separate PRs rather than immediately fixing them in the current PR when they are not directly related to the main changes.
Learnt from: OrangeAndGreen
Repo: dimagi/commcare-android PR: 3248
File: app/src/org/commcare/connect/network/ApiPersonalId.java:0-0
Timestamp: 2025-07-29T14:56:40.681Z
Learning: User OrangeAndGreen prefers to defer ApiPersonalId refactoring (specifically InputStream resource management improvements) to separate PRs rather than mixing them with Connect feature work, even when the code is already in master.
📚 Learning: 2025-05-22T14:32:53.133Z
Learnt from: OrangeAndGreen
Repo: dimagi/commcare-android PR: 3121
File: app/res/values-ti/strings.xml:350-350
Timestamp: 2025-05-22T14:32:53.133Z
Learning: PersonalID and Connect features haven't been translated to Spanish, Lithuanian, or Norwegian yet, so users with those language settings see the English strings by default.
Applied to files:
app/res/values-es/strings.xmlapp/res/values-pt/strings.xml
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Lint Code Base
🔇 Additional comments (6)
app/res/values-ti/strings.xml (1)
469-469: Tigrinya backup‑code warning matches base behavior and keeps placeholder intactThe updated message correctly describes the account being locked after
%dfurther incorrect attempts, and the%dplaceholder is preserved and positioned appropriately. Looks good.app/res/values-fr/strings.xml (1)
478-478: French string correctly updates to account lock and preserves%dThe new French text accurately reflects that the account will be blocked after
%dadditional incorrect attempts, and the placeholder usage matches the base string.app/res/values-pt/strings.xml (1)
484-484: Portuguese backup‑code message aligned and placeholder‑safeThe Portuguese text now communicates that the account will be blocked after
%dincorrect attempts, and the%dplaceholder is preserved correctly. This is consistent enough with the English base.app/res/values-sw/strings.xml (1)
483-483: Swahili string correctly reflects lockout after further attemptsThe Swahili translation keeps the
%dplaceholder and clearly states the account will be locked after%dmore incorrect attempts. No issues spotted.app/res/values-hi/strings.xml (1)
471-471: Hindi backup‑code text matches new behavior and formattingThe Hindi message now says the account will be locked after
%dmore wrong attempts, with the%dplaceholder preserved and placed naturally. Looks good.app/res/values/strings.xml (1)
635-635: Base English string correctly updated to describe lockoutThe base
personalid_wrong_backup_messagenow reflects that the account will be locked after%dmore incorrect attempts, with the existing%dplaceholder unchanged. This aligns with the PR intent and is safe for all callers.
| <string name="personalid_recovery_failed_message">Parece que olvidaste tu código de respaldo, estamos creando una nueva cuenta para ti</string> | ||
| <string name="personalid_recovery_lockout_message">Ha ingresado el código de respaldo incorrecto demasiadas veces. Su cuenta ha sido bloqueada.</string> | ||
| <string name="personalid_wrong_backup_message">Ingresaste un código de respaldo incorrecto. Por favor, inténtalo de nuevo. Deberás crear una nueva cuenta después de %d intentos incorrectos.</string> | ||
| <string name="personalid_wrong_backup_message">Ha introducido el código de seguridad incorrecto. Inténtelo de nuevo. Su cuenta se bloqueará después de %d intentos incorrectos más.</string> |
There was a problem hiding this comment.
Use consistent terminology: "código de respaldo" instead of "código de seguridad".
The updated message uses "código de seguridad" (security code), but all other related strings in this file consistently use "código de respaldo" (backup code):
- Line 473:
personalid_recovery_lockout_message - Line 472:
personalid_recovery_failed_message - Line 441:
connect_backup_code_message - Line 440:
connect_backup_code_message_title
Since the string key is personalid_wrong_backup_message, it should refer to the backup code, not security code. Using inconsistent terminology for the same concept can confuse users.
🔎 Proposed fix
- <string name="personalid_wrong_backup_message">Ha introducido el código de seguridad incorrecto. Inténtelo de nuevo. Su cuenta se bloqueará después de %d intentos incorrectos más.</string>
+ <string name="personalid_wrong_backup_message">Ha introducido el código de respaldo incorrecto. Inténtelo de nuevo. Su cuenta se bloqueará después de %d intentos incorrectos más.</string>📝 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.
| <string name="personalid_wrong_backup_message">Ha introducido el código de seguridad incorrecto. Inténtelo de nuevo. Su cuenta se bloqueará después de %d intentos incorrectos más.</string> | |
| <string name="personalid_wrong_backup_message">Ha introducido el código de respaldo incorrecto. Inténtelo de nuevo. Su cuenta se bloqueará después de %d intentos incorrectos más.</string> |
🤖 Prompt for AI Agents
In app/res/values-es/strings.xml around line 474, the message uses "código de
seguridad" but the rest of the file and the string key
(personalid_wrong_backup_message) refer to the same concept as "código de
respaldo"; update the string to replace "código de seguridad" with "código de
respaldo" so the wording matches other related strings and avoids user
confusion, keeping the rest of the sentence and the %d placeholder intact.
| <string name="personalid_recovery_failed_message">Looks like you’ve forgotten your Backup Code, we’re creating a new account for you</string> | ||
| <string name="personalid_recovery_lockout_message">You have entered the wrong Backup Code too many times. Your account has been locked.</string> | ||
| <string name="personalid_wrong_backup_message">You have entered the wrong Backup Code. Please try again. You will need to create a new account after %d more incorrect attempts.</string> | ||
| <string name="personalid_wrong_backup_message">You have entered the wrong Backup Code. Please try again. Your account will be locked after %d more incorrect attempts.</string> |
There was a problem hiding this comment.
I'm a bit confused here. If a user sees this message, I thought their only option was to create a new account unless they contact support?
There was a problem hiding this comment.
Unless what we're doing here is making the message more generic?
There was a problem hiding this comment.
The new message doesn't seem to provide a way forward. Do users know that in these situations they are supposed to reach out to Support or a supervisor of some sort?
There was a problem hiding this comment.
@conroy-ricketts There was a change in behavior here. For a short time we had it such that a user's account would automatically be orphaned and a new one created if they failed the backup code too many times. Instead, now accounts are locked in this scenario, and the user has to contact an admin to eventually get a web dev to unlock their account and assign them a new backup code.
@avazirna The message being modified here is shown before they've entered the wrong code too many times, so indicates the path forward simply as "Please try again" with the hope that they'll enter the correct code next time. However, the message that gets shown once their account actually gets locked is on the line above, and it indeed does not provide any guidance on what the user should do next. Perhaps it's worth adding something like "Please contact support to unlock your account"?
There was a problem hiding this comment.
Ah ok thanks for the context!
Perhaps it's worth adding something like "Please contact support to unlock your account"?
I like that idea!
Discovered while working on an interrupt ticket:
https://dimagi.atlassian.net/browse/CI-448
Product Description
Changed error message after wrong backup code to indicate account will be locked instead of creating a new one.
Technical Summary
Simple string change
Feature Flag
None
Safety Assurance
Safety story
Simple change, nothing much to test
Automated test coverage
None
QA Plan
Verify the new message text appears