Skip to content

Commit

Permalink
chore: move metrics identify to state listener (#13203)
Browse files Browse the repository at this point in the history
## **Description**

- moves identify call to active app state listener
- updates unit tests
- also fixes `trackEvent` mock wrongly being used as async in
app/core/AppStateEventListener.test.ts

## **Related issues**

Fixes MetaMask/mobile-planning#2119

## **Manual testing steps**

```gherkin
Feature: identify users
  Scenario: identify on app state change to active
    Given an already onboarded app
    And user opted in for metrics
    When you open the app in local dev mode
    Then you have the `IDENTIFY event saved` log
```

## **Screenshots/Recordings**

<!-- If applicable, add screenshots and/or recordings to visualize the
before and after of your change. -->

### **Before**

IDENTIFY event sent but only on app open and not when foregrounding
(active)

### **After**

```
 INFO  IDENTIFY event saved {"traits": {"Batch account balance requests": "ON", "Enable OpenSea API": "ON", "NFT Autodetection": "ON", "Theme": "light", "applicationVersion": "7.37.1", "currentBuildNumber": "1520", "deviceBrand": "Apple", "operatingSystemVersion": "18.2", "platform": "ios", "security_providers": "blockaid", "token_detection_enable": "ON"}, "type": "identify", "userId": "150739a9-38b7-4098-b1f0-7afbba0b2e5d"}
 INFO  TRACK event saved {"event": "App Opened", "properties": {}, "type": "track"}
 INFO  Sent 2 events
```

## **Pre-merge author checklist**

- [x] I’ve followed [MetaMask Contributor
Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Mobile
Coding
Standards](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/CODING_GUIDELINES.md).
- [x] I've completed the PR template to the best of my ability
- [x] I’ve included tests if applicable
- [x] I’ve documented my code using [JSDoc](https://jsdoc.app/) format
if applicable
- [x] I’ve applied the right labels on the PR (see [labeling
guidelines](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/LABELING_GUIDELINES.md)).
Not required for external contributors.

## **Pre-merge reviewer checklist**

- [ ] I've manually tested the PR (e.g. pull and build branch, run the
app, test code being changed).
- [ ] I confirm that this PR addresses all acceptance criteria described
in the ticket it closes and includes the necessary testing evidence such
as recordings and or screenshots.
  • Loading branch information
NicolasMassart authored Jan 31, 2025
1 parent 7ee851d commit 711a583
Show file tree
Hide file tree
Showing 4 changed files with 61 additions and 17 deletions.
12 changes: 1 addition & 11 deletions app/components/Nav/App/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -109,8 +109,6 @@ import SDKSessionModal from '../../Views/SDK/SDKSessionModal/SDKSessionModal';
import ExperienceEnhancerModal from '../../../../app/components/Views/ExperienceEnhancerModal';
import { MetaMetrics } from '../../../core/Analytics';
import trackErrorAsAnalytics from '../../../util/metrics/TrackError/trackErrorAsAnalytics';
import generateDeviceAnalyticsMetaData from '../../../util/metrics/DeviceAnalyticsMetaData/generateDeviceAnalyticsMetaData';
import generateUserSettingsAnalyticsMetaData from '../../../util/metrics/UserSettingsAnalyticsMetaData/generateUserProfileAnalyticsMetaData';
import LedgerSelectAccount from '../../Views/LedgerSelectAccount';
import OnboardingSuccess from '../../Views/OnboardingSuccess';
import DefaultSettings from '../../Views/OnboardingSuccess/DefaultSettings';
Expand Down Expand Up @@ -745,15 +743,7 @@ const App = (props) => {

useEffect(() => {
const initMetrics = async () => {
const metrics = MetaMetrics.getInstance();
await metrics.configure();
// identify user with the latest traits
// run only after the MetaMetrics is configured
const consolidatedTraits = {
...generateDeviceAnalyticsMetaData(),
...generateUserSettingsAnalyticsMetaData(),
};
await metrics.addTraitsToUser(consolidatedTraits);
await MetaMetrics.getInstance().configure();
};

initMetrics().catch((err) => {
Expand Down
4 changes: 0 additions & 4 deletions app/components/Nav/App/index.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -61,10 +61,6 @@ describe('App', () => {
);
await waitFor(() => {
expect(mockMetrics.configure).toHaveBeenCalledTimes(1);
expect(mockMetrics.addTraitsToUser).toHaveBeenNthCalledWith(1, {
deviceProp: 'Device value',
userProp: 'User value',
});
});
});
});
45 changes: 44 additions & 1 deletion app/core/AppStateEventListener.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ jest.mock('./processAttribution', () => ({
jest.mock('./Analytics/MetaMetrics');

const mockMetrics = {
trackEvent: jest.fn().mockImplementation(() => Promise.resolve()),
trackEvent: jest.fn(),
enable: jest.fn(() => Promise.resolve()),
addTraitsToUser: jest.fn(() => Promise.resolve()),
isEnabled: jest.fn(() => true),
Expand Down Expand Up @@ -105,6 +105,49 @@ describe('AppStateEventListener', () => {
);
});

it('identifies user when app becomes active', () => {
jest
.spyOn(ReduxService, 'store', 'get')
.mockReturnValue({} as unknown as ReduxStore);

mockAppStateListener('active');
jest.advanceTimersByTime(2000);

expect(mockMetrics.addTraitsToUser).toHaveBeenCalledTimes(1);
expect(mockMetrics.addTraitsToUser).toHaveBeenCalledWith({
'Batch account balance requests': 'OFF',
'Enable OpenSea API': 'OFF',
'NFT Autodetection': 'OFF',
'Theme': undefined,
'applicationVersion': expect.any(Promise),
'currentBuildNumber': expect.any(Promise),
'deviceBrand': 'Apple',
'operatingSystemVersion': 'ios',
'platform': 'ios',
'security_providers': '',
'token_detection_enable': 'OFF',
});
});

it('logs error when identifying user fails', () => {
jest
.spyOn(ReduxService, 'store', 'get')
.mockReturnValue({} as unknown as ReduxStore);
const testError = new Error('Test error');
mockMetrics.addTraitsToUser.mockImplementation(() => {
throw testError;
});

mockAppStateListener('active');
jest.advanceTimersByTime(2000);

expect(Logger.error).toHaveBeenCalledWith(
testError,
'AppStateManager: Error processing app state change'
);
expect(mockMetrics.trackEvent).not.toHaveBeenCalled();
});

it('handles errors gracefully', () => {
jest
.spyOn(ReduxService, 'store', 'get')
Expand Down
17 changes: 16 additions & 1 deletion app/core/AppStateEventListener.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,9 @@ import { MetricsEventBuilder } from './Analytics/MetricsEventBuilder';
import { processAttribution } from './processAttribution';
import DevLogger from './SDKConnect/utils/DevLogger';
import ReduxService from './redux';
import generateDeviceAnalyticsMetaData from '../util/metrics';
import generateUserSettingsAnalyticsMetaData
from '../util/metrics/UserSettingsAnalyticsMetaData/generateUserProfileAnalyticsMetaData';

export class AppStateEventListener {
private appStateSubscription:
Expand Down Expand Up @@ -48,6 +51,18 @@ export class AppStateEventListener {
currentDeeplink: this.currentDeeplink,
store: ReduxService.store,
});
const metrics = MetaMetrics.getInstance();
// identify user with the latest traits
const consolidatedTraits = {
...generateDeviceAnalyticsMetaData(),
...generateUserSettingsAnalyticsMetaData(),
};
metrics.addTraitsToUser(consolidatedTraits).catch((error) => {
Logger.error(
error as Error,
'AppStateManager: Error adding traits to user',
);
});
const appOpenedEventBuilder = MetricsEventBuilder.createEventBuilder(MetaMetricsEvents.APP_OPENED);
if (attribution) {
const { attributionId, utm, ...utmParams } = attribution;
Expand All @@ -57,7 +72,7 @@ export class AppStateEventListener {
);
appOpenedEventBuilder.addProperties({ attributionId, ...utmParams });
}
MetaMetrics.getInstance().trackEvent(appOpenedEventBuilder.build());
metrics.trackEvent(appOpenedEventBuilder.build());
} catch (error) {
Logger.error(
error as Error,
Expand Down

0 comments on commit 711a583

Please sign in to comment.