Complete profile Api changes as per ccct-1275 #3122
Conversation
📝 WalkthroughWalkthroughThe changes refactor how API service instances are obtained across the codebase, shifting from direct Retrofit client usage to accessing a singleton Sequence Diagram(s)sequenceDiagram
participant Fragment as PersonalIdPhotoCaptureFragment
participant ApiPersonalId
participant ApiClient
participant ApiService
participant Server
Fragment->>ApiPersonalId: setPhotoAndCompleteProfile(userId, password, userName, photoAsBase64, pin, callback)
ApiPersonalId->>ApiClient: getClientApi()
ApiClient-->>ApiPersonalId: ApiService instance
ApiPersonalId->>ApiService: completeProfile(authToken, body)
ApiService->>Server: POST /users/complete_profile (with recovery_pin)
Server-->>ApiService: Response
ApiService-->>ApiPersonalId: Response
ApiPersonalId-->>Fragment: callback.onSuccess/onFailure
Possibly related PRs
Suggested labels
Suggested reviewers
Note ⚡️ AI Code Reviews for VS Code, Cursor, WindsurfCodeRabbit now has a plugin for VS Code, Cursor and Windsurf. This brings AI code reviews directly in the code editor. Each commit is reviewed immediately, finding bugs before the PR is raised. Seamless context handoff to your AI code agent ensures that you can easily incorporate review feedback. Note ⚡️ Faster reviews with cachingCodeRabbit now supports caching for code and dependencies, helping speed up reviews. This means quicker feedback, reduced wait times, and a smoother review experience overall. Cached data is encrypted and stored securely. This feature will be automatically enabled for all accounts on May 30th. To opt out, configure ✨ 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 (
|
![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
🧹 Nitpick comments (2)
app/src/org/commcare/fragments/personalId/PersonalIdPhotoCaptureFragment.java (1)
118-120: Fix spacing in parameter listThe spacing around the PIN parameter is inconsistent. Also, the method is now correctly using the updated API endpoint which requires the PIN parameter.
- ApiPersonalId.setPhotoAndCompleteProfile(requireContext(), connectUserRecord.getUserId(), connectUserRecord.getPassword(), - connectUserRecord.getName(), photoAsBase64,connectUserRecord.getPin() ,networkResponseCallback); + ApiPersonalId.setPhotoAndCompleteProfile(requireContext(), connectUserRecord.getUserId(), connectUserRecord.getPassword(), + connectUserRecord.getName(), photoAsBase64, connectUserRecord.getPin(), networkResponseCallback);app/src/org/commcare/connect/network/ApiPersonalId.java (1)
320-336: Consider adding validation for the PIN parameter.The method accepts a PIN parameter but doesn't validate it before sending to the API. Consider adding validation to ensure it meets any format or length requirements specified by the backend.
public static void setPhotoAndCompleteProfile(Context context, String userId, String password, String userName, String photoAsBase64, String pin, IApiCallback callback) { Objects.requireNonNull(photoAsBase64); Objects.requireNonNull(userName); + Objects.requireNonNull(pin); + + // Consider adding PIN format validation + // if (!isValidPin(pin)) { + // callback.processFailure(400); + // return; + // } AuthInfo authInfo = new AuthInfo.ProvidedAuth(userId, password, false); String token = HttpUtils.getCredential(authInfo); Objects.requireNonNull(token); HashMap<String, String> params = new HashMap<>(); params.put("photo", photoAsBase64); params.put("name", userName); params.put("recovery_pin", pin); ApiService apiService = ApiClient.getClientApi(); Call<ResponseBody> call = apiService.completeProfile(token, params); callApi(context, call, callback); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
app/src/org/commcare/connect/network/ApiPersonalId.java(11 hunks)app/src/org/commcare/connect/network/connectId/ApiClient.java(1 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/fragments/personalId/PersonalIdPhotoCaptureFragment.java(3 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
app/src/org/commcare/connect/network/connectId/ApiService.java (1)
app/src/org/commcare/connect/network/connectId/ApiEndPoints.java (1)
ApiEndPoints(3-35)
app/src/org/commcare/fragments/personalId/PersonalIdPhotoCaptureFragment.java (1)
app/src/org/commcare/connect/network/ApiPersonalId.java (1)
ApiPersonalId(47-431)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Lint Code Base
🔇 Additional comments (7)
app/src/org/commcare/connect/network/connectId/ApiClient.java (2)
19-19: Good singleton pattern implementationChanged from holding a Retrofit instance to directly holding the ApiService interface instance. This is a better design that encapsulates the service creation logic.
24-33: Good refactoring to expose the API service directlyRenamed from
getClient()togetClientApi()with appropriate return type change. This improvement:
- Prevents redundant service creation across the codebase
- Ensures a consistent singleton instance is used
- Simplifies client code by removing the need for
.create(ApiService.class)callsThe double-checked locking pattern is correctly maintained for thread safety.
app/src/org/commcare/fragments/personalId/PersonalIdPhotoCaptureFragment.java (2)
75-75: Method name update aligns with new API endpointRenamed from
uploadImage()touploadImageAndCompleteProfile()to reflect the updated endpoint functionality.
85-85: Method implementation name correctly updatedMethod declaration renamed to match the caller in line 75.
app/src/org/commcare/connect/network/connectId/ApiService.java (1)
23-25: API method updated to reflect new endpoint nameRenamed from
setProfiletocompleteProfileto align with the endpoint rename in ApiEndPoints.java. This update is part of the changes needed for the profile completion flow that now requires a PIN parameter.app/src/org/commcare/connect/network/ApiPersonalId.java (2)
256-256: Good refactoring to use singleton ApiService.The code consistently switches from manually creating ApiService instances to using the singleton approach via
ApiClient.getClientApi(). This promotes better resource management and maintainability by centralizing the API client initialization.Also applies to: 269-269, 283-283, 296-296, 315-315, 333-333, 343-343, 351-351, 363-363, 374-374, 383-383, 395-395
320-321:Details
✅ Verification successful
Method signature updated to include PIN for profile completion.
The method has been renamed from
setPhotoAndNametosetPhotoAndCompleteProfilewith an additionalpinparameter. The implementation properly passes this new parameter to the API endpoint, which has also been renamed fromsetProfiletocompleteProfileto reflect the semantic change.Let's verify that all callers of this method have been updated to include the new PIN parameter:
Also applies to: 331-331, 334-334
🏁 Script executed:
#!/bin/bash # Find all references to the setPhotoAndCompleteProfile method rg -A 3 "setPhotoAndCompleteProfile" --type javaLength of output: 1152
Callers updated to include PIN parameter
Verified that all invocations of
setPhotoAndCompleteProfilenow passconnectUserRecord.getPin(), e.g. in:
app/src/org/commcare/fragments/personalId/PersonalIdPhotoCaptureFragment.javaNo further changes required.
| public static final String changePhoneNo = "/users/change_phone"; | ||
| public static final String updateProfile = "/users/update_profile"; | ||
| public static final String setProfile = "/users/set_profile"; | ||
| public static final String completeProfile = "/users/complete_profile "; |
There was a problem hiding this comment.
Fix trailing space in URL endpoint
There's an unintended trailing space in the URL string that could cause API failures.
- public static final String completeProfile = "/users/complete_profile ";
+ public static final String completeProfile = "/users/complete_profile";🤖 Prompt for AI Agents
In app/src/org/commcare/connect/network/connectId/ApiEndPoints.java at line 11,
remove the trailing space at the end of the completeProfile URL string to ensure
the endpoint is correct and prevent API call failures.
|
|
||
| public static Retrofit getClient() { | ||
| if (retrofit == null) { | ||
| public static ApiService getClientApi() { |
There was a problem hiding this comment.
yay, thanks for getting this in.
Technical Summary
https://dimagi.atlassian.net/browse/CCCT-1275
Feature Flag
Improvements
Labels and Review