Repository navigation
CCCT-2790 Add Journey-Aware Analytics To Backup Code And Email Flows - #3958
Open
Jignesh-dimagi wants to merge 8 commits into
Open
Jignesh-dimagi wants to merge 8 commits into
Jignesh-dimagi wants to merge 8 commits into
Conversation
One event covers the backup-code and email journeys, with workflow and event_type params separating them, mirroring how otp_requested already handles the same problem. Only the event name is new; every param constant already existed. Nothing reports it yet. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Reports the confirm-code screen, its attempts and lockout, the forgot-code tap, the set-new-code screen and its outcome. The abandon dialog goes to user_prompt, matching how the app already reports prompts. The confirm and set-new-code screens are shared by several journeys, and their existing arguments cannot say which one the user is in - emailWorkflow carries the forgot route available, not the reason for being there. So BackupCodeWorkflow, until now unused, becomes a nav argument that each entry point sets explicitly. The set-new-code base class spans two nav graphs, so it takes the workflow through an abstract method rather than Safe Args. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Covers the three recovery journeys - account configuration, existing user and pending backup code - which share the same email screens and are told apart by the workflow param. Only the navigation and limit moments are added. Sending and verifying the email OTP already report through otp_requested in EmailHelper, so repeating them here would give two sources for the same action. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Both reminder dialogs report their button taps through user_prompt, matching how the app already reports prompts, with the info param separating them. The code confirmation itself carries an outcome and an attempt count, which user_prompt has no params for, so it goes to the account security event and stays comparable with the same step in the other flows. The Forgot tap is reported from the click listener rather than inside onForgotClicked, which also runs automatically on lockout and would otherwise inflate the count. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Both email-change routes converge on the shared email screens carrying EmailWorkFlow.EXISTING_USER, so those screens cannot tell which auth step the user came through. EXISTING_USER now maps to a neutral email_change workflow, and the two route-specific workflows are emitted only from the auth steps, where the route is known. email_changed goes on the account security event rather than personalid_manage_profile_action, keeping the multi-screen funnel in one table. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
Author
Suggested Review OrderThe six commits tell this in order — reading commit-by-commit is easier than file-by-file:
|
Jignesh-dimagi
requested review from
a team and
OrangeAndGreen
and removed request for
a team
October 8, 2026 12:07
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configuration
📒 Files selected for processing (25)
Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthroughThe change adds account-security analytics for backup-code prompts, confirmation and setup, and email and phone verification. Backup-code workflow arguments now identify the journey through navigation and support workflow-specific analytics. Related tests now provide these arguments. Priority: ⬇️ Low Estimated code review effort: 3 (Moderate) | ~20 minutes Change: Feature Sequence Diagram(s)sequenceDiagram
actor User
participant PersonalIdProfileBackupCodeFragment
participant AccountSecurityAnalyticsMapper
participant FirebaseAnalyticsUtil
participant FirebaseAnalytics
User->>PersonalIdProfileBackupCodeFragment: Submit backup code
PersonalIdProfileBackupCodeFragment->>AccountSecurityAnalyticsMapper: Map BackupCodeWorkflow
AccountSecurityAnalyticsMapper-->>PersonalIdProfileBackupCodeFragment: Analytics workflow
PersonalIdProfileBackupCodeFragment->>FirebaseAnalyticsUtil: Report confirmation outcome and attempt count
FirebaseAnalyticsUtil->>FirebaseAnalytics: Log account-security event
Suggested reviewers: Merge Risk: ⚪ Minimal · up to No actionable merge-blocking issue was identified. The inspected backup-code navigation routes provide the required workflow argument. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 6.45% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 62 functions across 24 files. (1 skipped: 1 unsupported.)
✨ Finishing Touches 💡 2📝 Generate docstrings 💡
🛠️ Fix failing CI checks 💡
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. Comment |
Jignesh-dimagi
marked this pull request as ready for review
October 8, 2026 12:14
The backupCodeWorkflow argument is required, so Safe Args stopped generating the single-argument builders these tests used. Each call site now passes what its real counterpart passes. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Jignesh-dimagi
force-pushed
the
ccct-2790-analytics-backupcode-email-recovery-2.65
branch
from
October 8, 2026 13:09
aedf7b7 to
f96d5c1
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## commcare_2.65 #3958 +/- ##
===================================================
+ Coverage 37.32% 37.45% +0.12%
- Complexity 6864 6900 +36
===================================================
Files 1034 1036 +2
Lines 61094 61585 +491
Branches 7472 7571 +99
===================================================
+ Hits 22802 23064 +262
- Misses 35637 35825 +188
- Partials 2655 2696 +41 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
OrangeAndGreen
approved these changes
Oct 8, 2026
OrangeAndGreen
left a comment
OrangeAndGreen
left a comment
Contributor
There was a problem hiding this comment.
A couple suggestions but nothing I'd consider blocking
The forgot-backup-code verification screen serves both the forgot-code and pending-code journeys, but hardcoded the former, so five of the pending-code journey's events were landing in the forgot-code funnel. The screen now takes the workflow as a nav argument and passes a matching BackupCodeWorkflow on to set-new-code. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
OtpAnalyticsMapper.workflowParam(EmailWorkFlow) has the same signature in the same package, so the wrong import compiled silently. The two write the same workflow param but deliberately different vocabularies: launch context there, journey here. Renaming makes the choice explicit at each call site, and the KDoc records why they are not joinable. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
OrangeAndGreen
approved these changes
Oct 9, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
CCCT-2790
Technical Summary
The backup code and email screens are shared across many journeys, so a screen-level event would have recorded what the user saw without recording why they were there — a confirm-code screen looks identical whether the user is changing their code, recovering access after forgetting it, or authorising an email change. Every step therefore reports one event,
personalid_account_security_action, carrying aworkflowparam naming the journey and anevent_typeparam naming the step, so a single funnel can be read end to end per journey rather than as a pile of anonymous screen views. Where a screen is genuinely shared, the journey is passed in as a nav argument rather than inferred, because the existingEmailWorkFlowenum describes which recovery route is available, not why the user arrived. Anything presented as a dialog reports to the existinguser_promptevent instead, keeping prompts in one place alongside the email reminders, and a newshownaction gives those prompts a denominator so accept and skip read as rates rather than raw counts.outcome,failed_attemptsandreasonare attached only to the steps where they mean something, making dropoff and lockout visible per journey. The net addition is one event and three param values — not one event per flow.Safety Assurance
Safety story
What gives confidence
Risks to review
backupCodeWorkflowrequired, breaking four test files; commit 6 repairs them.Automated test coverage
No new tests. The 174 existing tests across 15 PersonalID screen classes pass.
🤖 Generated with Claude Code