Fixed double callback in after PersonalID API failure #3188
Conversation
OrangeAndGreen
requested review from
Jignesh-dimagi,
jaypanchal-13,
pm-dimagi and
shubham1g5
📝 WalkthroughWalkthroughThis update introduces a comprehensive set of changes focused on rebranding "ConnectID" features to "PersonalId," enhancing error handling, improving biometric key management, and refining UI and localization resources. Major changes include the addition of key validation and deletion methods to key store handler classes and interfaces, improved error reporting and handling in PersonalId API and OTP flows, and the introduction of new string resources while removing obsolete or unused ones across multiple locales. The update also modifies navigation and UI logic for PersonalId-related fragments, enforces device compatibility checks for sign-in options, and ensures robust biometric key invalidation handling and analytics reporting. Several method signatures and resource references are updated to reflect these improvements. Sequence Diagram(s)sequenceDiagram
participant User
participant LoginActivity
participant PersonalIdManager
participant EncryptionKeyProvider
participant FirebaseAnalyticsUtil
User->>LoginActivity: Initiates PersonalId sign-in
LoginActivity->>PersonalIdManager: unlockConnect()
PersonalIdManager->>EncryptionKeyProvider: isKeyValid()
alt Key invalid
EncryptionKeyProvider-->>PersonalIdManager: false
PersonalIdManager->>FirebaseAnalyticsUtil: reportBiometricInvalidated()
PersonalIdManager->>EncryptionKeyProvider: deleteKey()
PersonalIdManager->>EncryptionKeyProvider: generateEncryptionKey()
else Key valid
EncryptionKeyProvider-->>PersonalIdManager: true
end
PersonalIdManager->>LoginActivity: Proceed with biometric/PIN authentication
sequenceDiagram
participant User
participant PersonalIdPhoneVerificationFragment
participant OtpManager
participant FirebaseAuthService
User->>PersonalIdPhoneVerificationFragment: Requests OTP
PersonalIdPhoneVerificationFragment->>OtpManager: requestOtp(phone)
OtpManager->>FirebaseAuthService: requestOtp(phone)
FirebaseAuthService-->>OtpManager: OTP sent or error (OtpErrorType)
OtpManager-->>PersonalIdPhoneVerificationFragment: Callback with success or error
alt Error
PersonalIdPhoneVerificationFragment->>User: Show error message based on OtpErrorType
else Success
PersonalIdPhoneVerificationFragment->>User: Allow OTP entry
end
User->>PersonalIdPhoneVerificationFragment: Submits OTP
PersonalIdPhoneVerificationFragment->>OtpManager: submitOtp(code)
OtpManager->>FirebaseAuthService: verifyOtp(code)
FirebaseAuthService-->>OtpManager: Verification result (OtpErrorType)
OtpManager-->>PersonalIdPhoneVerificationFragment: Callback with result
alt Error
PersonalIdPhoneVerificationFragment->>User: Show error message based on OtpErrorType
else Success
PersonalIdPhoneVerificationFragment->>User: Proceed to next step
end
Possibly related PRs
Suggested labels
Suggested reviewers
✨ 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: 21
🔭 Outside diff range comments (1)
app/src/org/commcare/fragments/SelectInstallModeFragment.java (1)
213-223: 🛠️ Refactor suggestion
connectEnabledparameter is now dead code – remove or honour it
updateConnectButton()unconditionally setsenabled = false, so:
connectEnabledis ignored.- The Javadoc is misleading.
- Lint will flag an “unused parameter”.
If the temporary shutdown of the button is required, consider:
-public void updateConnectButton(boolean connectEnabled, View.OnClickListener listener) { +// TODO(phase-4): Restore logic once PersonalId rollout is complete +public void updateConnectButton(View.OnClickListener listener) {or keep the parameter but respect it (
boolean enabled = connectEnabled && …).
This avoids confusion for future maintainers.
🧹 Nitpick comments (23)
app/AndroidManifest.xml (1)
583-585: Specify exported attribute for MicroImageActivity
To ensure Android 12+ compliance and maintain consistency with other activities, explicitly define theandroid:exportedattribute.
Apply this diff:- <activity - android:name="org.commcare.fragments.MicroImageActivity" - android:screenOrientation="portrait"/> + <activity + android:name="org.commcare.fragments.MicroImageActivity" + android:exported="false" + android:screenOrientation="portrait"/>app/src/org/commcare/utils/OtpManager.java (1)
23-29: Input validation dropped – double-check downstream error handling
requestOtpandsubmitOtpnow delegate blindly. IfphoneNumber/codeare null or empty,FirebaseAuthServicemust cope; otherwise you’re trading a graceful early return for an NPE or FirebaseIllegalArgumentException.If validation belongs in the service layer that’s fine, but please verify:
// inside FirebaseAuthService.requestOtp(...) Preconditions.checkArgument(!TextUtils.isEmpty(phoneNumber), "phoneNumber required");or add minimal guards here again.
app/src/org/commcare/android/security/KeyStoreHandler.kt (1)
5-9: Consider return feedback fordeleteKey()
deleteKey()isUnit; callers won’t know if deletion failed (e.g.KeyPermanentlyInvalidatedException). ReturningBoolean(success) or throwing a typed exception would make lifecycle management more robust.- fun deleteKey() + /** + * @return true if the key was deleted, false otherwise. + */ + fun deleteKey(): BooleanMinor, but improves diagnosability.
app/src/org/commcare/activities/LoginActivityUIController.java (1)
567-569: Connect button permanently hidden – is the surrounding UI still needed?
setConnectButtonVisible(false)is now unconditional, so the button/or-label never shows.
If this is a temporary kill-switch (per the TODO), fine. Otherwise you can:
- Remove dead layout elements to reduce view inflation cost.
- Strip related state branches (
isAppSelectorVisible, etc.) for clarity.app/src/org/commcare/utils/PhoneNumberHelper.java (1)
95-108: Log parse failures & document sentinel value
getNationalNumber()swallowsNumberParseExceptionand silently returns-1. Returning a magic number without logging obscures debugging.} catch (NumberParseException e) { - // Ignore + Logger.log(LogTypes.TYPE_WARNING, + "PhoneNumberHelper:getNationalNumber invalid phone " + phone);Also add Javadoc noting that
-1means “invalid/unparseable”.app/src/org/commcare/android/security/AndroidKeyStore.kt (1)
16-20: Expose outcome / propagate exceptions
deleteKey()discards anyKeyStoreExceptionraised bycontainsAlias/deleteEntry.
Consider either:
- Wrapping in
runCatching { … }.onFailure { Logger.exception("KS delete", it) }, or- Returning
Booleanto signal success / non-existence to callers.This will make key-deletion failures visible instead of silently continuing.
app/src/org/commcare/connect/database/ConnectDatabaseHelper.java (1)
52-54: Redundant façade method – consider deprecating or in-lining
ConnectDatabaseHelper.dbExists()is now a 1-liner that simply forwards toDatabaseConnectOpenHelper.dbExists().
Unless you foresee additional logic being introduced here, this extra hop adds indirection without benefit and increases the surface area that needs to stay in sync.- public static boolean dbExists() { - return DatabaseConnectOpenHelper.dbExists(); - } +// ✓ Call DatabaseConnectOpenHelper.dbExists() at the call-siteapp/src/org/commcare/utils/OtpErrorType.java (1)
3-9: Enum LGTM – add Javadoc for discoverabilityEnum values are clear and self-descriptive.
A short class-level comment describing when each error is returned will help future maintainers and translators.app/src/org/commcare/AppUtils.java (1)
173-174: Hard-coded duplicate argument feels brittlePassing
ccvtwice relies on parallel string/placeholder changes staying aligned.
Consider building a dedicated DTO or wrapper to assemble the version pieces once and feed bothLocalization.get()calls, avoiding silent drift.app/src/org/commcare/activities/CommCareSetupActivity.java (1)
510-512: Spelling error in API name leaks into public surface
checkDeviceCompability()is miss-spelled. While not functionally incorrect, it does become part of the public API ofPersonalIdManagerand will spread quickly.Consider renaming to the conventional
checkDeviceCompatibility()and updating the call here:-item.setVisible(!fromManager && !fromExternal && !PersonalIdManager.getInstance().isloggedIn() - && PersonalIdManager.getInstance().checkDeviceCompability()); +item.setVisible(!fromManager && !fromExternal + && !PersonalIdManager.getInstance().isloggedIn() + && PersonalIdManager.getInstance().checkDeviceCompatibility());app/src/org/commcare/utils/BiometricsHelper.java (1)
250-254: Typo in comment
// end: min secruity requirements→// end: min security requirementsapp/src/org/commcare/utils/OtpVerificationCallback.java (1)
30-35: Javadoc out of sync with new method signature
onFailurenow takesOtpErrorType+ optional message, but the Javadoc still documents onlymessage.- * @param message A description of the error that occurred + * @param errorType Specific category of OTP error + * @param message Optional description of the errorUpdating docs avoids confusion for implementers.
app/res/values-pt/strings.xml (1)
420-460: Placeholder mismatches & stray markup in new PersonalId stringsA few of the newly-added strings diverge from existing style guidelines:
%s/%dplaceholders should be wrapped with numbered indexes to ease future re-ordering in translations (%1$s,%2$d, …).
Examples:personalid_welcome_back_msg,personalid_wrong_backup_message.
personalid_this_is_not_mecontains HTML underline tags but most UI components now rely onSpannableString. Confirm that the view using this string callsHtml.fromHtml(...), otherwise the markup will be shown verbatim.Ensure every new string is referenced; dead resources add to translation burden.
Please double-check before merging.
app/src/org/commcare/fragments/personalId/PersonalIdBiometricConfigFragment.java (1)
258-263: Guard against redundant instantiationEvery call creates a fresh
EncryptionKeyProvider. A simple early-return when the key is already valid prevents unnecessary object creation and keystore access:private void storeBiometricInvalidationKey() { - new EncryptionKeyProvider(requireContext(), true, BIOMETRIC_INVALIDATION_KEY) - .getKeyForEncryption(); + EncryptionKeyProvider provider = + new EncryptionKeyProvider(requireContext(), true, BIOMETRIC_INVALIDATION_KEY); + if (!provider.isKeyValid()) { + provider.getKeyForEncryption(); + } }app/src/org/commcare/models/database/connect/DatabaseConnectOpenHelper.java (1)
76-78: Failure handling for deletion
deleteDb()ignores the return value ofFile#delete()and silent failures go unnoticed.
Return a boolean or log the outcome so callers can react.app/src/org/commcare/utils/FirebaseAuthService.java (1)
86-99: Fall-through for unknownExceptionproduces user-visible stacktrace
handleFirebaseExceptionthrows a generic message for anything not mapped, but still leaks the originale.getMessage()to the UI.
Consider logging the raw exception and surface a localized, user-friendly message instead.app/src/org/commcare/connect/network/PersonalIdApiErrorHandler.java (2)
31-53: Returning empty string can confuse callersFor
TOKEN_DENIED_ERRORthe method returns"".
Call-sites now need additional checks (isEmpty) to decide whether to display a message. Returningnullor a dedicated constant would be clearer.
49-53: Default branch escalates to crash for benign errorsThrowing
RuntimeExceptionfor anUNKNOWN_ERRORbubbles up to the UI and may crash the app. If analytics or graceful degradation is desired, return a generic message and log the throwable instead.app/src/org/commcare/android/security/AesKeyStoreHandler.kt (1)
53-62: Simplify redundant type checking in getKeyIfExists().The method checks if the entry is a
SecretKeyEntrytwice (lines 55 and 57), which is redundant.Apply this diff to remove the redundant check:
fun getKeyIfExists(): SecretKey? { val keystore = AndroidKeyStore.instance - if (keystore.containsAlias(alias) && keystore.getEntry(alias, null) is KeyStore.SecretKeyEntry) { - val entry = keystore.getEntry(alias, null) - if (entry is KeyStore.SecretKeyEntry) { - return entry.secretKey - } + if (keystore.containsAlias(alias)) { + val entry = keystore.getEntry(alias, null) + if (entry is KeyStore.SecretKeyEntry) { + return entry.secretKey + } } return null }app/res/values-fr/strings.xml (1)
276-276: Missing terminal punctuation: The message “Veuillez configurer une méthode ci-dessous pour déverrouiller à la fois votre appareil et votre compte PersonalId” lacks a period at the end. Add a trailing period to maintain consistency with other full-sentence strings.app/src/org/commcare/fragments/personalId/PersonalIdPhoneFragment.java (1)
147-159: Well-implemented phone number parsing with proper validation.The method correctly handles edge cases and only updates the UI when valid data is available. The separation of country code and national number improves user experience.
Consider adding a space after
iffor consistency with Java style guidelines:- if(TextUtils.isEmpty(fullPhoneNumber))return; + if (TextUtils.isEmpty(fullPhoneNumber)) return; - if(countryCodeFromFullPhoneNumber!=-1 && nationPhoneNumberFromFullPhoneNumber!=-1){ + if (countryCodeFromFullPhoneNumber != -1 && nationPhoneNumberFromFullPhoneNumber != -1) {app/src/org/commcare/connect/network/PersonalIdApiHandler.java (1)
88-104: Good error message extraction with room for improvement.The error response parsing correctly extracts error messages and provides fallback handling.
Consider removing the Toast display from this handler class to maintain separation of concerns. The error message should be returned to the UI layer which can then decide how to display it:
if (json.has("error")) { info.append(": ").append(json.optString("error")); - Toast.makeText(CommCareApplication.instance(), json.optString("error"), - Toast.LENGTH_LONG).show(); }app/src/org/commcare/connect/PersonalIdManager.java (1)
686-689: Device compatibility check for PersonalId features.The method correctly checks for Android N (API 24) which is required for certain security features used by PersonalId.
Consider adding a comment explaining why API 24 is the minimum requirement:
+ /** + * Checks if the device meets the minimum API requirements for PersonalId. + * API 24 (Android N) is required for certain security features. + */ public boolean checkDeviceCompability() { return Build.VERSION.SDK_INT >= Build.VERSION_CODES.N; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (50)
app/AndroidManifest.xml(2 hunks)app/assets/locales/android_translatable_strings.txt(1 hunks)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_phoneno.xml(1 hunks)app/res/layout/screen_personalid_photo_capture.xml(3 hunks)app/res/navigation/nav_graph_personalid.xml(1 hunks)app/res/values-es/strings.xml(0 hunks)app/res/values-fr/strings.xml(3 hunks)app/res/values-lt/strings.xml(0 hunks)app/res/values-no/strings.xml(0 hunks)app/res/values-pt/strings.xml(3 hunks)app/res/values-sw/strings.xml(3 hunks)app/res/values-ti/strings.xml(3 hunks)app/res/values/strings.xml(3 hunks)app/src/org/commcare/AppUtils.java(1 hunks)app/src/org/commcare/activities/CommCareSetupActivity.java(2 hunks)app/src/org/commcare/activities/GeoPointMapActivity.java(1 hunks)app/src/org/commcare/activities/LoginActivity.java(2 hunks)app/src/org/commcare/activities/LoginActivityUIController.java(1 hunks)app/src/org/commcare/android/security/AesKeyStoreHandler.kt(2 hunks)app/src/org/commcare/android/security/AndroidKeyStore.kt(1 hunks)app/src/org/commcare/android/security/KeyStoreHandler.kt(1 hunks)app/src/org/commcare/android/security/RsaKeyStoreHandler.kt(1 hunks)app/src/org/commcare/connect/PersonalIdManager.java(9 hunks)app/src/org/commcare/connect/database/ConnectDatabaseHelper.java(1 hunks)app/src/org/commcare/connect/database/ConnectUserDatabaseUtil.java(2 hunks)app/src/org/commcare/connect/network/ApiPersonalId.java(1 hunks)app/src/org/commcare/connect/network/PersonalIdApiErrorHandler.java(1 hunks)app/src/org/commcare/connect/network/PersonalIdApiHandler.java(2 hunks)app/src/org/commcare/fragments/MicroImageActivity.java(1 hunks)app/src/org/commcare/fragments/SelectInstallModeFragment.java(1 hunks)app/src/org/commcare/fragments/personalId/PersonalIdBackupCodeFragment.java(6 hunks)app/src/org/commcare/fragments/personalId/PersonalIdBiometricConfigFragment.java(5 hunks)app/src/org/commcare/fragments/personalId/PersonalIdMessageFragment.java(3 hunks)app/src/org/commcare/fragments/personalId/PersonalIdNameFragment.java(3 hunks)app/src/org/commcare/fragments/personalId/PersonalIdPhoneFragment.java(7 hunks)app/src/org/commcare/fragments/personalId/PersonalIdPhoneVerificationFragment.java(8 hunks)app/src/org/commcare/fragments/personalId/PersonalIdPhotoCaptureFragment.java(2 hunks)app/src/org/commcare/google/services/analytics/CCAnalyticsEvent.java(1 hunks)app/src/org/commcare/google/services/analytics/FirebaseAnalyticsUtil.java(1 hunks)app/src/org/commcare/models/database/connect/DatabaseConnectOpenHelper.java(2 hunks)app/src/org/commcare/utils/BiometricsHelper.java(1 hunks)app/src/org/commcare/utils/EncryptionKeyProvider.java(1 hunks)app/src/org/commcare/utils/FirebaseAuthService.java(2 hunks)app/src/org/commcare/utils/OtpErrorType.java(1 hunks)app/src/org/commcare/utils/OtpManager.java(2 hunks)app/src/org/commcare/utils/OtpVerificationCallback.java(2 hunks)app/src/org/commcare/utils/PhoneNumberHelper.java(1 hunks)
💤 Files with no reviewable changes (3)
- app/res/values-no/strings.xml
- app/res/values-es/strings.xml
- app/res/values-lt/strings.xml
🧰 Additional context used
🧬 Code Graph Analysis (12)
app/src/org/commcare/google/services/analytics/FirebaseAnalyticsUtil.java (1)
app/src/org/commcare/google/services/analytics/CCAnalyticsEvent.java (1)
CCAnalyticsEvent(7-55)
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)
app/src/org/commcare/activities/CommCareSetupActivity.java (1)
app/src/org/commcare/connect/PersonalIdManager.java (1)
PersonalIdManager(70-715)
app/src/org/commcare/connect/database/ConnectDatabaseHelper.java (1)
app/src/org/commcare/models/database/connect/DatabaseConnectOpenHelper.java (1)
DatabaseConnectOpenHelper(39-165)
app/src/org/commcare/utils/EncryptionKeyProvider.java (4)
app/src/org/commcare/android/security/AesKeyStoreHandler.kt (2)
isKeyValid(38-47)deleteKey(49-51)app/src/org/commcare/android/security/KeyStoreHandler.kt (2)
isKeyValid(7-7)deleteKey(8-8)app/src/org/commcare/android/security/RsaKeyStoreHandler.kt (2)
isKeyValid(33-35)deleteKey(37-39)app/src/org/commcare/android/security/AndroidKeyStore.kt (1)
deleteKey(16-20)
app/src/org/commcare/connect/database/ConnectUserDatabaseUtil.java (3)
app/src/org/commcare/connect/database/ConnectDatabaseHelper.java (1)
ConnectDatabaseHelper(34-126)app/src/org/commcare/models/database/connect/DatabaseConnectOpenHelper.java (1)
DatabaseConnectOpenHelper(39-165)app/src/org/commcare/CommCareApplication.java (1)
CommCareApplication(154-1246)
app/src/org/commcare/models/database/connect/DatabaseConnectOpenHelper.java (1)
app/src/org/commcare/CommCareApplication.java (1)
CommCareApplication(154-1246)
app/src/org/commcare/fragments/personalId/PersonalIdBiometricConfigFragment.java (4)
app/src/org/commcare/utils/EncryptionKeyProvider.java (1)
EncryptionKeyProvider(15-56)app/src/org/commcare/android/database/connect/models/PersonalIdSessionData.kt (1)
PIN(30-32)app/src/org/commcare/connect/PersonalIdManager.java (1)
PersonalIdManager(70-715)app/src/org/commcare/connect/ConnectConstants.java (1)
ConnectConstants(8-54)
app/src/org/commcare/connect/network/PersonalIdApiErrorHandler.java (1)
app/src/org/commcare/connect/network/ConnectNetworkHelper.java (1)
ConnectNetworkHelper(46-464)
app/src/org/commcare/fragments/personalId/PersonalIdNameFragment.java (1)
app/src/org/commcare/connect/network/PersonalIdApiErrorHandler.java (1)
PersonalIdApiErrorHandler(21-55)
app/src/org/commcare/fragments/personalId/PersonalIdBackupCodeFragment.java (2)
app/src/org/commcare/connect/ConnectConstants.java (1)
ConnectConstants(8-54)app/src/org/commcare/connect/network/PersonalIdApiErrorHandler.java (1)
PersonalIdApiErrorHandler(21-55)
app/src/org/commcare/fragments/personalId/PersonalIdPhoneFragment.java (1)
app/src/org/commcare/connect/network/PersonalIdApiErrorHandler.java (1)
PersonalIdApiErrorHandler(21-55)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Lint Code Base
🔇 Additional comments (61)
app/src/org/commcare/activities/GeoPointMapActivity.java (1)
71-72: Request window feature before super.onCreate
RepositioningrequestWindowFeature(Window.FEATURE_NO_TITLE)to occur beforesuper.onCreate(savedInstanceState)aligns with Android’s requirement to request window features before the window is created. Please verify on all supported API levels to ensure noIllegalStateExceptionis thrown during startup.app/AndroidManifest.xml (1)
5-5: Verify versionName and versionCode alignment
Theandroid:versionNameis set to "2.57" without a correspondingversionCodebump and appears to be a downgrade. Please confirm that this change matches your release/versioning strategy to avoid unintended update rollbacks.app/res/layout/screen_login.xml (1)
136-136: Resource reference updated correctly to PersonalId namespace
The login prompt now uses@string/personalid_login_via_connect, aligning with the PersonalId branding.app/res/layout/screen_personalid_name.xml (1)
37-37: Text resource updated to PersonalId variant
Changed subtitle to reference@string/personalid_name_fragment_subtitle, keeping naming conventions consistent.app/res/layout/fragment_recovery_code.xml (1)
188-188: Use PersonalId-specific string for "Not Me" text
The button now correctly references@string/personalid_this_is_not_me, matching other PersonalId UI elements.app/src/org/commcare/google/services/analytics/CCAnalyticsEvent.java (1)
53-53: Added Biometric Invalidated analytics event constant
IntroducedCCC_BIOMETRIC_INVALIDATEDto track biometric key invalidation, consistent with existingccc_event naming.app/res/layout/screen_personalid_photo_capture.xml (1)
27-66: Verify presence of newly-named string resources across all localesAll hard-coded references now point to the
personalid_*keys. A missing entry in anyvalues-xx/strings.xmlwill crash the screen at runtime (Resources$NotFoundException).
Quick grep through the repo (or Android Studio’s “Search in Path”) is recommended before merging.app/src/org/commcare/google/services/analytics/FirebaseAnalyticsUtil.java (1)
529-532: New analytics event looks good
CCC_BIOMETRIC_INVALIDATEDconstant exists and the helper follows the existing pattern.app/res/layout/screen_personalid_phoneno.xml (1)
117-126:Details
✅ Verification successful
Confirm all Java/Kotlin references switched to the new view-ID
The
TextView’s ID has been renamed topersonalid_phone_error.
Please grep the codebase for the old ID (personalid_name_error) to make sure no runtimefindViewById()/ViewBindinglookup is left dangling.A stale reference will crash the app the first time the error view is accessed.
🏁 Script executed:
rg -n "personalid_name_error"Length of output: 134
No stale
personalid_name_errorreferences in code
Thergsearch only found the old ID in its own layout (app/res/layout/screen_personalid_name.xml:82) and no Java/Kotlin lookups (findViewByIdor view-binding) remain. Everything else has been correctly renamed topersonalid_phone_error.app/res/navigation/nav_graph_personalid.xml (1)
38-39: Navigation back-stack change – ensure UX intent
popUpTo="@id/personalid_phone_fragment"clears the OTP page before pushingpersonalid_name.
Result: back-press from Name returns to Phone (skipping OTP). Confirm this matches the desired recovery flow, otherwise users won’t be able to re-enter the OTP without restarting the flow.app/assets/locales/android_translatable_strings.txt (1)
211-212: Placeholder re-ordering and ${5} usage – double-check against runtime arrayThe string now expects six tokens (0-5).
After the recent changeprofileVersionis index 5, shown as “App v${5}”. Confirm that this is indeed the intended app version (profile) and not build number / CC version to avoid misleading UI.app/src/org/commcare/utils/EncryptionKeyProvider.java (1)
48-55: New helper methods look good
isKeyValid()anddeleteKey()cleanly delegate to the appropriate keystore handler. No correctness or concurrency issues spotted. 👍app/src/org/commcare/connect/network/ApiPersonalId.java (1)
243-248: Good fix – eliminates duplicate failure callbacksReplacing the previous double-dispatch with a single
processFailurecall prevents race conditions in UI fragments that assumed exactly one failure callback. Nice catch.app/src/org/commcare/fragments/personalId/PersonalIdBiometricConfigFragment.java (1)
221-226: 👍 Correct result propagationForwarding the actual
resultCodeinstead of blindly usingRESULT_OKfixes the double-callback bug and preserves caller intent.app/src/org/commcare/utils/FirebaseAuthService.java (1)
51-54: Timeout set to0L– verify default/edge-casePassing
0tosetTimeoutis undocumented in some Firebase versions and may be treated as immediate timeout rather than “use default”.
Confirm with the SDK docs or restore a safe value (e.g. 60 s).app/src/org/commcare/fragments/personalId/PersonalIdPhotoCaptureFragment.java (3)
78-79: LGTM! String resource updates align with PersonalID rebranding.The string resource keys have been properly updated from generic ConnectID to PersonalID-specific keys.
85-110: Good error handling improvements!The error handling has been enhanced with:
- Clearing existing errors before starting upload (line 85)
- Updated failure callback to accept Throwable parameter for better error context
- Centralized error handling using PersonalIdApiErrorHandler.handle()
- Proper retry logic based on error code
137-145: Clean implementation of error display methods.The
clearError()andshowError()methods provide a consistent way to manage error visibility across the fragment.app/src/org/commcare/fragments/personalId/PersonalIdBackupCodeFragment.java (3)
70-70: Good code organization improvement.Moving the
isRecoveryinitialization to the beginning of the method improves readability by establishing the mode before UI configuration.
172-198: Excellent error handling enhancements!The improvements include:
- Clearing errors before API calls (line 172)
- Enhanced failure callback with Throwable parameter for better error context
- Centralized error message handling via PersonalIdApiErrorHandler
- Proper retry logic based on error codes
214-223: Clean error display implementation.The
clearError()andshowError()methods follow the same pattern as other PersonalID fragments, ensuring consistency.app/src/org/commcare/fragments/personalId/PersonalIdNameFragment.java (2)
77-94: Consistent error handling implementation.The error handling follows the established pattern:
- Clears existing errors before API calls
- Enhanced failure callback with Throwable parameter
- Delegates to navigateFailure for error processing
97-113: Well-structured error handling methods.The error handling is properly centralized with:
- Use of PersonalIdApiErrorHandler.handle() for consistent error messages
- Clear separation of concerns between error processing and UI updates
- Proper retry logic based on error codes
app/src/org/commcare/android/security/AesKeyStoreHandler.kt (2)
23-26: Good practice: Centralized cipher transformation constant.Extracting the transformation string to a companion object constant improves maintainability.
38-47: Excellent key validation implementation.The
isKeyValid()method properly checks key validity by attempting cipher initialization and catchingKeyPermanentlyInvalidatedException, which occurs when biometric enrollment changes invalidate the key.app/src/org/commcare/fragments/personalId/PersonalIdPhoneVerificationFragment.java (5)
59-59: Excellent lifecycle management for OTP callback!Promoting the callback to a class field and adding null checks (lines 86, 92, 105) prevents crashes if callbacks are invoked after fragment destruction. Clearing the reference in
onDestroyView()prevents memory leaks.Also applies to: 83-119, 222-222
104-117: Great user experience improvement with specific OTP error messages!The switch expression maps
OtpErrorTypeenum values to user-friendly localized messages, providing clear feedback for different failure scenarios.
197-198: Proper lifecycle management for timer and receiver.Moving the timer start and SMS receiver registration to
onResume()and their cleanup toonPause()ensures they're only active when the fragment is visible to the user.Also applies to: 205-205
182-186: Good defensive programming with null/empty check.The check prevents showing empty error messages in the UI.
273-273: Good UX: Prevent double submissions.Disabling the verify button immediately prevents users from accidentally submitting the OTP multiple times.
app/src/org/commcare/fragments/personalId/PersonalIdPhoneFragment.java (8)
74-74: Good addition for clickable consent links.Enabling link movement method allows users to click on the privacy policy and terms links in the consent text.
147-159: Well-implemented phone number parsing with proper validation.The method correctly handles empty input and validates parsed components before setting UI fields.
211-217: Correct implementation of the updated failure callback.The changes properly handle the new
onFailuresignature with the throwable parameter and differentiate between forbidden errors (device configuration failures) and other errors. This aligns with the PersonalIdApiHandler changes to fix the double callback issue.
232-248: Clean error handling implementation.The separation of error display logic into dedicated methods and use of PersonalIdApiErrorHandler for message generation improves maintainability.
74-74: Good addition for clickable links in consent text.Setting LinkMovementMethod enables users to interact with links in the consent text, improving the user experience.
171-171: Excellent error handling improvements.The changes provide better user feedback by:
- Clearing previous errors before new operations
- Distinguishing between configuration failures (FORBIDDEN_ERROR) and other recoverable errors
- Displaying specific error messages in the UI
Also applies to: 212-217, 232-238
240-248: Clean error display helper methods.The helper methods provide a consistent way to show and clear error messages in the UI, improving code maintainability.
201-201: Fixed typo in method name.app/src/org/commcare/connect/network/PersonalIdApiHandler.java (6)
27-44: Well-structured error code additions with appropriate retry logic.The new error codes provide better granularity for error handling, and the retry logic correctly includes transient errors (server errors and rate limiting).
46-64: Clean refactoring of callback creation and error handling.The simplified
createCallbacksignature and addition of theThrowableparameter toonFailureprovides better error context. The distinction between JSON parsing and IO errors inprocessSuccessis appropriate.Also applies to: 165-165
67-104:⚠️ Potential issueCritical fix for double callback issue implemented correctly.
The early returns after each
onFailurecall ensure the callback is invoked exactly once, preventing the double callback issue. The error handling is well-structured with specific HTTP status codes mapped to appropriate error types.However, consider moving the Toast display after successful JSON parsing to avoid showing a toast when parsing fails:
if (errorResponse != null) { try (InputStream in = errorResponse) { JSONObject json = new JSONObject(new String(StreamsUtil.inputStreamToByteArray(in))); if (json.has("error")) { info.append(": ").append(json.optString("error")); - Toast.makeText(CommCareApplication.instance(), json.optString("error"), - Toast.LENGTH_LONG).show(); } + Toast.makeText(CommCareApplication.instance(), json.optString("error"), + Toast.LENGTH_LONG).show(); } catch (JSONException | IOException e) {Likely an incorrect or invalid review comment.
28-43: Comprehensive error code additions with appropriate retry logic.The new error codes provide granular error handling for different API failure scenarios. The retry logic correctly allows retries only for transient errors (network issues, server errors, rate limiting).
55-61: Proper exception categorization for better error handling.The separation of JSON parsing errors from IO errors allows the UI to provide more specific feedback to users.
68-86: Excellent HTTP status code handling following REST conventions.The explicit handling of different HTTP status codes allows for appropriate user feedback:
- 401: Authentication failures
- 403: Permission/configuration issues
- 429/503: Rate limiting/temporary unavailability
- 500: Server errors
This granularity enables better retry strategies and user messaging.
app/src/org/commcare/connect/PersonalIdManager.java (6)
71-71: Excellent biometric key invalidation detection and recovery.The implementation proactively detects when biometric keys are invalidated (e.g., when user adds new fingerprints), reports analytics for monitoring, and regenerates the key for future checks.
Also applies to: 187-187, 221-232
522-525: Proper resource management with try-with-resources.Good fix to prevent potential resource leaks by ensuring the InputStream is automatically closed.
686-689: Appropriate device compatibility check.The API level 24 (Android N) requirement is reasonable for PersonalID security features.
221-232: Excellent biometric key invalidation detection and logging.This implementation properly detects when biometric credentials have been invalidated (e.g., when users add new fingerprints) and:
- Reports the event for analytics
- Regenerates the key to maintain security
This is a critical security feature that helps maintain the integrity of biometric authentication.
522-524: Good resource management with try-with-resources.Using try-with-resources ensures the InputStream is properly closed, preventing resource leaks.
187-187: Strategic placement of biometric invalidation check.Calling
logBiometricInvalidations()at the start ofunlockConnectensures that key invalidations are detected and handled before attempting authentication, preventing potential authentication failures.app/res/values-sw/strings.xml (1)
1-464: Localization updates for PersonalID rebranding.The string resource changes appropriately replace ConnectID references with PersonalID and add new strings for PersonalID-specific features.
app/res/values/strings.xml (2)
228-232: Well-crafted error messages for API failures.The new recovery network error messages provide clear, user-friendly descriptions that align with the error codes in PersonalIdApiHandler.
590-622: Comprehensive PersonalID error and UI strings.The new PersonalID strings provide clear messaging for various error scenarios and user interactions, supporting the improved error handling in the PersonalID flow.
app/res/values-fr/strings.xml (5)
278-278: Approve code changes: The keyconnect_verify_fingerprint_configuredcorrectly references PersonalId in its value and is well‐translated.
281-281: Approve code changes: The keyconnect_verify_pin_configuredand its updated value align with the PersonalId branding.
422-426: Approve code changes: OTP error strings are now consistently prefixed withpersonalid_and the French translations are accurate.
427-436: Clarify resource key prefix: The keys in this block (connect_registration_subtitle,connect_backup_code_message_title, etc.) still useconnect_but seem to support PersonalId flows (backup codes, messaging failures). Can you confirm whether these should be renamed topersonalid_?
444-457: Approve code changes: Photo capture and camera permission strings are correctly prefixed withpersonalid_, and the French translations appear accurate and complete.app/res/values-ti/strings.xml (3)
235-237: LGTM! Consistent rebranding to PersonalId.The string resources have been properly updated to reflect the PersonalId branding, replacing the previous ConnectID references.
406-410: Well-structured OTP error messages.The error messages provide clear and specific feedback for different OTP verification failure scenarios, which will help users understand and resolve issues.
411-449: Comprehensive PersonalId UI strings added.The new strings provide complete coverage for PersonalId functionality including registration, messaging, recovery flows, and camera permissions. The naming convention is consistent throughout.
| getString(R.string.personalid_camera_permission_title), | ||
| getString(R.string.personalid_camera_permission_msg)); | ||
| dialog.showNonPersistentDialog(this); |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Runtime-string existence check
The new keys personalid_camera_permission_title and personalid_camera_permission_msg must exist in all values-* folders or Localization.get() will fall back to the default locale and may display the resource ID on devices whose language hasn’t been updated yet.
Consider running the static string-lint or spot-checking:
🏁 Script executed:
#!/bin/bash
fd --glob "values*/strings.xml" | xargs grep -Hn "personalid_camera_permission_"Length of output: 80
🏁 Script executed:
#!/bin/bash
# Search for the new string keys anywhere in the repository
grep -R 'personalid_camera_permission_' -n .Length of output: 1832
🏁 Script executed:
#!/bin/bash
# Identify any values*/strings.xml files that are missing the new keys
mapfile -t files < <(fd --glob "app/res/values*/strings.xml")
for f in "${files[@]}"; do
if ! grep -q 'personalid_camera_permission_' "$f"; then
echo "Missing in: $f"
fi
doneLength of output: 170
🏁 Script executed:
#!/bin/bash
# Verify that both new string keys exist in every values*/strings.xml
shopt -s nullglob
for f in app/res/values*/strings.xml; do
if ! grep -q "personalid_camera_permission_title" "$f" || ! grep -q "personalid_camera_permission_msg" "$f"; then
echo "Missing keys in: $f"
fi
doneLength of output: 2527
Add missing translations for new camera-permission strings
The keys personalid_camera_permission_title and personalid_camera_permission_msg are only defined in the default and a subset of language folders (fr, pt, sw, ti). They’re missing in your Spanish, Lithuanian, and Norwegian resource files:
- app/res/values-es/strings.xml
- app/res/values-lt/strings.xml
- app/res/values-no/strings.xml
Please add entries for both keys to each of these files to prevent fallback to the default locale.
🤖 Prompt for AI Agents
In app/src/org/commcare/fragments/MicroImageActivity.java around lines 115 to
117, the new string resource keys personalid_camera_permission_title and
personalid_camera_permission_msg are used but missing in some localization
files. To fix this, add these two string entries to the strings.xml files in the
Spanish (values-es), Lithuanian (values-lt), and Norwegian (values-no) resource
folders to ensure all locales have these keys and prevent fallback to the
default language.
| menu.findItem(MENU_CONNECT_SIGN_IN).setVisible( | ||
| !personalIdManager.isloggedIn() && personalIdManager.checkDeviceCompability()); | ||
| menu.findItem(MENU_CONNECT_FORGET).setVisible(personalIdManager.isloggedIn()); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Typo in API name propagates – decide now before it spreads
checkDeviceCompability() is invoked here. The word should be “Compatibility”.
Leaving the misspelling will cement it in the public API of PersonalIdManager, making future cleanup painful.
- !personalIdManager.isloggedIn() && personalIdManager.checkDeviceCompability());
+ !personalIdManager.isloggedIn() && personalIdManager.checkDeviceCompatibility());Follow-up: rename the method in PersonalIdManager and all call-sites.
📝 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.
| menu.findItem(MENU_CONNECT_SIGN_IN).setVisible( | |
| !personalIdManager.isloggedIn() && personalIdManager.checkDeviceCompability()); | |
| menu.findItem(MENU_CONNECT_FORGET).setVisible(personalIdManager.isloggedIn()); | |
| menu.findItem(MENU_CONNECT_SIGN_IN).setVisible( | |
| !personalIdManager.isloggedIn() && personalIdManager.checkDeviceCompatibility()); | |
| menu.findItem(MENU_CONNECT_FORGET).setVisible(personalIdManager.isloggedIn()); |
🤖 Prompt for AI Agents
In app/src/org/commcare/activities/LoginActivity.java around lines 553 to 555,
the method checkDeviceCompability() is misspelled and should be renamed to
checkDeviceCompatibility(). Rename this method in the PersonalIdManager class
and update all call-sites accordingly to use the correct spelling, ensuring
consistency and preventing the typo from becoming part of the public API.
| uiController.setErrorMessageUI(getString(R.string.personalid_failed_to_login_with_connectid), false); | ||
| } | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify new string resource exists in all locales
personalid_failed_to_login_with_connectid replaces the old key.
Missing entries in one of the locale XMLs will crash at runtime (Resources$NotFoundException).
Run:
🏁 Script executed:
#!/bin/bash
# Ensure the string is defined for every strings.xml in the repo
fd strings.xml | xargs grep -L '<string name="personalid_failed_to_login_with_connectid">'Length of output: 379
Add missing string resource entries for all locales
The new key personalid_failed_to_login_with_connectid is defined in the default values/strings.xml but missing in several locale-specific resource files. Please add the following entry (with appropriate translations) to each of these:
- app/res/values-es/strings.xml
- app/res/values-large/strings.xml
- app/res/values-lt/strings.xml
- app/res/values-no/strings.xml
- app/res/values-small/strings.xml
- app/res/values-sw720dp-land/strings.xml
- app/res/values-xlarge/strings.xml
- commcare-support-library/src/main/res/values/strings.xml
Example entry:
<string name="personalid_failed_to_login_with_connectid">Failed to login with Connect ID</string>This will prevent a Resources$NotFoundException at runtime.
🤖 Prompt for AI Agents
In app/src/org/commcare/activities/LoginActivity.java around lines 529 to 531,
the string resource key personalid_failed_to_login_with_connectid is used but
missing in several locale-specific strings.xml files. To fix this, add the
string entry <string name="personalid_failed_to_login_with_connectid">Failed to
login with Connect ID</string> to all locale-specific resource files listed,
including app/res/values-es/strings.xml, app/res/values-large/strings.xml,
app/res/values-lt/strings.xml, app/res/values-no/strings.xml,
app/res/values-small/strings.xml, app/res/values-sw720dp-land/strings.xml,
app/res/values-xlarge/strings.xml, and
commcare-support-library/src/main/res/values/strings.xml, ensuring appropriate
translations are provided to prevent runtime ResourceNotFoundException.
| case ConnectConstants.PERSONALID_RECOVERY_ACCOUNT_ORPHANED: | ||
| personalIdSessionData.setAccountExists(false); | ||
| directions = navigateToBackupCode(); | ||
| break; |
There was a problem hiding this comment.
Null-safe write to PersonalIdSessionData
Same NPE concern when hitting the orphaned-account path. Add a null check (or require the instance earlier).
-case ConnectConstants.PERSONALID_RECOVERY_ACCOUNT_ORPHANED:
- personalIdSessionData.setAccountExists(false);
- directions = navigateToBackupCode();
- break;
+case ConnectConstants.PERSONALID_RECOVERY_ACCOUNT_ORPHANED:
+ if (personalIdSessionData != null) {
+ personalIdSessionData.setAccountExists(false);
+ }
+ directions = navigateToBackupCode();
+ break;📝 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.
| case ConnectConstants.PERSONALID_RECOVERY_ACCOUNT_ORPHANED: | |
| personalIdSessionData.setAccountExists(false); | |
| directions = navigateToBackupCode(); | |
| break; | |
| case ConnectConstants.PERSONALID_RECOVERY_ACCOUNT_ORPHANED: | |
| if (personalIdSessionData != null) { | |
| personalIdSessionData.setAccountExists(false); | |
| } | |
| directions = navigateToBackupCode(); | |
| break; |
🤖 Prompt for AI Agents
In app/src/org/commcare/fragments/personalId/PersonalIdMessageFragment.java
around lines 143 to 146, the code writes to personalIdSessionData without
checking if it is null, risking a NullPointerException on the orphaned-account
path. Add a null check before calling setAccountExists(false) on
personalIdSessionData, or ensure personalIdSessionData is initialized earlier so
it cannot be null at this point.
| personalIdSessionData = new ViewModelProvider(requireActivity()).get( | ||
| PersonalIdSessionDataViewModel.class).getPersonalIdSessionData(); | ||
| binding.connectMessageButton.setOnClickListener(v -> handleContinueButtonPress()); |
There was a problem hiding this comment.
Possible NPE – guard against a null PersonalIdSessionData instance
ViewModelProvider#getPersonalIdSessionData() may legally return null (for instance, if the session has not been initialised yet).
A null value will crash later when setAccountExists(false) is called.
-personalIdSessionData = new ViewModelProvider(requireActivity()).get(
- PersonalIdSessionDataViewModel.class).getPersonalIdSessionData();
+personalIdSessionData = new ViewModelProvider(requireActivity())
+ .get(PersonalIdSessionDataViewModel.class)
+ .getPersonalIdSessionData();
+
+if (personalIdSessionData == null) {
+ Log.w(TAG, "PersonalIdSessionData not available – proceeding with a null object");
+}📝 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.
| personalIdSessionData = new ViewModelProvider(requireActivity()).get( | |
| PersonalIdSessionDataViewModel.class).getPersonalIdSessionData(); | |
| binding.connectMessageButton.setOnClickListener(v -> handleContinueButtonPress()); | |
| personalIdSessionData = new ViewModelProvider(requireActivity()) | |
| .get(PersonalIdSessionDataViewModel.class) | |
| .getPersonalIdSessionData(); | |
| if (personalIdSessionData == null) { | |
| Log.w(TAG, "PersonalIdSessionData not available – proceeding with a null object"); | |
| } | |
| binding.connectMessageButton.setOnClickListener(v -> handleContinueButtonPress()); |
🤖 Prompt for AI Agents
In app/src/org/commcare/fragments/personalId/PersonalIdMessageFragment.java
around lines 50 to 52, the code assigns personalIdSessionData without checking
for null, which can cause a NullPointerException later. Add a null check after
retrieving personalIdSessionData from the ViewModelProvider; if it is null,
handle this case appropriately (e.g., initialize it or avoid calling methods on
it) before proceeding to prevent crashes.
| <string name="connect_register_name">Nom</string> | ||
| <string name="connect_register_success_title">Compte créé</string> | ||
| <string name="connect_register_success_message">Votre compte ConnectID a été créé avec succès et est maintenant prêt à être utilisé!</string> | ||
| <string name="connect_register_success_message">Votre compte PersonalId a été créé avec succès et est maintenant prêt à être utilisé!</string> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Resource key branding mismatch: The key connect_register_success_message should use the personalid_ prefix (e.g. personalid_register_success_message) to match updated branding.
🤖 Prompt for AI Agents
In app/res/values-fr/strings.xml at line 273, the resource key
connect_register_success_message does not follow the updated branding prefix
convention. Rename the key to personalid_register_success_message to align with
the personalid_ prefix standard used for branding consistency.
| <string name="connect_recovery_success_message">Vous avez récupéré votre compte avec succès et pouvez désormais reprendre l\'utilisation de votre PersonalId.</string> | ||
| <string name="connect_unlock_title">Déverrouiller PersonalId</string> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Resource key branding mismatch: Both connect_recovery_success_message and connect_unlock_title retain the “connect” prefix despite referring to PersonalId flows. Rename them (e.g. to personalid_recovery_success_message and personalid_unlock_title).
🤖 Prompt for AI Agents
In app/res/values-fr/strings.xml at lines 265 to 266, the resource keys use the
prefix "connect" which is inconsistent with the PersonalId branding. Rename the
keys from "connect_recovery_success_message" to
"personalid_recovery_success_message" and from "connect_unlock_title" to
"personalid_unlock_title" to align with the PersonalId naming convention.
| <string name="app_select_message">Sélectionnez une application:</string> | ||
| <string name="connect_id_enabled">ConnectID activé</string> | ||
| <string name="login_password_by_connect">Via ConnectID (ou appuyez ici)</string> | ||
| <string name="login_password_by_connect">Via PersonalId (ou appuyez ici)</string> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Resource key branding mismatch: The string value now references “PersonalId”, but the key login_password_by_connect still uses “connect”. Rename the key (e.g. to login_password_by_personalid) to keep resource names aligned with the updated branding.
🤖 Prompt for AI Agents
In app/res/values-fr/strings.xml at line 245, the resource key
`login_password_by_connect` does not match the updated branding in the string
value which references "PersonalId". Rename the key to
`login_password_by_personalid` to align the resource name with the new branding.
| <string name="login_button_connectid">Connectez-vous avec PersonalId</string> | ||
| <string name="login_link_connectid_title">Lien vers PersonalId ?</string> | ||
| <string name="login_link_connectid_message">Souhaitez-vous associer cet identifiant à votre compte PersonalId? Vous n\'aurez plus besoin de saisir votre mot de passe à l\'avenir.</string> | ||
| <string name="login_unlink_connectid_title">Dissocier PersonalId?</string> | ||
| <string name="login_unlink_connectid_message">Je vois que vous vous êtes connecté avec votre mot de passe, bien que PersonalId ait été configuré pour la connexion automatique. Souhaitez-vous dissocier cette connexion de PersonalId?</string> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Resource key branding mismatch: Multiple keys (login_button_connectid, login_link_connectid_title, login_link_connectid_message, login_unlink_connectid_title, login_unlink_connectid_message) still reference “connectid” but their values use “PersonalId.” Rename these keys (e.g. login_button_personalid, login_link_personalid_title, etc.) to avoid confusion.
🤖 Prompt for AI Agents
In app/res/values-fr/strings.xml around lines 347 to 351, the resource keys use
"connectid" while their string values use "PersonalId," causing a branding
mismatch. Rename all keys from "connectid" to "personalid" to align with the
branding in the values, for example, change "login_button_connectid" to
"login_button_personalid" and similarly for the other keys.
| <string name="personalid_login_via_connect">Prêt à vous connecter !\n</string> | ||
| <string name="personalid_failed_to_login_with_connectid">Nous rencontrons une erreur irrécupérable lors de la connexion avec votre identifiant personnel. Veuillez vous connecter avec votre mot de passe ou restaurer votre identifiant personnel.</string> | ||
| <string name="personalid_name_fragment_subtitle">Veuillez saisir votre nom tel qu\'il apparaît sur votre pièce d\'identité émise par le gouvernement</string> | ||
| <string name="personalid_name_empty_error">Le champ Nom ne peut pas être vide</string> | ||
| <string name="personalid_configuration_process_failed_title">Échec du processus</string> | ||
| <string name="personalid_configuration_process_failed_subtitle">Votre appareil n\'est pas compatible avec PersonalID pour le moment. Veuillez réessayer sur un autre appareil.</string> | ||
| <string name="personalid_configuration_process_failed_security_subtitle">Votre appareil n\'est pas compatible avec PersonalID pour le moment en raison de l\'indisponibilité de la fonctionnalité de sécurité. Veuillez réessayer sur un autre appareil.</string> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Key naming inconsistency: The key personalid_failed_to_login_with_connectid mixes both personalid and connectid. Rename it to something like personalid_failed_to_login to avoid confusion.
🤖 Prompt for AI Agents
In app/res/values-fr/strings.xml around lines 437 to 443, the key
personalid_failed_to_login_with_connectid mixes personalid and connectid,
causing inconsistency. Rename this key to personalid_failed_to_login to maintain
consistent naming conventions and avoid confusion.
https://dimagi.atlassian.net/browse/QA-7849
Product Description
Fixes an issue that could occur after failed PersonalID API calls.
Technical Summary
Fixed a corner-case where a PersonalID API call fails and returns an error body, and the mobile code would erroneously call the processFailure callback twice.
Feature Flag
PersonalID
Safety Assurance
Safety story
Tested locally. Crashed before on failed device configuration (i.e. bad integrity token), now it doesn't
Automated test coverage
None
QA Plan
Already tested (bug identified by QA)