feat(analytics-controller): update controller and adapter interface for mobile integration [Phase 1.4] #7080
+959
−314
Conversation
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
- Add onSetupCompleted lifecycle hook to AnalyticsPlatformAdapter interface - Remove add method from interface (breaking change) - Change AnalyticsPlatformAdapter from type alias to interface - Add UUIDv4 validation for analyticsId with error handling - Add AnalyticsUserTraits type for better semantic separation - Rename trackEvent to track and trackPage to view in adapter interface - Update controller to call onSetupCompleted after initialization - Add comprehensive tests for lifecycle hook and validation - Update documentation with lifecycle hook usage examples
NicolasMassart
changed the title
…y method - Add isOptedIn method and corresponding action type - Refactor identify method to use analyticsId from state instead of userId parameter - Update tests to use realistic traits (ENABLE_OPENSEA_API, NFT_AUTODETECTION) instead of email - Update UUID test example from v1 to v5 - Remove obsolete tests - Fix formatting issues
…TrackingEvent object - Add AnalyticsJsonValue and AnalyticsJsonMap types similar to legacy types for easier migration - Add AnalyticsTrackingEvent type matching legacy ITrackingEvent structure - Update trackEvent method to accept AnalyticsTrackingEvent instead of separate eventName and properties - Implement same logic as legacy trackEvent (handles hasProperties, isAnonymous, sensitiveProperties) - Remove redundant saveDataRecording parameter (now part of event object) - Convert interfaces to type aliases for consistency - Update tests
…m:MetaMask/core into refactor/7079_update_analytics_controller
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
mcmire
reviewed
Nov 17, 2025
Contributor
mcmire
left a comment
mcmire
left a comment
There was a problem hiding this comment.
Hi @NicolasMassart! Thanks for the ping. I had some comments and suggestions below.
| * Represents values that can be passed as properties to the event tracking function. | ||
| * Similar to JsonValue from Segment SDK but decoupled for platform agnosticism. | ||
| */ | ||
| type AnalyticsJsonValue = |
Contributor
There was a problem hiding this comment.
Is it possible to simplify this type and use Json? I see that there are slight differences between this type and Json, namely that objects are allowed to have number keys. However if this data really gets serialized to JSON, wouldn't the keys get converted to strings anyway?
Contributor
Author
There was a problem hiding this comment.
Here are the resons why I used this explicit type:
Using Record<string, Json> (current):
✅ Explicit: must be an object whith named props of Json type value
✅ Type-safe: rejects arrays, primitives, null
✅ Consistent with BaseController.StateConstraint and ApprovalController
Using Json directly:
✅ Simpler
✅ Matches runtime JSON serialization behavior
❌ Less type-safe: allows arrays, primitives, null
❌ Less explicit intent as naming the AnalyticsEventProperties and AnalyticsUserTraits types allows to clearly identify the usage in controller.
My recommendation:
Keep Record<string, Json> for type safety and consistency. If you really prefers Json, we can switch, but it weakens compile-time guarantees.
packages/analytics-controller/src/AnalyticsPlatformAdapter.types.ts
Outdated
Show resolved
Hide resolved
mcmire
reviewed
Nov 17, 2025
…and extract validation - Remove isAnonymous field, derive sensitivity from sensitiveProperties presence - Extract state validation to analyticsStateValidator module (called before super) - Add selectors module for state access - Add comprehensive unit tests for validator and selectors - Update documentation for clarity
…m:MetaMask/core into refactor/7079_update_analytics_controller # Conflicts: # packages/analytics-controller/src/selectors.test.ts
Contributor
Author
|
@metamaskbot publish-preview |
Contributor
|
Preview builds have been published. See these instructions for more information about preview builds. Expand for full list of packages and versions. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Important
This is an unreleased package which will continue to be improved until ready for release.
analytics-controllerupdateonSetupCompletedlifecycle hook for post-initialization setupgetAnalyticsId(),isEnabled(),isOptedIn(),isSocialOptedIn()socialOptIn(),socialOptOut()AnalyticsPlatformAdapterSetupErrorfor setup failuresenabled→ computed viacomputeEnabledState(),optedIn→user_optedIn,analyticsId→user_analyticsId, addeduser_socialOptedInanalyticsStateComputermodule withcomputeEnabledState()function to compute enabled state from user opt-in statusenable()anddisable()methods (replaced by computed enabled state)AnalyticsTrackingEventtype withproperties,sensitiveProperties,isAnonymous,hasPropertiesfor anonymous/non-anonymous trackingtrackEventto acceptAnalyticsTrackingEventinstead of separateeventNameandpropertiesparametersAnalyticsPlatformAdaptertrackPage→trackViewfor platform agnostic usagetrackEvent→trackto better match Segment SDKsidentify: removeduserIdparameter, usesanalyticsIdfrom state, addedAnalyticsUserTraitstypeReferences
fixes #7079
Checklist
Note
Refactors controller/adapter APIs (track/view, new event type), adds onSetupCompleted and UUID validation, splits opt-in state with computed enabled, and introduces selectors with updated actions, tests, and docs.
trackEvent→trackandtrackPage→trackView; update messenger actions totrackViewand new opt-in actions for regular/social accounts.identifynow uses currentanalyticsIdand acceptstraitsonly.trackEventnow acceptsAnalyticsTrackingEvent(supportsproperties/sensitiveProperties,hasProperties).enabled/optedInwithoptedInForRegularAccountandoptedInForSocialAccount; enabled is derived viacomputeEnabledState.selectors.ts(selectAnalyticsId,selectOptedIn,selectSocialOptedIn,selectEnabled).track,identify,view; addonSetupCompleted(analyticsId)lifecycle hook andAnalyticsPlatformAdapterSetupError.analyticsIdas UUIDv4 (analyticsStateValidator).isSensitiveflag.Written by Cursor Bugbot for commit 21a5dda. This will update automatically on new commits. Configure here.