Mobile posthog analytics#3769
Merged
Merged
Conversation
- Remove research_mode feature-flag gate so events capture for all users; drop the now-unused flag and isAnalyticsEnabled getter - Add forecast events (forecast_viewed, forecast_scope_changed) - Add share funnel events (share_sheet_opened with source, share_tab_selected, share_completed gated on ShareResult success) - Add Clean Air Forum filter funnel events (tab opened, selfie source/capture, filter shared, wall consent and submission sent/failed) - Return ShareResult from AirQualityShareService so only real shares count - Enable lifecycle event capture and switch personProfiles to identifiedOnly - Add PosthogObserver for automatic $screen tracking and name all MaterialPageRoute push sites via RouteSettings - Register is_guest super property on guest entry/login; reset identity on session expiry fallback to guest - Delete unused manual session tracking (SDK manages sessions)
- Split AnalyticsService: core transport (capture/identity/super props) stays in analytics_service.dart; typed event wrappers move to per-domain extension files under services/analytics/, re-exported so call sites are unchanged - Inject AnalyticsService into AuthBloc and SurveyBloc (defaulting to the singleton) so bloc tests can mock analytics, matching the repository injection convention - Fold share-tab label and analytics name into the _ShareTab enhanced enum, restoring the single-entry-per-tab promise
Contributor
|
Warning Review limit reached
Next review available in: 26 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughThis PR expands PostHog analytics across the mobile app: it enables lifecycle capture, adds a navigator observer for screen tracking, refactors AnalyticsService into domain-specific event extensions, injects AnalyticsService into AuthBloc and SurveyBloc, updates share flows to return ShareResult with success-based tracking, adjusts feature flag handling, and attaches named RouteSettings to dozens of navigation calls app-wide. ChangesAnalytics Instrumentation and Navigation Tracking
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
Suggested labels: Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsErrors were encountered while retrieving linked issues. Errors (1)
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 |
PosthogObserver fires Posthog().screen on navigation; in tests the channel throws MissingPluginException, which the SDK only guards as PlatformException, failing the app-boot test.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
src/mobile/lib/src/app/surveys/bloc/survey_bloc.dart (1)
258-262: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
trackSurveyCompletedfires unconditionally regardless of sync success.
repository.submitSurveyResponsereturns asuccessflag that distinguishes server-synced submissions from locally-saved-only ones. However,trackSurveyCompletedis called without passing this flag, andtrackSurveySubmissionFailedonly fires on exceptions. Whensuccess == false(saved locally, not synced), the event is still recorded as "completed" with nosyncedproperty — making it impossible to distinguish the two outcomes in analytics dashboards.Consider adding a
syncedproperty to the event, or callingtrackSurveySubmissionFailedwhensuccess == false.♻️ Proposed refactor: pass sync status to analytics
await analytics.trackSurveyCompleted( surveyId: currentState.survey.id, responseTime: completionTime.inSeconds, deviceId: currentState.currentResponse.deviceId, + synced: success, );This requires adding a
syncedparameter totrackSurveyCompletedinanalytics_survey_events.dart:Future<void> trackSurveyCompleted({ String? surveyId, int? responseTime, String? deviceId, + bool? synced, }) => trackEvent('survey_completed', properties: { if (surveyId != null) 'survey_id': surveyId, if (responseTime != null) 'response_time': responseTime, if (deviceId != null) 'device_id': deviceId, + if (synced != null) 'synced': synced, });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/mobile/lib/src/app/surveys/bloc/survey_bloc.dart` around lines 258 - 262, `trackSurveyCompleted` in `SurveyBloc` is being emitted even when `repository.submitSurveyResponse` reports `success == false`, so analytics can’t distinguish synced vs locally saved submissions. Update the survey completion flow to thread the sync result from `submitSurveyResponse` into `trackSurveyCompleted` (for example by adding a `synced` parameter in `analytics_survey_events.dart`), or route unsynced outcomes to `trackSurveySubmissionFailed` when `success` is false. Make sure the analytics call sites in `SurveyBloc` and the event API stay aligned.src/mobile/lib/src/app/profile/pages/profile_page.dart (1)
162-169: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRouteSettings formatting could be simplified.
The
RouteSettingsis split across four lines for a simple name string. Collapsing to a single line improves readability and matches the style used in other files (e.g.,welcome_screen.dartline 131).♻️ Suggested formatting cleanup
onTap: () => Navigator.of(context) .push(MaterialPageRoute( - settings: - const RouteSettings( - name: - 'edit_profile'), + settings: const RouteSettings(name: 'edit_profile'), builder: (context) => EditProfile())),🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/mobile/lib/src/app/profile/pages/profile_page.dart` around lines 162 - 169, Simplify the RouteSettings formatting in the Navigator push for EditProfile by collapsing the const RouteSettings(name: 'edit_profile') into a single line. Update the onTap callback in profile_page.dart to match the cleaner style used elsewhere, keeping the MaterialPageRoute and EditProfile symbols unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/mobile/lib/src/app/auth/bloc/auth_bloc.dart`:
- Around line 66-74: Add the missing analytics fallback in the
`_onSessionExpired` handler so it matches the session-expiry branch in
`_onAppStarted`. In `AuthBloc`, before emitting the guest/session-expired states
from `_onSessionExpired`, call `analytics.resetUser()` and
`analytics.markGuestSession()` just like the app-start path does. Keep the
existing session-expiry flow intact, but ensure the runtime `SessionExpired`
path clears the prior identity and marks the user as guest before falling back.
In `@src/mobile/lib/src/app/dashboard/widgets/air_quality_share_sheet.dart`:
- Around line 346-349: Avoid forwarding raw exception text to analytics in the
catch block around CleanAirForumSubmissionService.submitSelfie and
AnalyticsService().trackCafWallSubmissionFailed; replace e.toString() with a
fixed error code or a sanitized failure bucket so internal API details from
thrown exceptions are not sent to PostHog. Keep the existing success/failure
flow in air_quality_share_sheet.dart, but update the failure tracking call to
use a safe, predefined value that identifies the error category without exposing
response content.
In `@src/mobile/lib/src/app/shared/services/analytics_service.dart`:
- Around line 38-46: The logout flow in _onLogoutUser resets analytics identity
but does not restore the guest super property, so make sure markGuestSession()
is called after resetUser() completes, or centralize that behavior inside
resetUser() itself. Update the logout handling in the analytics service so every
identity reset leaves Posthog in guest mode, using the existing
markGuestSession() method and the resetUser() path as the key symbols to modify.
---
Nitpick comments:
In `@src/mobile/lib/src/app/profile/pages/profile_page.dart`:
- Around line 162-169: Simplify the RouteSettings formatting in the Navigator
push for EditProfile by collapsing the const RouteSettings(name: 'edit_profile')
into a single line. Update the onTap callback in profile_page.dart to match the
cleaner style used elsewhere, keeping the MaterialPageRoute and EditProfile
symbols unchanged.
In `@src/mobile/lib/src/app/surveys/bloc/survey_bloc.dart`:
- Around line 258-262: `trackSurveyCompleted` in `SurveyBloc` is being emitted
even when `repository.submitSurveyResponse` reports `success == false`, so
analytics can’t distinguish synced vs locally saved submissions. Update the
survey completion flow to thread the sync result from `submitSurveyResponse`
into `trackSurveyCompleted` (for example by adding a `synced` parameter in
`analytics_survey_events.dart`), or route unsynced outcomes to
`trackSurveySubmissionFailed` when `success` is false. Make sure the analytics
call sites in `SurveyBloc` and the event API stay aligned.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 5d0dd540-100c-4595-a4b3-8805ea34abbe
📒 Files selected for processing (39)
src/mobile/lib/main.dartsrc/mobile/lib/src/app/auth/bloc/auth_bloc.dartsrc/mobile/lib/src/app/auth/pages/email_verification_screen.dartsrc/mobile/lib/src/app/auth/pages/login_page.dartsrc/mobile/lib/src/app/auth/pages/password_reset/forgot_password.dartsrc/mobile/lib/src/app/auth/pages/password_reset/password_reset.dartsrc/mobile/lib/src/app/auth/pages/password_reset/reset_link_sent.dartsrc/mobile/lib/src/app/auth/pages/password_reset/reset_success.dartsrc/mobile/lib/src/app/auth/pages/register_page.dartsrc/mobile/lib/src/app/auth/pages/welcome_screen.dartsrc/mobile/lib/src/app/auth/services/auth_validation_helper.dartsrc/mobile/lib/src/app/dashboard/pages/dashboard_page.dartsrc/mobile/lib/src/app/dashboard/pages/forecast_overview_page.dartsrc/mobile/lib/src/app/dashboard/pages/location_selection/location_selection_screen.dartsrc/mobile/lib/src/app/dashboard/widgets/air_quality_share_sheet.dartsrc/mobile/lib/src/app/dashboard/widgets/analytics_specifics.dartsrc/mobile/lib/src/app/dashboard/widgets/dashboard_app_bar.dartsrc/mobile/lib/src/app/dashboard/widgets/my_places_view.dartsrc/mobile/lib/src/app/exposure/pages/exposure_dashboard_view.dartsrc/mobile/lib/src/app/learn/pages/learn_surveys_page.dartsrc/mobile/lib/src/app/profile/pages/guest_about_page.dartsrc/mobile/lib/src/app/profile/pages/guest_account_access_page.dartsrc/mobile/lib/src/app/profile/pages/profile_page.dartsrc/mobile/lib/src/app/profile/pages/widgets/account_deletion_handler.dartsrc/mobile/lib/src/app/profile/pages/widgets/guest_settings_widget.dartsrc/mobile/lib/src/app/profile/pages/widgets/settings_widget.dartsrc/mobile/lib/src/app/shared/services/air_quality_share_service.dartsrc/mobile/lib/src/app/shared/services/analytics/analytics_app_events.dartsrc/mobile/lib/src/app/shared/services/analytics/analytics_auth_events.dartsrc/mobile/lib/src/app/shared/services/analytics/analytics_dashboard_events.dartsrc/mobile/lib/src/app/shared/services/analytics/analytics_learn_events.dartsrc/mobile/lib/src/app/shared/services/analytics/analytics_share_events.dartsrc/mobile/lib/src/app/shared/services/analytics/analytics_survey_events.dartsrc/mobile/lib/src/app/shared/services/analytics_service.dartsrc/mobile/lib/src/app/shared/services/feature_flag_service.dartsrc/mobile/lib/src/app/shared/services/navigation_service.dartsrc/mobile/lib/src/app/surveys/bloc/survey_bloc.dartsrc/mobile/lib/src/app/surveys/widgets/new_survey_banner.dartsrc/mobile/lib/src/app/surveys/widgets/survey_list_content.dart
…hygiene - resetUser() now re-registers is_guest=true after clearing identity, making guest fallback an invariant of every identity reset (logout, expiry) - Runtime SessionExpired handler resets analytics identity like the app-start expiry path already did - Failure events report the exception type instead of raw e.toString() so API response details are not sent to PostHog (CAF wall + survey submission)
… paths Sweep for the same class of issue as the review findings: account deletion, the app-start error fallback, and the session-expiry cleanup-failure path all emitted GuestUser without resetUser(), leaving events attributed to the prior identity. Account deletion was the worst case - a deleted user's device kept reporting under their identity. resetUser() is safe in catch blocks since it never throws.
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.
Summary by CodeRabbit