Brings back missing analytics events And Token Exceptions Handling #3075

shubham1g5 · 2025-05-06T06:31:18Z

Product Description

Make some lint changes and logs cleanup
Adds missing analytics code that was PR'ed before in https://github.com/dimagi/commcare-android/pull/2687/files

Labels and Review

Do we need to enhance the manual QA test coverage ? If yes, the "QA Note" label is set correctly
Does the PR introduce any major changes worth communicating ? If yes, the "Release Note" label is set and a "Release Note" is specified in PR description.
Risk label is set correctly
The set of people pinged as reviewers is appropriate for the level of risk of the change

coderabbitai · 2025-05-06T06:32:58Z

📝 Walkthrough

Walkthrough

This change introduces new Firebase Analytics event reporting for user actions such as forgetting a ConnectID account and clicking the login button. Analytics event names are updated, with "ccc_sign_out" renamed to "ccc_forget." The logic for obtaining HQ token authentication is refactored: the relevant method becomes an instance method and its login status check is inverted. The navigation destination change listener for analytics logging is renamed and updated throughout the codebase. Exception handling and logging in authentication building are clarified and simplified, focusing on specific exception types and removing unnecessary log statements.

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant LoginActivity
    participant FirebaseAnalyticsUtil
    participant ConnectIDManager

    User->>LoginActivity: Clicks "Forget" (ConnectID)
    LoginActivity->>FirebaseAnalyticsUtil: reportCccForget()
    LoginActivity->>ConnectIDManager: forgetUser()
    LoginActivity->>LoginActivity: Clear password/PIN, set state, refresh UI

sequenceDiagram
    participant User
    participant LoginActivityUIController
    participant FirebaseAnalyticsUtil
    participant LoginActivity

    User->>LoginActivityUIController: Clicks "Login"
    LoginActivityUIController->>FirebaseAnalyticsUtil: reportLoginClicks()
    LoginActivityUIController->>LoginActivity: initiateLoginAttempt()

sequenceDiagram
    participant CommcareRequestGenerator
    participant ConnectIDManager

    CommcareRequestGenerator->>ConnectIDManager: getHqTokenIfLinked(username)
    alt Manager not logged in
        ConnectIDManager-->>CommcareRequestGenerator: null
    else User and app record found
        ConnectIDManager-->>CommcareRequestGenerator: TokenAuth
    end

Possibly related PRs

Additional logging for failed HQ token auth in ConnectID-managed apps. #2984: Adds detailed logging for failed HQ token authentication in ConnectID-managed apps, modifying related authentication and error handling logic.

Suggested labels

cross requested

Suggested reviewers

OrangeAndGreen

📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 9f002a6 and d21859e.

📒 Files selected for processing (8)

