Minor changes, lint from connect branch #3068
Conversation
📝 WalkthroughWalkthroughThis set of changes primarily focuses on code cleanup, refactoring, and minor improvements across multiple files in the codebase. Several files have had whitespace, indentation, or comment formatting adjusted, with no impact on logic or control flow. Some unused import statements were removed for clarity. A few new utility methods were introduced, such as in Sequence Diagram(s)sequenceDiagram
participant User
participant App
participant ResourceInstallUtils
participant PromptApkUpdateActivity
participant TargetMismatchErrorActivity
User->>App: Triggers app installation/update
App->>ResourceInstallUtils: showApkUpdatePrompt(context, versionRequired, versionAvailable)
ResourceInstallUtils->>PromptApkUpdateActivity: Start activity with extras (version info, title)
User->>App: Triggers target mismatch error
App->>ResourceInstallUtils: showTargetMismatchError(context)
ResourceInstallUtils->>TargetMismatchErrorActivity: Start activity
sequenceDiagram
participant CrashUtil
participant ReportingUtils
participant ConnectIDManager
CrashUtil->>ReportingUtils: getUserForCrashes()
alt User available via getUser()
ReportingUtils-->>CrashUtil: user id
else ConnectIDManager logged in
ReportingUtils->>ConnectIDManager: getUserId()
ConnectIDManager-->>ReportingUtils: connect user id
ReportingUtils-->>CrashUtil: connect user id
else
ReportingUtils-->>CrashUtil: null
end
CrashUtil->>Crashlytics: set user id (if not null)
Possibly related PRs
Suggested labels
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 (
|
![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/sync/FirebaseMessagingDataSyncer.java (1)
197-197: Minor code style improvement.Adding this blank line improves readability by logically separating the intent setup from the broadcast action.
app/src/org/commcare/utils/CrashUtil.java (1)
49-52: Improved user identification for crash reporting.The implementation now uses
ReportingUtils.getUserForCrashes()which provides more robust user identification by:
- First trying to get the username from the current session
- Falling back to the ConnectID user if the session user isn't available
- Adding a null check before setting the user ID to prevent potential errors
This unified approach eliminates the need for separate Connect-specific crash registration methods.
Consider adding a comment explaining the fallback behavior since it's not immediately obvious from the method name alone:
public static void registerUserData() { if (crashlyticsEnabled) { + // getUserForCrashes tries session user first, falls back to ConnectID user String user = ReportingUtils.getUserForCrashes(); if(user != null) { FirebaseCrashlytics.getInstance().setUserId(user); } } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (29)
app/src/org/commcare/activities/CommCareSetupActivity.java(7 hunks)app/src/org/commcare/activities/CommCareVerificationActivity.java(4 hunks)app/src/org/commcare/activities/DispatchActivity.java(1 hunks)app/src/org/commcare/activities/FormEntryActivity.java(1 hunks)app/src/org/commcare/activities/FormEntryActivityUIController.java(1 hunks)app/src/org/commcare/activities/FullscreenVideoViewActivity.kt(1 hunks)app/src/org/commcare/activities/GlobalPrivilegeClaimingActivity.java(3 hunks)app/src/org/commcare/activities/HomeButtons.java(1 hunks)app/src/org/commcare/activities/SettingsHelper.java(1 hunks)app/src/org/commcare/activities/connect/ConnectIdActivity.java(1 hunks)app/src/org/commcare/adapters/EntityListAdapter.java(2 hunks)app/src/org/commcare/android/database/user/models/FormRecord.java(1 hunks)app/src/org/commcare/android/logging/ReportingUtils.java(2 hunks)app/src/org/commcare/connect/ConnectConstants.java(2 hunks)app/src/org/commcare/connect/ConnectIDManager.java(1 hunks)app/src/org/commcare/engine/references/JavaHttpReference.java(1 hunks)app/src/org/commcare/engine/resource/ResourceInstallUtils.java(3 hunks)app/src/org/commcare/fragments/BreadcrumbBarHelper.java(1 hunks)app/src/org/commcare/heartbeat/HeartbeatRequester.java(0 hunks)app/src/org/commcare/interfaces/CommcareRequestEndpoints.java(0 hunks)app/src/org/commcare/logic/AndroidFormController.java(1 hunks)app/src/org/commcare/models/AndroidSessionWrapper.java(2 hunks)app/src/org/commcare/sync/ExternalDataUpdateHelper.java(1 hunks)app/src/org/commcare/sync/FirebaseMessagingDataSyncer.java(1 hunks)app/src/org/commcare/tasks/FormLoaderTask.java(3 hunks)app/src/org/commcare/utils/CrashUtil.java(1 hunks)app/src/org/commcare/utils/DimensionUtils.kt(1 hunks)app/src/org/commcare/utils/StringUtils.java(1 hunks)app/src/org/commcare/views/media/CommCareVideoView.java(0 hunks)
💤 Files with no reviewable changes (3)
- app/src/org/commcare/interfaces/CommcareRequestEndpoints.java
- app/src/org/commcare/heartbeat/HeartbeatRequester.java
- app/src/org/commcare/views/media/CommCareVideoView.java
🧰 Additional context used
🧬 Code Graph Analysis (3)
app/src/org/commcare/activities/connect/ConnectIdActivity.java (1)
app/src/org/commcare/connect/ConnectConstants.java (1)
ConnectConstants(8-84)
app/src/org/commcare/connect/ConnectIDManager.java (1)
app/src/org/commcare/utils/CrashUtil.java (1)
CrashUtil(15-61)
app/src/org/commcare/utils/CrashUtil.java (1)
app/src/org/commcare/android/logging/ReportingUtils.java (1)
ReportingUtils(24-159)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Lint Code Base
🔇 Additional comments (40)
app/src/org/commcare/activities/SettingsHelper.java (1)
9-11: Formatting-only change: added blank line between methods for readability.This edit visually separates the two static launcher methods without altering any logic or behavior.
app/src/org/commcare/adapters/EntityListAdapter.java (1)
235-241: Formatting-only indentation adjustment
The re-indentation of thecreateTileForEntitySelectDisplaycall is purely cosmetic and does not affect logic or behavior.app/src/org/commcare/engine/references/JavaHttpReference.java (1)
76-76: Formatting update is safe.A blank line was added after the SSLException catch block to separate error handling from the response processing. This change only affects readability and has no impact on functionality.
app/src/org/commcare/utils/StringUtils.java (1)
9-9: Reintroduced @nonnull import is appropriate.The
@NonNullannotation on theString[] argsparameter ingetStringRobust(line 29) requires this import for compilation and null-safety enforcement.app/src/org/commcare/activities/FormEntryActivity.java (1)
1168-1171: Formatting improvement – ternary indentation.The ternary expression determining
localeKeyhas been re-indented to align with the project’s style guidelines. This change improves readability without affecting any logic or behavior.app/src/org/commcare/android/database/user/models/FormRecord.java (1)
471-471: Removed trailing newline
This formatting cleanup aligns with the repository’s conventions and has no impact on functionality.app/src/org/commcare/logic/AndroidFormController.java (1)
100-100: Removed trailing newline
Minor whitespace cleanup; code behavior remains unchanged.app/src/org/commcare/activities/FullscreenVideoViewActivity.kt (1)
85-85: Removed trailing newline
Consistent formatting cleanup, no functional changes.app/src/org/commcare/sync/ExternalDataUpdateHelper.java (1)
62-62: Removed trailing newline
Pure formatting adjustment; no effect on logic.app/src/org/commcare/activities/HomeButtons.java (1)
31-31: Removed extraneous comma inbuttonNamesarray
This fixes a minor syntax artifact without altering runtime behavior.app/src/org/commcare/activities/FormEntryActivityUIController.java (1)
334-334: Clean code formatting improvement.Removed trailing whitespace from the comment line.
app/src/org/commcare/models/AndroidSessionWrapper.java (2)
52-52: Clean code formatting improvement.Removed trailing whitespace from the comment line.
397-397: Clean code formatting improvement.Removed trailing newline at the end of file.
app/src/org/commcare/tasks/FormLoaderTask.java (3)
129-129: Formatting cleanup: removed trailing whitespace from comment
The trailing space on this comment line has been trimmed. This is a harmless style adjustment and doesn’t affect functionality.
329-330: Formatting cleanup: removed trailing whitespace from comments in serializeFormDef
These lines had trailing spaces after the comment text; they’ve been cleaned up for consistency. No behavior changes introduced.
381-381: Formatting cleanup: removed trailing newline at end of file
The final empty line has been removed to align with the repository’s end-of-file style. Functional behavior is unchanged.app/src/org/commcare/activities/GlobalPrivilegeClaimingActivity.java (3)
61-61: Rename aligns with lint and naming conventions.
ChangingrefreshUItorefreshUiconforms with camelCase lint rules and stylistic consistency across the codebase.
111-111: Refresh UI after QR code scan.
Good addition ofrefreshUi()here to immediately update the UI and menu state after privileges are activated.
161-161: Refresh UI after disabling privileges.
EnsuringrefreshUi()is called after disabling privileges keeps the activity UI and options menu synchronized.app/src/org/commcare/utils/DimensionUtils.kt (1)
1-12: Clean, well-implemented utility class for dimension conversion.The implementation of
convertDpToPixelcorrectly handles the conversion from dp to pixels by using the system display metrics and appropriate scaling. This addition will help maintain consistent UI dimensions across different device densities.app/src/org/commcare/fragments/BreadcrumbBarHelper.java (1)
62-67: Good defensive programming with null check.Adding a null check for the ActionBar prevents potential NullPointerExceptions. This improves robustness since getSupportActionBar() can return null in certain scenarios.
app/src/org/commcare/engine/resource/ResourceInstallUtils.java (3)
3-10: Appropriate imports added for new functionality.These imports are necessary for the new methods being added to the class.
159-165: Well-encapsulated APK update prompt logic.This new utility method centralizes the logic for showing APK update prompts, following the Single Responsibility Principle. The implementation correctly formats the version mismatch message and passes it along with the required version to the prompt activity.
171-174: Clean implementation of target mismatch error display.This utility method simplifies the process of showing target mismatch errors during app installation by encapsulating the intent creation and activity launching.
app/src/org/commcare/connect/ConnectIDManager.java (1)
219-221: Updated crash reporting registration approach.The method now uses the more generalized
registerUserData()instead of the Connect-specific registration method. This change aligns with the unified user identification approach for crash reporting implemented inCrashUtil.app/src/org/commcare/activities/connect/ConnectIdActivity.java (1)
48-48:Details
✅ Verification successful
Updated request code for activity result handling.
The change replaces the previously used
CONNECTID_REQUEST_CODEconstant withConnectConstants.CONNECT_JOB_INFO(value 1034) for consistent request code management across the application.Let's verify that this request code is defined and used consistently:
🏁 Script executed:
#!/bin/bash # Check definition of CONNECT_JOB_INFO constant and any usages rg "CONNECT_JOB_INFO" --type java -A 1 -B 1Length of output: 905
🏁 Script executed:
#!/bin/bash # Verify no lingering references to the old CONNECTID_REQUEST_CODE constant rg "CONNECTID_REQUEST_CODE" --type java -A1 -B1 || echo "No occurrences of CONNECTID_REQUEST_CODE found"Length of output: 150
Consistent request code usage verified
CONNECT_JOB_INFOis correctly defined inapp/src/org/commcare/connect/ConnectConstants.javaand used inapp/src/org/commcare/activities/connect/ConnectIdActivity.java. No remaining references toCONNECTID_REQUEST_CODEwere found.app/src/org/commcare/activities/DispatchActivity.java (1)
68-69: Code organization improvement.The boolean fields
connectIdManagedLoginandconnectManagedLoginhave been repositioned to group them with other instance variables, improving code organization without changing functionality.app/src/org/commcare/activities/CommCareVerificationActivity.java (4)
46-47: Added new constant for Connect integration.The new
KEY_LAUNCH_FROM_CONNECTconstant provides a standardized key for passing intent data when this activity is launched from the Connect feature.
73-73: Added flag to track Connect source.This boolean field tracks whether the activity was launched from Connect, similar to the existing flags for other launch sources.
92-93: Initialized fromConnect flag from intent extras.The implementation follows the same pattern used for the other launch sources, maintaining consistency in the codebase.
137-137: Updated visibility condition to include Connect source.The
shouldBeHerecheck now properly includes the Connect launch case, ensuring the activity remains visible when launched from Connect.app/src/org/commcare/android/logging/ReportingUtils.java (2)
10-10: Added import for ConnectIDManager.Necessary import to support the new crash reporting fallback mechanism.
146-158: Added robust user identification for crash reporting.This new method enhances crash reporting by implementing a fallback mechanism that attempts to retrieve the user ID from ConnectIDManager when the standard user retrieval fails. This creates a more reliable user identification system for crash reports.
The implementation properly:
- Attempts standard user retrieval first
- Falls back to Connect user ID if available
- Handles exceptions gracefully
- Returns null when no user can be identified
app/src/org/commcare/activities/CommCareSetupActivity.java (5)
76-77: Improved comment formatting.Added proper spacing between comment blocks for better readability.
123-124: Relocated menu constants.These constants have been moved from the top-level static fields to the UiState enum section to better organize menu-related code. The constants themselves appear to be used in the onPrepareOptionsMenu and onOptionsItemSelected methods.
643-643: Fixed Connect job navigation.Updated to use the proper ConnectIDManager singleton instance for navigating to Connect jobs list.
744-744: Refactored APK update prompt.Replaced inline UI creation with a call to the centralized utility method
ResourceInstallUtils.showApkUpdatePrompt. This improves code maintenance by centralizing UI prompt logic for APK updates.
780-780: Refactored target mismatch error handling.Replaced inline UI creation with a call to the centralized utility method
ResourceInstallUtils.showTargetMismatchError. This improves code maintenance by centralizing UI prompt logic for target mismatch errors.app/src/org/commcare/connect/ConnectConstants.java (2)
14-14: Added biometric configuration request code.Added a new request code constant for biometric configuration workflows in the Connect feature.
31-34: Added destination page constants for Connect.These new constants represent different CCC (CommCare Connect) destination pages, likely used for navigation or analytics tracking. They follow a consistent naming pattern and are properly organized within the file.
| protected void onResume() { | ||
| super.onResume(); | ||
| refreshUI(); | ||
| refreshUi(); |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Ensure no stale references to refreshUI remain.
Although you've updated calls to refreshUi(), let's verify there are no leftover calls to the old refreshUI() method elsewhere, which could lead to compilation errors.
🏁 Script executed:
#!/bin/bash
# Verify no stale references to 'refreshUI'
rg -n 'refreshUI\('Length of output: 1024
Update stale refreshUI() references to refreshUi()
Several remaining calls and declarations of the old refreshUI() method must be renamed to refreshUi() to prevent compilation errors. Please update the following locations:
- app/src/org/commcare/activities/RootMenuHomeActivity.java:
• public void refreshUI() → refreshUi() - app/src/org/commcare/activities/StandardHomeActivity.java (line 263):
• public void refreshUI() → refreshUi() - app/src/org/commcare/activities/HomeScreenBaseActivity.java:
• Lines 623, 810, 825, 930, 1004, 1111, 1317, 1352: changerefreshUI();→refreshUi();
• Line 1511: changeabstract void refreshUI();→abstract void refreshUi();
After renaming these, rerun your build to confirm no stale references remain.
🤖 Prompt for AI Agents (early access)
In app/src/org/commcare/activities/RootMenuHomeActivity.java, rename the method declaration from refreshUI() to refreshUi(). In app/src/org/commcare/activities/StandardHomeActivity.java at line 263, rename the method declaration from refreshUI() to refreshUi(). In app/src/org/commcare/activities/HomeScreenBaseActivity.java, update all calls to refreshUI() to refreshUi() at lines 623, 810, 825, 930, 1004, 1111, 1317, and 1352, and rename the abstract method declaration at line 1511 from refreshUI() to refreshUi(). After making these changes, rebuild the project to ensure no stale references remain.
| private static final String KEY_REQUIRE_REFRESH = "require_referesh"; | ||
| public static final String KEY_LAUNCH_FROM_SETTINGS = "from_settings"; | ||
|
|
||
| public static final String KEY_LAUNCH_FROM_CONNECT = "from_connect"; |
There was a problem hiding this comment.
This key is never used to set the value
shubham1g5
force-pushed
the
connect_to_master_smallish
branch
from
14f69a8 to
d166677
Compare
Parity changes between master and connect.
Labels and Review