-
-
Notifications
You must be signed in to change notification settings - Fork 45
Confirm backup code api call #3133
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
📝 WalkthroughWalkthroughThis change refactors the backup code confirmation flow in the CommCare Connect application. The method for confirming a backup code is renamed and its parameters are updated to use token-based authentication instead of phone and secret. A new response parser class is introduced to handle the API response, extracting relevant session data. The API endpoint constant and service interface methods are renamed for clarity and to reflect their new purpose. The fragment responsible for user interaction now uses a higher-level API handler abstraction, simplifying error handling and response parsing. Sequence Diagram(s)sequenceDiagram
participant User
participant PersonalIdBackupCodeFragment
participant PersonalIdApiHandler
participant ApiPersonalId
participant ApiService
participant Server
User->>PersonalIdBackupCodeFragment: Enter backup code
PersonalIdBackupCodeFragment->>PersonalIdApiHandler: confirmBackupCode(activity, backupCode, sessionData)
PersonalIdApiHandler->>ApiPersonalId: confirmBackupCode(context, pin, token, callback)
ApiPersonalId->>ApiService: confirmPin("Authorization: Bearer <token>", {recovery_pin: pin})
ApiService->>Server: POST /users/set_recovery_pin
Server-->>ApiService: JSON response
ApiService-->>ApiPersonalId: ResponseBody
ApiPersonalId-->>PersonalIdApiHandler: callback.onSuccess/onFailure
PersonalIdApiHandler-->>PersonalIdBackupCodeFragment: onSuccess/onFailure
PersonalIdBackupCodeFragment-->>User: Display result
Suggested reviewers
✨ Finishing Touches
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 (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
🧹 Nitpick comments (1)
app/src/org/commcare/connect/network/ApiPersonalId.java (1)
262-263: Fix formatting and add parameter validation.There are two issues with the method signature:
- Formatting: Missing space between parameters
pin,String tokenshould bepin, String token- Parameter validation: The
tokenparameter should be validated to prevent potential issuesApply this diff to fix the formatting and add validation:
- public static void confirmBackupCode(Context context, - String pin,String token ,IApiCallback callback) { + public static void confirmBackupCode(Context context, + String pin, String token, IApiCallback callback) { + if (token == null || token.trim().isEmpty()) { + callback.processFailure(400); // or appropriate error handling + return; + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
app/src/org/commcare/connect/network/ApiPersonalId.java(1 hunks)app/src/org/commcare/connect/network/PersonalIdApiHandler.java(2 hunks)app/src/org/commcare/connect/network/connectId/ApiEndPoints.java(1 hunks)app/src/org/commcare/connect/network/connectId/ApiService.java(1 hunks)app/src/org/commcare/connect/network/parser/ConfirmBackupCodeResponseParser.java(1 hunks)app/src/org/commcare/fragments/personalId/PersonalIdBackupCodeFragment.java(4 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (3)
app/src/org/commcare/connect/network/parser/ConfirmBackupCodeResponseParser.java (1)
app/src/org/commcare/utils/JsonExtensions.kt (1)
optStringSafe(7-10)
app/src/org/commcare/connect/network/connectId/ApiService.java (1)
app/src/org/commcare/connect/network/connectId/ApiEndPoints.java (1)
ApiEndPoints(3-36)
app/src/org/commcare/connect/network/ApiPersonalId.java (3)
app/src/org/commcare/connect/workers/ConnectHeartbeatWorker.kt (1)
context(12-28)app/src/org/commcare/network/HttpUtils.java (1)
HttpUtils(27-100)app/src/org/commcare/connect/network/connectId/ApiClient.java (1)
ApiClient(16-63)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Lint Code Base
🔇 Additional comments (9)
app/src/org/commcare/connect/network/connectId/ApiEndPoints.java (1)
23-23: LGTM! Good refactoring for clarity.Renaming the constant from
setPINtosetBackupCodeimproves code readability and aligns with the backup code terminology being adopted throughout the codebase, while maintaining the actual endpoint URL.app/src/org/commcare/connect/network/PersonalIdApiHandler.java (2)
7-7: LGTM! Proper import for the new parser.The import for
ConfirmBackupCodeResponseParseris correctly added to support the new backup code confirmation functionality.
96-101: LGTM! Well-structured API handler method.The
confirmBackupCodemethod follows the established pattern in this class:
- Properly extracts the session token from
sessionData- Uses the appropriate parser (
ConfirmBackupCodeResponseParser)- Handles errors consistently with the defined error code
- Integrates seamlessly with the existing callback infrastructure
app/src/org/commcare/connect/network/parser/ConfirmBackupCodeResponseParser.java (1)
1-26: LGTM! Excellent parser implementation.The
ConfirmBackupCodeResponseParserclass is well-implemented with several strengths:
- Safe JSON extraction: Uses
JsonExtensions.optStringSafeto handle null/missing values gracefully- Proper error handling: Uses
optBooleanwith sensible default for the boolean field- Clear documentation: Well-documented class and method purposes
- Consistent pattern: Follows the established parser interface pattern
The parser correctly extracts all expected fields (
username,db_key,account_orphaned,password) from the API response and populates the session data appropriately.app/src/org/commcare/connect/network/ApiPersonalId.java (2)
268-270: LGTM! Proper token-based authentication implementation.The authentication mechanism has been correctly updated:
- Uses
AuthInfo.TokenAuthfor secure token-based authentication- Properly generates the authorization header using
HttpUtils.getCredential- Improves security by replacing phone/secret authentication
This change aligns well with modern authentication best practices.
264-274:Details
✅ Verification successful
Verify the service method name consistency.
The method
confirmBackupCodecallsapiService.confirmPin(tokenAuth, params)on line 272. While this might be intentional if the service interface uses different naming, it could be confusing for maintainability.Please verify that the service method naming is consistent across the codebase:
🏁 Script executed:
#!/bin/bash # Description: Check for consistency between method names in ApiService interface # Expected: Either all methods use 'confirmPin' or they should be updated to 'confirmBackupCode' # Search for confirmPin method definition in ApiService rg -A 3 "confirmPin.*\(" --type java # Search for any other backup code related method names rg -A 3 "(confirmBackupCode|confirm.*backup|backup.*confirm)" --type javaLength of output: 3894
Service method naming is consistent
The
ApiServiceinterface defines a singleconfirmPin(...)method (mapped tosetBackupCode), and all calls to confirm a backup code go through theconfirmBackupCodewrapper inApiPersonalId. There is no conflicting or missing method inApiService, and the wrapper naming aligns with other handlers. No changes required.app/src/org/commcare/connect/network/connectId/ApiService.java (1)
52-53: Good naming convention improvement.The method name change from
confirmPINtoconfirmPinfollows proper camelCase convention, and renaming the parameter fromconfirmPINRequesttoconfirmBackupRequestis more descriptive and aligns with the backup code confirmation functionality.app/src/org/commcare/fragments/personalId/PersonalIdBackupCodeFragment.java (2)
177-177: Good method name improvement.The method name change from
confirmBackupPin()toconfirmBackupCode()better reflects the functionality and aligns with the backup code terminology used throughout the codebase.
239-241: Good centralization of error handling.The new
navigateFailure()method properly delegates error handling to the centralizedPersonalIdApiErrorHandler, which improves maintainability and consistency across the application.
| @Body Map<String, String> confirmBackupRequest); | ||
|
|
||
| @POST(ApiEndPoints.setPIN) | ||
| @POST(ApiEndPoints.setBackupCode) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
Verify endpoint mapping aligns with method purpose.
The endpoint annotation was changed from @POST(ApiEndPoints.setPIN) to @POST(ApiEndPoints.setBackupCode). Based on the relevant code snippets, setBackupCode maps to "/users/set_recovery_pin" which appears correct for setting a backup code. However, ensure this change aligns with the actual backend API expectations.
🏁 Script executed:
#!/bin/bash
# Description: Verify the endpoint constants and their usage across the codebase
# Expected: Confirm setBackupCode endpoint is used consistently for backup code operations
# Search for setBackupCode endpoint usage
rg -A 2 -B 2 "setBackupCode"
# Search for setPIN endpoint usage to see if it's still referenced elsewhere
rg -A 2 -B 2 "setPIN"Length of output: 2477
Align APIService method signature with backup code endpoint
In app/src/org/commcare/connect/network/connectId/ApiService.java (at the @POST(ApiEndPoints.setBackupCode) annotation), the method and its parameter are still named for PIN changes, which is misleading. Rename them to reflect the backup-code operation:
Suggested diff:
- @POST(ApiEndPoints.setBackupCode)
- Call<ResponseBody> changePIN(@Header("Authorization") String token,
- @Body Map<String, String> changePINRequest);
+ @POST(ApiEndPoints.setBackupCode)
+ Call<ResponseBody> setBackupCode(@Header("Authorization") String token,
+ @Body Map<String, String> backupCodeRequest);This keeps the endpoint mapping clear and consistent with its purpose.
📝 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.
| @POST(ApiEndPoints.setBackupCode) | |
| @POST(ApiEndPoints.setBackupCode) | |
| Call<ResponseBody> setBackupCode(@Header("Authorization") String token, | |
| @Body Map<String, String> backupCodeRequest); |
🤖 Prompt for AI Agents
In app/src/org/commcare/connect/network/connectId/ApiService.java at line 55,
the method annotated with @POST(ApiEndPoints.setBackupCode) still uses names
related to PIN changes, which is misleading. Rename the method and its
parameters to clearly indicate they handle backup code operations, ensuring the
method signature aligns with the setBackupCode endpoint and reflects its purpose
accurately.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is the format Is used keep the function name as same as end point. of the api... here it is users/set_recovery_pin
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@pm-dimagi Thank you for clarifying your naming convention! I understand now that you maintain consistency between the API service method names and the actual endpoint names. Since the endpoint is users/set_recovery_pin, keeping the method name aligned with that endpoint structure makes perfect sense for maintainability and creates a clear mapping between the code and API specification.
I'll keep this convention in mind for future reviews.
ヘ(^◡^)ノ
✏️ Learnings added
Learnt from: pm-dimagi
PR: dimagi/commcare-android#3133
File: app/src/org/commcare/connect/network/connectId/ApiService.java:55-55
Timestamp: 2025-05-28T11:30:37.986Z
Learning: In the CommCare Android codebase, API service method names in ApiService.java should match the format of the actual API endpoint names rather than using semantically meaningful names. For example, if the endpoint is "users/set_recovery_pin", the method name should follow that endpoint structure for consistency and maintainability.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@pm-dimagi Thank you for the commit! I can see you've addressed the method naming to align with your established convention of matching API endpoint structure. This maintains consistency in your codebase.
🧠 Learnings used
Learnt from: pm-dimagi
PR: dimagi/commcare-android#3133
File: app/src/org/commcare/connect/network/connectId/ApiService.java:55-55
Timestamp: 2025-05-28T11:30:37.986Z
Learning: In the CommCare Android codebase, API service method names in ApiService.java should match the format of the actual API endpoint names rather than using semantically meaningful names. For example, if the endpoint is "users/set_recovery_pin", the method name should follow that endpoint structure for consistency and maintainability.
If you found this review helpful, would you consider giving us a shout-out on X?
Thank you for using CodeRabbit!
app/src/org/commcare/fragments/personalId/PersonalIdBackupCodeFragment.java
Show resolved
Hide resolved
| private void confirmBackupCode() { | ||
| String backupCode = binding.connectPinInput.getText().toString(); | ||
|
|
||
| ApiPersonalId.checkPin(getActivity(), name, secret, pin, new IApiCallback() { | ||
| new PersonalIdApiHandler() { | ||
| @Override | ||
| public void processSuccess(int responseCode, InputStream responseData) { | ||
| try { | ||
| JSONObject json = new JSONObject(new String(StreamsUtil.inputStreamToByteArray(responseData))); | ||
| handleSuccessfulRecovery(json, pin); | ||
| } catch (IOException | JSONException e) { | ||
| Logger.exception("Error parsing recovery response", e); | ||
| showRecoveryFailure(); | ||
| } | ||
| protected void onSuccess(PersonalIdSessionData sessionData) { | ||
| } | ||
|
|
||
| @Override | ||
| public void processFailure(int responseCode) { | ||
| handleFailedPinAttempt(); | ||
| protected void onFailure(PersonalIdApiErrorCodes failureCode) { | ||
| navigateFailure(failureCode); | ||
| } | ||
| }.confirmBackupCode( | ||
| requireActivity(), | ||
| backupCode, | ||
| personalIdSessionDataViewModel.getPersonalIdSessionData()); | ||
|
|
||
| @Override | ||
| public void processNetworkFailure() { | ||
| ConnectNetworkHelper.showNetworkError(getActivity()); | ||
| } | ||
|
|
||
| @Override | ||
| public void processTokenUnavailableError() { | ||
| ConnectNetworkHelper.handleTokenUnavailableException(requireActivity()); | ||
| } | ||
|
|
||
| @Override | ||
| public void processTokenRequestDeniedError() { | ||
| ConnectNetworkHelper.handleTokenDeniedException(requireActivity()); | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
Excellent architectural improvement, but verify missing functionality.
The refactoring to use PersonalIdApiHandler improves separation of concerns and centralizes error handling. However, there are concerns about missing functionality:
- The
onSuccesscallback is empty (line 225-226), but the old implementation had significant logic for handling successful recovery including JSON parsing and navigation. - The old implementation called
handleSuccessfulRecovery()which performed database operations and user management.
Please verify that the missing success handling logic is now handled elsewhere, such as within the PersonalIdApiHandler.confirmBackupCode() method itself. Run the following script to check:
🏁 Script executed:
#!/bin/bash
# Description: Search for the missing success handling logic in PersonalIdApiHandler
# Expected: Find where handleSuccessfulRecovery or similar logic is now implemented
# Search for PersonalIdApiHandler implementation
ast-grep --pattern 'class PersonalIdApiHandler {
$$$
confirmBackupCode($$$) {
$$$
}
$$$
}'
# Search for handleSuccessfulRecovery logic
rg -A 10 "handleSuccessfulRecovery\|username.*json\|ConnectConstants.CONNECT_KEY_USERNAME"Length of output: 197
🏁 Script executed:
#!/bin/bash
set -e
echo "1. Locating PersonalIdApiHandler file(s):"
rg -l "class PersonalIdApiHandler"
echo
echo "2. Displaying class definition and surrounding context:"
file_path=$(rg -l "class PersonalIdApiHandler")
rg -n "class PersonalIdApiHandler" -A 50 "$file_path"
echo
echo "3. Inspecting confirmBackupCode(...) implementation in that file:"
rg -n "confirmBackupCode" -A 50 "$file_path"
echo
echo "4. Searching for legacy handleSuccessfulRecovery references:"
rg -n "handleSuccessfulRecovery" -A 10Length of output: 5144
🏁 Script executed:
#!/bin/bash
set -e
# 1. Locate ConfirmBackupCodeResponseParser file
echo "Locating ConfirmBackupCodeResponseParser implementation..."
parser_file=$(rg -l "class ConfirmBackupCodeResponseParser" -g "app/src/**/*.java")
echo "Found: $parser_file"
# 2. Display the full class for analysis
echo
echo "Displaying ConfirmBackupCodeResponseParser contents:"
rg -n "class ConfirmBackupCodeResponseParser" -A 200 "$parser_file"Length of output: 1544
Missing success handling in confirmBackupCode callback
The refactor now parses the response into PersonalIdSessionData via ConfirmBackupCodeResponseParser, but never invokes the legacy recovery logic (DB passphrase, user record setup, PIN reset, navigation). You need to re-introduce that workflow in the anonymous onSuccess(...) override.
Locations to update:
app/src/org/commcare/fragments/personalId/PersonalIdBackupCodeFragment.java, insideconfirmBackupCode()around lines 220–237.
Suggested diff:
new PersonalIdApiHandler() {
- @Override
- protected void onSuccess(PersonalIdSessionData sessionData) {
- }
+ @Override
+ protected void onSuccess(PersonalIdSessionData sessionData) {
+ // restore DB passphrase
+ ConnectDatabaseHelper.handleReceivedDbPassphrase(
+ requireActivity(), sessionData.getDbKey());
+
+ // recreate user and reset PIN
+ ConnectUserRecord user = new ConnectUserRecord(
+ sessionData.getName(),
+ sessionData.getUsername(),
+ "", sessionData.getName(), "");
+ user.setPin(backupCode);
+ user.setLastPinDate(new Date());
+ resetUserPassword(user);
+
+ // proceed to next screen
+ navigateNext();
+ }📝 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.
| private void confirmBackupCode() { | |
| String backupCode = binding.connectPinInput.getText().toString(); | |
| ApiPersonalId.checkPin(getActivity(), name, secret, pin, new IApiCallback() { | |
| new PersonalIdApiHandler() { | |
| @Override | |
| public void processSuccess(int responseCode, InputStream responseData) { | |
| try { | |
| JSONObject json = new JSONObject(new String(StreamsUtil.inputStreamToByteArray(responseData))); | |
| handleSuccessfulRecovery(json, pin); | |
| } catch (IOException | JSONException e) { | |
| Logger.exception("Error parsing recovery response", e); | |
| showRecoveryFailure(); | |
| } | |
| protected void onSuccess(PersonalIdSessionData sessionData) { | |
| } | |
| @Override | |
| public void processFailure(int responseCode) { | |
| handleFailedPinAttempt(); | |
| protected void onFailure(PersonalIdApiErrorCodes failureCode) { | |
| navigateFailure(failureCode); | |
| } | |
| }.confirmBackupCode( | |
| requireActivity(), | |
| backupCode, | |
| personalIdSessionDataViewModel.getPersonalIdSessionData()); | |
| @Override | |
| public void processNetworkFailure() { | |
| ConnectNetworkHelper.showNetworkError(getActivity()); | |
| } | |
| @Override | |
| public void processTokenUnavailableError() { | |
| ConnectNetworkHelper.handleTokenUnavailableException(requireActivity()); | |
| } | |
| @Override | |
| public void processTokenRequestDeniedError() { | |
| ConnectNetworkHelper.handleTokenDeniedException(requireActivity()); | |
| } | |
| } | |
| private void confirmBackupCode() { | |
| String backupCode = binding.connectPinInput.getText().toString(); | |
| new PersonalIdApiHandler() { | |
| @Override | |
| protected void onSuccess(PersonalIdSessionData sessionData) { | |
| // restore DB passphrase | |
| ConnectDatabaseHelper.handleReceivedDbPassphrase( | |
| requireActivity(), sessionData.getDbKey()); | |
| // recreate user and reset PIN | |
| ConnectUserRecord user = new ConnectUserRecord( | |
| sessionData.getName(), | |
| sessionData.getUsername(), | |
| "", sessionData.getName(), ""); | |
| user.setPin(backupCode); | |
| user.setLastPinDate(new Date()); | |
| resetUserPassword(user); | |
| // proceed to next screen | |
| navigateNext(); | |
| } | |
| @Override | |
| protected void onFailure(PersonalIdApiErrorCodes failureCode) { | |
| navigateFailure(failureCode); | |
| } | |
| }.confirmBackupCode( | |
| requireActivity(), | |
| backupCode, | |
| personalIdSessionDataViewModel.getPersonalIdSessionData()); | |
| } |
🤖 Prompt for AI Agents
In app/src/org/commcare/fragments/personalId/PersonalIdBackupCodeFragment.java
around lines 220 to 237, the onSuccess callback of
PersonalIdApiHandler.confirmBackupCode is currently empty, missing the legacy
success handling logic that manages database passphrase setup, user record
initialization, PIN reset, and navigation. To fix this, reintroduce the legacy
recovery workflow by invoking the appropriate methods or logic inside the
onSuccess override, ensuring the successful recovery process is fully handled as
before.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@shubham1g5 Thanks for confirming! The missing success handling logic definitely needs to be restored to maintain the complete recovery workflow.
If you'd like me to help implement this fix or need assistance with any other aspects of the backup code confirmation functionality, just let me know!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this will be done in the next subsequent pr just to keep the logic separate i will create anothe rpr for that @shubham1g5
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
app/src/org/commcare/connect/network/parser/ConfirmBackupCodeResponseParser.java
Show resolved
Hide resolved
app/src/org/commcare/fragments/personalId/PersonalIdBackupCodeFragment.java
Show resolved
Hide resolved
| private void confirmBackupCode() { | ||
| String backupCode = binding.connectPinInput.getText().toString(); | ||
|
|
||
| ApiPersonalId.checkPin(getActivity(), name, secret, pin, new IApiCallback() { | ||
| new PersonalIdApiHandler() { | ||
| @Override | ||
| public void processSuccess(int responseCode, InputStream responseData) { | ||
| try { | ||
| JSONObject json = new JSONObject(new String(StreamsUtil.inputStreamToByteArray(responseData))); | ||
| handleSuccessfulRecovery(json, pin); | ||
| } catch (IOException | JSONException e) { | ||
| Logger.exception("Error parsing recovery response", e); | ||
| showRecoveryFailure(); | ||
| } | ||
| protected void onSuccess(PersonalIdSessionData sessionData) { | ||
| } | ||
|
|
||
| @Override | ||
| public void processFailure(int responseCode) { | ||
| handleFailedPinAttempt(); | ||
| protected void onFailure(PersonalIdApiErrorCodes failureCode) { | ||
| navigateFailure(failureCode); | ||
| } | ||
| }.confirmBackupCode( | ||
| requireActivity(), | ||
| backupCode, | ||
| personalIdSessionDataViewModel.getPersonalIdSessionData()); | ||
|
|
||
| @Override | ||
| public void processNetworkFailure() { | ||
| ConnectNetworkHelper.showNetworkError(getActivity()); | ||
| } | ||
|
|
||
| @Override | ||
| public void processTokenUnavailableError() { | ||
| ConnectNetworkHelper.handleTokenUnavailableException(requireActivity()); | ||
| } | ||
|
|
||
| @Override | ||
| public void processTokenRequestDeniedError() { | ||
| ConnectNetworkHelper.handleTokenDeniedException(requireActivity()); | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1
User photo and name
# Conflicts: # app/src/org/commcare/fragments/personalId/PersonalIdBackupCodeFragment.java
| clearPinFields(); | ||
|
|
||
| clearBackupCodeFields(); | ||
| personalIdSessionDataViewModel = new ViewModelProvider(this).get(PersonalIdSessionDataViewModel.class); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@pm-dimagi This has wrong owner, it should be activity and not fragment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| private static final String KEY_NAME = "name"; | ||
| private static final String KEY_RECOVERY = "is_recovery"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@pm-dimagi What is the reason that we are saving the name and is_recovery in saved instance?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is the old flow code we will refactor this in the later pr
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@pm-dimagi Ok. This is effecting the initArguments method so I think we should remove it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes sure will remove it in next pr which is about the same things
| user.setPin(pin); | ||
| user.setPin(backupCode); | ||
| user.setLastPinDate(new Date()); | ||
| resetUserPassword(user); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@pm-dimagi Just asking: Are we allowing to reset the password? Do we have password thing in connect id?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is about the user password we create internally now i will confirm this dave that is needed now or not in new flow... but this is not about the user password which user sets manully earlier
| ConnectUserRecord user = ConnectUserDatabaseUtil.getUser(getActivity()); | ||
|
|
||
| ApiPersonalId.setBackupCode(getActivity(), user.getUserId(), user.getPassword(), pin, new IApiCallback() { | ||
| ApiPersonalId.setBackupCode(getActivity(), user.getUserId(), user.getPassword(), backupCode, new IApiCallback() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@pm-dimagi When are we setting user and password during registration flow?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same as above creply
| if (commaIndex != -1) { | ||
| base64Image = base64Image.substring(commaIndex + 1); | ||
| } else { | ||
| throw (new IllegalStateException("Not a valid base64 string")); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@pm-dimagi IllegalArgumentException instead of IllegalStateException?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
shubham has asked for illegalStateException
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agree it should be IllegalArgumentException
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Product Description
This pr is just about to make api call for the confirm backup code by the new syntax we are following for the api call
Note: navigation flow, photo and set backupcode will be in separate pr
Technical Summary
Feature Flag
Safety Assurance
Safety story
Automated test coverage
QA Plan
Labels and Review