app/src/org/commcare/activities/LoginActivity.java (1 hunks)
app/src/org/commcare/activities/LoginActivityUIController.java (2 hunks)
app/src/org/commcare/activities/connect/ConnectActivity.java (1 hunks)
app/src/org/commcare/activities/connect/ConnectMessagingActivity.java (1 hunks)
app/src/org/commcare/connect/ConnectIDManager.java (1 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/network/CommcareRequestGenerator.java (3 hunks)

🧰 Additional context used

🧬 Code Graph Analysis (4)

app/src/org/commcare/activities/connect/ConnectActivity.java (1)

app/src/org/commcare/google/services/analytics/FirebaseAnalyticsUtil.java (1)

FirebaseAnalyticsUtil (38-524)

app/src/org/commcare/activities/LoginActivityUIController.java (1)

app/src/org/commcare/google/services/analytics/FirebaseAnalyticsUtil.java (1)

FirebaseAnalyticsUtil (38-524)

app/src/org/commcare/activities/LoginActivity.java (1)

app/src/org/commcare/google/services/analytics/FirebaseAnalyticsUtil.java (1)

FirebaseAnalyticsUtil (38-524)

app/src/org/commcare/google/services/analytics/FirebaseAnalyticsUtil.java (1)

app/src/org/commcare/google/services/analytics/CCAnalyticsEvent.java (1)

CCAnalyticsEvent (7-54)

🔇 Additional comments (14)

app/src/org/commcare/google/services/analytics/CCAnalyticsEvent.java (1)

35-35: Event name updated to better reflect action intent.

The event name change from "CCC_SIGN_OUT" to "CCC_FORGET" better represents the user action of forgetting a ConnectID account rather than just signing out. This aligns with corresponding changes in other files.

app/src/org/commcare/google/services/analytics/FirebaseAnalyticsUtil.java (3)

486-488: Implemented Firebase event tracking for account forget functionality.

This method reports the CCC_FORGET event when a user forgets their ConnectID account, replacing the previous sign-out event tracking.

490-492: Added login click tracking for improved analytics.

This new method enables tracking when users click the login button, providing valuable user interaction data for analytics.

494-508: Improved method naming with clear descriptive comment.

The method has been renamed from getDestinationChangeListener() to getNavControllerPageChangeLoggingListener() with an explanatory comment. This provides better clarity about its purpose - logging screen view events when attached to a navigation controller.

app/src/org/commcare/activities/connect/ConnectMessagingActivity.java (1)

31-31: Updated to use renamed analytics listener method.

Updated to use the more descriptively named getNavControllerPageChangeLoggingListener() method, consistent with changes in FirebaseAnalyticsUtil class.

app/src/org/commcare/activities/LoginActivityUIController.java (2)

31-31: Added Firebase analytics import.

Added import for FirebaseAnalyticsUtil to support login click tracking.

162-165: Added Firebase analytics for login button interactions.

Enhanced the login button click listener to report user login attempts to Firebase Analytics before initiating the actual login process. This provides valuable user interaction data without affecting the existing login flow.

app/src/org/commcare/activities/LoginActivity.java (1)

583-583: Adding analytics tracking for user "forget" action.

This line adds Firebase analytics tracking for when a user chooses to forget their ConnectID account, restoring previously missing analytics code.

app/src/org/commcare/activities/connect/ConnectActivity.java (1)

77-77: Method name updated for clarity.

The navigation destination listener assignment has been updated to use the renamed getNavControllerPageChangeLoggingListener() method instead of the previous getDestinationChangeListener(). The new method name is more descriptive of its functionality.

app/src/org/commcare/connect/ConnectIDManager.java (2)

633-635: Method signature and logic changed for HQ token retrieval.

The getHqTokenIfLinked method has been changed from static to instance scope, and the login condition has been inverted. Now the method returns null if the user is logged in (which makes logical sense since a logged-in user wouldn't need a separate HQ token auth).

638-638: Updated variable references for instance method.

These lines have been updated to use the instance field parentActivity instead of the static manager.parentActivity to align with the method's conversion from static to instance scope.

Also applies to: 644-644

app/src/org/commcare/network/CommcareRequestGenerator.java (3)

181-181: Updated token auth retrieval to use instance method.

Changed from using the static method ConnectIDManager.getHqTokenIfLinked() to the instance method ConnectIDManager.getInstance().getHqTokenIfLinked() to match the updated method signature in ConnectIDManager.

186-188: Simplified logging for token auth failures.

Logging for token auth failures has been simplified to reduce verbosity while still providing essential information.

194-196: Improved exception handling with specific exception type.

Changed from catching general Exception to the more specific SessionUnavailableException, which is better practice as it makes the error handling more precise and the intent clearer.

✨ Finishing Touches

📝 Generate Docstrings

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.

❤️ Share

🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

Review comments: Directly reply to a review comment made by CodeRabbit. Example:
- I pushed a fix in commit <commit_id>, please review it.
- Generate unit testing code for this file.
- Open a follow-up GitHub issue for this discussion.
Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
- @coderabbitai generate unit testing code for this file.
- @coderabbitai modularize this function.
PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
- @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
- @coderabbitai read src/utils.ts and generate unit testing code.
- @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
- @coderabbitai help me debug CodeRabbit configuration file.

Support

Need 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)

@coderabbitai pause to pause the reviews on a PR.
@coderabbitai resume to resume the paused reviews.
@coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
@coderabbitai full review to do a full review from scratch and review all the files again.
@coderabbitai summary to regenerate the summary of the PR.
@coderabbitai generate docstrings to generate docstrings for this PR.
@coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
@coderabbitai resolve resolve all the CodeRabbit review comments.
@coderabbitai configuration to show the current CodeRabbit configuration for the repository.
@coderabbitai help to get help.

Other keywords and placeholders

Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (`.coderabbit.yaml`)

You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
Please see the configuration documentation for more information.
If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

Visit our Documentation for detailed information on how to use CodeRabbit.
Join our Discord Community to get help, request features, and share feedback.
Follow us on X/Twitter for updates and announcements.

app/src/org/commcare/activities/SyncCapableCommCareActivity.java

app/src/org/commcare/activities/connect/ConnectActivity.java

app/src/org/commcare/google/services/analytics/CCAnalyticsEvent.java

app/src/org/commcare/connect/ConnectIDManager.java

app/src/org/commcare/network/CommcareRequestGenerator.java

shubham1g5 · 2025-05-07T06:53:47Z

app/assets/locales/android_translatable_strings.txt

 notification.token.denied.title=Couldn't connect to the server.
 notification.token.denied.detail=CommCare can no longer communicate with the ConnectID server.
-notification.token.denied.action=Please recover your ConnectID account, then try again.
+notification.token.denied.action=Please re-login into your ConnectID account, then try again.


@OrangeAndGreen Given we are moving away from recover workflow, I changed this to re-login but think there is a better word out there that I am missing here.

I agree in the change but not sure of a better term. This is short-lived anyway I think until we can provide a better user experience for situations like this

OrangeAndGreen · 2025-05-07T17:17:34Z

app/assets/locales/android_translatable_strings.txt

 notification.token.denied.title=Couldn't connect to the server.
 notification.token.denied.detail=CommCare can no longer communicate with the ConnectID server.
-notification.token.denied.action=Please recover your ConnectID account, then try again.
+notification.token.denied.action=Please re-login into your ConnectID account, then try again.


I agree in the change but not sure of a better term. This is short-lived anyway I think until we can provide a better user experience for situations like this

shubham1g5 added 4 commits May 6, 2025 10:09

rename method

10fcedf

static to non static

d2121b0

clean logs for buildAuth and correct exception handling

981b8a6

re-establish forgotten events

d21859e

shubham1g5 requested review from OrangeAndGreen and pm-dimagi May 6, 2025 06:31

shubham1g5 added the skip-integration-tests Skip android tests. label May 6, 2025

remove log as we are already logging the token error in network layer

15751f4

OrangeAndGreen requested changes May 6, 2025

View reviewed changes

shubham1g5 added 5 commits May 6, 2025 23:41

refactor to abstract common nav Controller in an abstract class

7cfa930

fix the log in check

4dca13f

revert to logger.exception

2461bf3

Add handling of token exceptions in sync and login code

c5c10ab

rename exception

787f46c

shubham1g5 changed the title ~~Brings back missing analytics events~~ Brings back missing analytics events And Token Exceptions Handling May 7, 2025

shubham1g5 requested a review from OrangeAndGreen May 7, 2025 06:31

shubham1g5 commented May 7, 2025

View reviewed changes

OrangeAndGreen approved these changes May 7, 2025

View reviewed changes

shubham1g5 merged commit 513dfc5 into dv/connect_initial May 7, 2025
1 of 2 checks passed

shubham1g5 deleted the sg/minor_connect branch May 7, 2025 17:50

shubham1g5 mentioned this pull request May 8, 2025

Correct method name #3082

Merged

4 tasks

Uh oh!

Brings back missing analytics events And Token Exceptions Handling #3075

Brings back missing analytics events And Token Exceptions Handling #3075

Uh oh!

Conversation

shubham1g5 commented May 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Product Description

Labels and Review

Uh oh!

coderabbitai bot commented May 6, 2025

Walkthrough

Sequence Diagram(s)

Possibly related PRs

Suggested labels

Suggested reviewers

Chat

Support

CodeRabbit Commands (Invoked using PR comments)

Other keywords and placeholders

CodeRabbit Configuration File (.coderabbit.yaml)

Documentation and Community

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

shubham1g5 May 7, 2025

Choose a reason for hiding this comment

Uh oh!

OrangeAndGreen May 7, 2025

Choose a reason for hiding this comment

Uh oh!

OrangeAndGreen May 7, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

shubham1g5 commented May 6, 2025 •

edited

Loading

CodeRabbit Configuration File (`.coderabbit.yaml`)