app: AnalyticsAdapter abstraction + rename Mixpanel-named layer to Analytics#7205
app: AnalyticsAdapter abstraction + rename Mixpanel-named layer to Analytics#7205
Conversation
The mobile app finished migrating from Mixpanel to PostHog but kept the
legacy class/file naming, leaving the analytics layer in a state where
the names lie about what's running. This is the first of three commits
that fix that AND add a clean swap point for future provider changes.
This commit is purely additive:
- analytics/analytics_adapter.dart — abstract AnalyticsAdapter interface
with the seven primitives every SDK has to satisfy: init, identify,
alias, track, enable, disable, reset.
- analytics/adapters/posthog_adapter.dart — PostHogAnalyticsAdapter
implementation. Owns apiKey/host/captureLifecycleEvents/debug, the
Posthog() SDK calls, and the per-instance _initialized flag. Calls
before init are no-ops at this layer, so the manager doesn't have to
branch on init state for every method.
setUserProperty is intentionally NOT a primitive — it composes to
`identify(uid, userProperties: {k: v})` and that ergonomic lives at the
manager layer. Keeps the adapter surface narrow so any future provider
(Mixpanel-redux, Amplitude, Segment, …) only has to satisfy seven
methods.
Nothing in the rest of the app imports these yet — added only.
Renames the legacy Mixpanel-named analytics surface to its actual identity post-PostHog migration, and routes all analytics through the AnalyticsAdapter introduced in the previous commit so future provider swaps are one file plus one boot-wiring line. Architectural changes: - analytics_manager.dart absorbs the entire MixpanelManager event surface (132 helper methods + the timed-event machinery + the property coercion helper). Class is now AnalyticsManager. Every adapter call goes through the nullable static `_adapter` field; calling a method before `configure(...)` runs is a clean no-op (the right behavior for environments without a PostHog key, e.g. local dev with no Env). - The old AnalyticsManager surface (setUserAttribute / setUserAttributes / trackEvent — 6 callers across onboarding_provider + home_provider) is preserved as back-compat methods on the new class. setUserAttribute + setUserAttributes still fan out to Intercom for user attributes — that fan-out is the only piece of the multi-target dispatch anyone actually consumed. The dead trackEvent → Intercom fan-out is dropped; if events ever need to land in Intercom, an IntercomAnalyticsAdapter can be configured as a second adapter at boot. - platform_manager.dart `mixpanel` getter renamed to `analytics` and now returns AnalyticsManager. initializeServices calls AnalyticsManager.init(), which lazily constructs a PostHogAnalyticsAdapter from Env.posthogApiKey if no adapter was configured explicitly — preserving today's "PostHog is the default" behavior without locking the manager to PostHog at the type level. - platform_service.dart consolidates the duplicate `isMixpanelSupported` / `isMixpanelNativelySupported` / `isAnalyticsSupported` flags into a single `isAnalyticsSupported` whose value matches the previously-effective gate (`!kIsWeb`). The former `isAnalyticsSupported = true` was dead — every call site actually checked `isMixpanelSupported`, so behavior is preserved. - mixpanel.dart deleted. 396 `MixpanelManager()` invocations and 114 imports across 116 files codemodded to AnalyticsManager / analytics_manager.dart. User-visible "Mixpanel" strings in settings + privacy copy are not touched here — that's a separate concern (privacy policy / disclosure review) and not on the architectural path.
…e.analytics Treats AnalyticsManager as a service held by PlatformManager rather than a global singleton called directly. Same pattern as PlatformManager.instance.intercom and .crashReporter — analytics now sits where readers expect to find platform services, with a single canonical access path. Why this is the cleaner shape: - One way to reach the service. Before, AnalyticsManager() (factory-singleton) and PlatformManager.instance.analytics both worked, and reviewers had to check both surfaces when auditing analytics calls. Single grep target now: `PlatformManager.instance. analytics.`. - Consistent with intercom and crashReporter, which are accessed exclusively through PlatformManager today. New readers don't have to remember that analytics is a one-off. - Decouples 408 call sites from the AnalyticsManager class identity. Renaming or splitting AnalyticsManager later only touches the PlatformManager getter, not the call sites. Codemod scope: 408 invocations across 116 files rewritten from AnalyticsManager().X(...) to PlatformManager.instance.analytics.X(...). Files that no longer reference AnalyticsManager get their now-unused analytics_manager.dart import dropped; files that newly use PlatformManager.instance.analytics get the platform_manager.dart import added if it wasn't already there. The only remaining AnalyticsManager references in the app are the two in platform_manager.dart itself: the getter definition and the boot-time AnalyticsManager.init() call.
|
@greptile-apps review |
Greptile SummaryThis PR replaces the legacy
Confidence Score: 4/5Safe to merge for most deployments; the configure() ordering gap is a latent API hazard not triggered by the current boot path. The 408-site codemod and file rename are mechanical and low-risk. The boot flow in PlatformManager never calls configure(), so the unguarded post-init configure() path is not exercised today. The Intercom event-logging removal in trackEvent() is intentional per the PR description but represents a real behavioural delta that should be confirmed with the team. The setUserProperty per-call identify fan-out is pre-existing. No data-loss or crash scenario is introduced on the current code paths. app/lib/utils/analytics/analytics_manager.dart — configure() ordering contract and trackEvent Intercom removal warrant a second look. Important Files Changed
Sequence DiagramsequenceDiagram
participant Boot as App Boot
participant PM as PlatformManager
participant AM as AnalyticsManager
participant PA as PostHogAnalyticsAdapter
participant PH as Posthog SDK
Boot->>PM: initializeServices()
PM->>AM: init()
AM->>AM: "_adapter == null && Env.posthogApiKey != null?"
AM->>PA: new PostHogAnalyticsAdapter(apiKey)
AM->>PA: init()
PA->>PH: Posthog().setup(config)
PA-->>AM: "_initialized = true"
Note over Boot,PH: Runtime analytics calls
Boot->>PM: instance.analytics.track(event)
PM->>AM: track(event)
AM->>AM: executeIfSupported(isAnalyticsSupported)
AM->>PA: track(eventName, properties)
PA->>PA: if (!_initialized) return
PA->>PH: Posthog().capture(eventName)
Reviews (1): Last reviewed commit: "style(app): apply dart formatter to chan..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| static void configure(AnalyticsAdapter adapter) { | ||
| _adapter = adapter; | ||
| } |
There was a problem hiding this comment.
configure() after init() silently disables all analytics
If configure() is called after init() has already run (e.g., in a test harness or to hot-swap an adapter), _adapter is replaced with a brand-new, uninitialised instance. Every PostHogAnalyticsAdapter guards all its methods with if (!_initialized) return;, so every subsequent track, identify, enable, and disable call becomes a silent no-op. The comment says "Must be called before init()", but there is no runtime enforcement. Any caller who violates that contract gets zero indication that analytics has stopped working. Consider adding an assert(_adapter == null, 'configure() must be called before init()') or calling adapter.init() inside configure() to make the API safe at any order.
| void trackEvent(String eventName, {Map<String, dynamic>? properties}) { | ||
| PlatformManager.instance.mixpanel.track(eventName, properties: properties); | ||
| PlatformManager.instance.intercom.logEvent(eventName, metaData: properties); | ||
| track(eventName, properties: properties); | ||
| } |
There was a problem hiding this comment.
trackEvent silently drops Intercom event logging
In the old AnalyticsManager.trackEvent, the call forwarded to both mixpanel.track(...) and intercom.logEvent(...). The new implementation delegates only to track(), routing exclusively to the PostHog adapter. Any call sites that relied on trackEvent to record Intercom events — e.g., for user lifecycle funnels in Intercom conversations — will no longer receive those events. The PR description notes this drop as intentional, but Intercom event visibility should be verified before merging if Intercom inbox automations depend on these events.
| setUserProperty(String key, dynamic value) => | ||
| PlatformService.executeIfSupported(PlatformService.isAnalyticsSupported, () { | ||
| final adapter = _adapter; | ||
| if (adapter == null) return; | ||
| final uid = _preferences.uid; | ||
| if (uid.isEmpty) return; | ||
| final coerced = _coerceProperty(value); | ||
| if (coerced == null) return; | ||
| adapter.identify(userId: uid, userProperties: {key: coerced}); | ||
| }); |
There was a problem hiding this comment.
setUserProperty fires one identify call per property
Every setUserProperty call invokes adapter.identify with a single-key userProperties map. Callers such as setPeopleValues() chain 8 sequential calls, each issuing its own identify request to PostHog. The old MixpanelManager had the same pattern so this is not a new regression, but now that the architecture is being cleaned up this is a good opportunity to batch all property updates into a single identify call to reduce PostHog ingest load.
… calls - assert configure() runs before init() so a misordered call no longer silently replaces the active adapter with an uninitialised one - collapse setPeopleValues / setNameAndEmail / setUserProperties from N sequential identify() calls into a single batched identify
1 participant
Summary
The mobile app finished migrating from Mixpanel to PostHog but kept the legacy file/class naming, so today the analytics surface is named after a provider that's no longer running. This PR fixes that AND introduces an abstraction so future provider swaps are one file plus one boot-wiring line.
Architecture
Adapter interface —
app/lib/utils/analytics/analytics_adapter.dartProvider primitives only:
init,identify,alias,track,enable,disable,reset. Seven methods. Adding a new analytics SDK = one new file inanalytics/adapters/.PostHog implementation —
app/lib/utils/analytics/adapters/posthog_adapter.dartclass PostHogAnalyticsAdapter implements AnalyticsAdapter. OwnsapiKey,host,captureLifecycleEvents,debug, thePosthog()SDK calls, and the per-instance_initializedflag.Service —
app/lib/utils/analytics/analytics_manager.dartHolds a nullable
AnalyticsAdapter. Staticconfigure(adapter)for explicit injection at boot;init()lazily falls back to aPostHogAnalyticsAdapterfromEnv.posthogApiKeyif no adapter was configured (preserves today's "PostHog is the default" behavior). Owns the 132 event helper methods + the_pendingTimedEventsmachinery + the_coercePropertyrecursive coercion helper. Back-compat surface preserved:setUserAttribute/setUserAttributes/trackEventkeep their PostHog+Intercom user-attribute fan-out (the only fan-out anyone actually consumed); the deadtrackEvent→ Intercom path is dropped.Access pattern —
PlatformManager.instance.analytics.X(...)Call sites reach analytics exclusively through
PlatformManager, the same way they reachintercomandcrashReporter. Never viaAnalyticsManager()direct. Single grep target for audits, decoupled from the class identity for future renames.Naming notes
Provider,Backend,Service, andManagerare all already overloaded in this codebase (state-managementChangeNotifiers,app/lib/backend/directory holding the omi-server API clients,app/lib/services/BLE/FCM bucket,Managertaken at the existing public surface).Adapteris unused exceptBluetoothAdapterin a different domain — and Adapter is the actual design pattern at play (each SDK exposes its own API, we adapt to one common surface).Codemod scope
MixpanelManager()(then briefly viaAnalyticsManager()) toPlatformManager.instance.analyticsanalytics/mixpanel.dartconsolidated; files that no longer referenceAnalyticsManagerdirectly have their imports dropped — onlyplatform_manager.dartitself importsanalytics_manager.dartPlatformManager.instance.mixpanelgetter →.analytics(return type updated toAnalyticsManager)PlatformService.isMixpanelSupported/isMixpanelNativelySupportedconsolidated into the existingisAnalyticsSupported. Value is!kIsWeb(matches what the now-removedisMixpanelSupportedevaluated to)app/lib/utils/analytics/mixpanel.dartdeletedUser-visible "Mixpanel" strings in settings + privacy copy are not touched here — that's a privacy disclosure / legal review concern, not architectural.
Commits
feat(app): introduce AnalyticsAdapter interface + PostHog implementation— additive, nothing breaks.refactor(app): rewrite analytics layer behind AnalyticsAdapter— atomic rename + first codemod + boot wiring + deletemixpanel.dart. Single commit because splitting it leaves a half-state where the 6 existing back-compat callers either lose their PostHog write (null adapter) or double-write (dual-class active).refactor(app): route analytics access through PlatformManager.instance.analytics— second codemod that moves all 408 call sites from the direct singleton to the platform-services access path, matching howintercomandcrashReporterare already accessed.Test plan
MixpanelManager/Posthog()outside the adapter /isMixpanelSupported/analytics/mixpanel.dartimport strings acrossapp/lib.