Skip to content

Commit 9a297b4

Browse files
ieowieow
authored
feat: SL-312: simplified metric for social login (MetaMask#22469)
<!-- Please submit this PR as a draft initially. Do not mark it as "Ready for review" until the template has been completely filled out, and PR status checks have passed at least once. --> ## **Description** Due to previous implementation to have social login to default enable metametric collection, it is made the metametric logic more complex and hard to debug. This PR is to simplify the implementation The new metametric will always - enabled for social login user - disable for srp user ( user will be prompted meta metricscreen for consent once wallet is created) This PR also implement migration from `social login metametric flag` that is used before to metametric flag so that existing social login user that updated metamask app will enable metametic flag Old SRP behavior metametric flag is persisted after wallet reseted and create new SRP wallet New Behaviour metametric flag will be default to false when user created SRP wallet ( even if enabled before wallet reset) <!-- Write a short description of the changes included in this pull request, also include relevant motivation and context. Have in mind the following questions: 1. What is the reason for the change? 2. What is the improvement/solution? --> ## **Changelog** <!-- If this PR is not End-User-Facing and should not show up in the CHANGELOG, you can choose to either: 1. Write `CHANGELOG entry: null` 2. Label with `no-changelog` If this PR is End-User-Facing, please write a short User-Facing description in the past tense like: `CHANGELOG entry: Added a new tab for users to see their NFTs` `CHANGELOG entry: Fixed a bug that was causing some NFTs to flicker` (This helps the Release Engineer do their job more quickly and accurately) --> CHANGELOG entry: Social Login user create wallet will always enable metametric flag CHANGELOG entry: SRP user create wallet will always disable metametric flag by default and prompted metametric screen ## **Related issues** Fixes: https://consensyssoftware.atlassian.net/browse/SL-312 ## **Manual testing steps** ```gherkin Feature: my feature name Scenario: user create new wallet Given create wallet with social login Then metametric will be enabled Scenario: user create new wallet Given create wallet with SRP wallet Then metametric will be disabled by default and prompt metamteric screen when wallet created ``` ## **Screenshots/Recordings** <!-- If applicable, add screenshots and/or recordings to visualize the before and after of your change. --> ### **Before** <!-- [screenshots/recordings] --> ### **After** <!-- [screenshots/recordings] --> https://github.com/user-attachments/assets/3fefdc27-4ab7-47bf-817b-a5c8b0fc89c3 https://github.com/user-attachments/assets/e2e1be10-d8fd-427c-9d83-88cc42e109dc ## **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. <!-- CURSOR_SUMMARY --> --- > [!NOTE] > Replaces social-login-specific metrics toggle with a single MetaMetrics.enable flow, migrates prior social-login opt-in to standard opt-in, and updates onboarding, settings, tracing, and tests accordingly. > > - **Analytics/Core** > - Remove `enableSocialLogin` and related state from `MetaMetrics`; `isEnabled` now reflects only `enable` state. > - Update `IMetaMetrics`/hook types and `useMetrics` to expose only `enable(...)`. > - Simplify ID retrieval tests and adjust call order expectations. > - **Onboarding** (`app/components/Views/Onboarding`) > - Use `metrics.enable(false)` for SRP create/import and `metrics.enable(true)` after successful OAuth. > - Align tests to expect `enable(true|false)` calls. > - **Settings** (`MetaMetricsAndDataCollectionSection`) > - Replace social-login-specific enabling with `enable(...)` for toggles; keep marketing sync and traits updates. > - Update tests to reflect new enable behavior across SRP/social flows. > - **Tracing** (`util/trace.ts`) > - Consent check now only reads `METRICS_OPT_IN`; remove social-login opt-in fallback. > - **Migration** > - Add migration `108`: if `METRICS_OPT_IN_SOCIAL_LOGIN === AGREED` and `METRICS_OPT_IN !== AGREED`, set `METRICS_OPT_IN` to `AGREED` and remove the social-login key. > - Register migration and add comprehensive tests. > - **Tests** > - Widespread updates removing `enableSocialLogin` mocks/assertions; add new assertions for `enable(...)` and migration behavior. > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit bc4a6aa. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY -->
1 parent 0de9932 commit 9a297b4

15 files changed

Lines changed: 210 additions & 370 deletions

File tree

‎app/components/Views/Onboarding/index.test.tsx‎

Lines changed: 4 additions & 115 deletions
Original file line numberDiff line numberDiff line change
@@ -115,8 +115,6 @@ jest.mock('../../../core/OAuthService/OAuthService', () => ({
115115
accountName: 'test@example.com',
116116
}),
117117
resetOauthState: jest.fn(),
118-
getMetricStateBeforeOauth: jest.fn().mockReturnValue(false),
119-
setMetricStateBeforeOauth: jest.fn(),
120118
localState: {
121119
metricStateBeforeOauth: null,
122120
loginInProgress: false,
@@ -128,6 +126,7 @@ jest.mock('../../../core/OAuthService/OAuthService', () => ({
128126

129127
jest.mock('../../../store/storage-wrapper', () => ({
130128
getItem: jest.fn(),
129+
setItem: jest.fn(),
131130
}));
132131

133132
jest.mock('../../../core', () => ({
@@ -148,7 +147,6 @@ jest.mock('../../../util/trace', () => ({
148147
const mockMetricsIsEnabled = jest.fn().mockReturnValue(false);
149148
const mockTrackEvent = jest.fn();
150149
const mockEnable = jest.fn();
151-
const mockEnableSocialLogin = jest.fn();
152150
const mockCreateEventBuilder = jest.fn().mockReturnValue({
153151
addProperties: jest.fn().mockReturnThis(),
154152
build: jest.fn().mockReturnValue({}),
@@ -158,7 +156,6 @@ jest.mock('../../../core/Analytics/MetaMetrics', () => ({
158156
isEnabled: mockMetricsIsEnabled,
159157
trackEvent: mockTrackEvent,
160158
enable: mockEnable,
161-
enableSocialLogin: mockEnableSocialLogin,
162159
createEventBuilder: mockCreateEventBuilder,
163160
}),
164161
}));
@@ -173,7 +170,6 @@ interface MetricsProps {
173170
isEnabled: () => boolean;
174171
trackEvent: (...args: unknown[]) => void;
175172
enable: (...args: unknown[]) => void;
176-
enableSocialLogin: (...args: unknown[]) => void;
177173
createEventBuilder: () => EventBuilder;
178174
};
179175
}
@@ -189,7 +185,6 @@ jest.mock(
189185
isEnabled: mockMetricsIsEnabled,
190186
trackEvent: mockTrackEvent,
191187
enable: mockEnable,
192-
enableSocialLogin: mockEnableSocialLogin,
193188
createEventBuilder: mockCreateEventBuilder,
194189
}}
195190
/>
@@ -259,7 +254,6 @@ describe('Onboarding', () => {
259254
beforeEach(() => {
260255
jest.clearAllMocks();
261256
mockEnable.mockClear();
262-
mockEnableSocialLogin.mockClear();
263257
mockCreateEventBuilder.mockClear();
264258

265259
jest.spyOn(BackHandler, 'addEventListener').mockImplementation(() => ({
@@ -582,6 +576,8 @@ describe('Onboarding', () => {
582576
onboardingTraceCtx: expect.any(Object),
583577
}),
584578
);
579+
580+
expect(mockEnable).toHaveBeenCalledWith(false);
585581
});
586582
});
587583

@@ -1233,7 +1229,6 @@ describe('Onboarding', () => {
12331229
existingUser: false,
12341230
accountName: 'test@example.com',
12351231
});
1236-
mockEnableSocialLogin.mockClear();
12371232

12381233
const { getByTestId } = renderScreen(
12391234
Onboarding,
@@ -1262,113 +1257,7 @@ describe('Onboarding', () => {
12621257
await googleOAuthFunction(true);
12631258
});
12641259

1265-
expect(mockEnableSocialLogin).toHaveBeenCalledWith(true);
1266-
});
1267-
});
1268-
1269-
describe('Metrics Enable/Disable Tests', () => {
1270-
const mockOAuthService = jest.requireMock(
1271-
'../../../core/OAuthService/OAuthService',
1272-
).default;
1273-
1274-
beforeEach(() => {
1275-
mockSeedlessOnboardingEnabled.mockReturnValue(false);
1276-
(StorageWrapper.getItem as jest.Mock).mockResolvedValue(null);
1277-
mockOAuthService.getMetricStateBeforeOauth.mockReturnValue(false);
1278-
});
1279-
1280-
afterEach(() => {
1281-
jest.clearAllMocks();
1282-
mockSeedlessOnboardingEnabled.mockReset();
1283-
mockOAuthService.getMetricStateBeforeOauth.mockReturnValue(false);
1284-
});
1285-
1286-
it('disables social login metrics when non-OAuth user creates wallet', async () => {
1287-
mockOAuthService.getMetricStateBeforeOauth.mockReturnValue(false);
1288-
mockEnableSocialLogin.mockClear();
1289-
1290-
const { getByTestId } = renderScreen(
1291-
Onboarding,
1292-
{ name: 'Onboarding' },
1293-
{
1294-
state: mockInitialState,
1295-
},
1296-
);
1297-
1298-
const createWalletButton = getByTestId(
1299-
OnboardingSelectorIDs.NEW_WALLET_BUTTON,
1300-
);
1301-
await act(async () => {
1302-
fireEvent.press(createWalletButton);
1303-
});
1304-
1305-
expect(mockEnableSocialLogin).toHaveBeenCalledWith(false);
1306-
});
1307-
1308-
it('disables social login metrics when non-OAuth user imports wallet', async () => {
1309-
mockOAuthService.getMetricStateBeforeOauth.mockReturnValue(false);
1310-
mockEnableSocialLogin.mockClear();
1311-
1312-
const { getByTestId } = renderScreen(
1313-
Onboarding,
1314-
{ name: 'Onboarding' },
1315-
{
1316-
state: mockInitialState,
1317-
},
1318-
);
1319-
1320-
const importWalletButton = getByTestId(
1321-
OnboardingSelectorIDs.EXISTING_WALLET_BUTTON,
1322-
);
1323-
await act(async () => {
1324-
fireEvent.press(importWalletButton);
1325-
});
1326-
1327-
expect(mockEnableSocialLogin).toHaveBeenCalledWith(false);
1328-
});
1329-
1330-
it('disables social login metrics when OAuth user creates wallet', async () => {
1331-
mockOAuthService.getMetricStateBeforeOauth.mockReturnValue(true);
1332-
mockEnableSocialLogin.mockClear();
1333-
1334-
const { getByTestId } = renderScreen(
1335-
Onboarding,
1336-
{ name: 'Onboarding' },
1337-
{
1338-
state: mockInitialState,
1339-
},
1340-
);
1341-
1342-
const createWalletButton = getByTestId(
1343-
OnboardingSelectorIDs.NEW_WALLET_BUTTON,
1344-
);
1345-
await act(async () => {
1346-
fireEvent.press(createWalletButton);
1347-
});
1348-
1349-
expect(mockEnableSocialLogin).toHaveBeenCalledWith(false);
1350-
});
1351-
1352-
it('disables social login metrics when OAuth user imports wallet', async () => {
1353-
mockOAuthService.getMetricStateBeforeOauth.mockReturnValue(true);
1354-
mockEnableSocialLogin.mockClear();
1355-
1356-
const { getByTestId } = renderScreen(
1357-
Onboarding,
1358-
{ name: 'Onboarding' },
1359-
{
1360-
state: mockInitialState,
1361-
},
1362-
);
1363-
1364-
const importWalletButton = getByTestId(
1365-
OnboardingSelectorIDs.EXISTING_WALLET_BUTTON,
1366-
);
1367-
await act(async () => {
1368-
fireEvent.press(importWalletButton);
1369-
});
1370-
1371-
expect(mockEnableSocialLogin).toHaveBeenCalledWith(false);
1260+
expect(mockEnable).toHaveBeenCalledWith(true);
13721261
});
13731262
});
13741263

‎app/components/Views/Onboarding/index.tsx‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -318,7 +318,7 @@ const Onboarding = () => {
318318
if (SEEDLESS_ONBOARDING_ENABLED) {
319319
OAuthLoginService.resetOauthState();
320320
}
321-
await metrics.enableSocialLogin?.(false);
321+
await metrics.enable(false);
322322
// need to call hasMetricConset to update the cached consent state
323323
await hasMetricsConsent();
324324

@@ -347,7 +347,7 @@ const Onboarding = () => {
347347
if (SEEDLESS_ONBOARDING_ENABLED) {
348348
OAuthLoginService.resetOauthState();
349349
}
350-
await metrics.enableSocialLogin?.(false);
350+
await metrics.enable(false);
351351
await hasMetricsConsent();
352352

353353
const action = async () => {
@@ -584,7 +584,7 @@ const Onboarding = () => {
584584
navigation.navigate('Onboarding');
585585

586586
// Enable metrics for OAuth users
587-
await metrics.enableSocialLogin?.(true);
587+
await metrics.enable(true);
588588
discardBufferedTraces();
589589
await setupSentry();
590590

‎app/components/Views/Settings/SecuritySettings/Sections/MetaMetricsAndDataCollectionSection/MetaMetricsAndDataCollectionSection.test.tsx‎

Lines changed: 3 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,6 @@ jest.mock('../../../../../../util/identity/hooks/useAuthentication', () => ({
6262
const mockMetrics = {
6363
trackEvent: jest.fn(),
6464
enable: jest.fn(() => Promise.resolve()),
65-
enableSocialLogin: jest.fn(() => Promise.resolve()),
6665
addTraitsToUser: jest.fn(() => Promise.resolve()),
6766
isEnabled: jest.fn(() => false),
6867
};
@@ -259,7 +258,6 @@ describe('MetaMetricsAndDataCollectionSection', () => {
259258
expect(metaMetricsSwitch.props.value).toBe(false);
260259
expect(marketingSwitch.props.value).toBe(false);
261260
expect(mockMetrics.enable).toHaveBeenCalledWith(false);
262-
expect(mockMetrics.enableSocialLogin).not.toHaveBeenCalled();
263261
expect(mockAlert).toHaveBeenCalled();
264262
expect(mockMetrics.addTraitsToUser).not.toHaveBeenCalled();
265263
expect(mockMetrics.trackEvent).not.toHaveBeenCalled();
@@ -295,7 +293,6 @@ describe('MetaMetricsAndDataCollectionSection', () => {
295293
expect(metaMetricsSwitch.props.value).toBe(true);
296294
expect(marketingSwitch.props.value).toBe(false);
297295
expect(mockMetrics.enable).toHaveBeenCalledWith();
298-
expect(mockMetrics.enableSocialLogin).not.toHaveBeenCalled();
299296
expect(mockAlert).not.toHaveBeenCalled();
300297
expect(mockMetrics.addTraitsToUser).toHaveBeenCalledWith({
301298
deviceProp: 'Device value',
@@ -338,8 +335,7 @@ describe('MetaMetricsAndDataCollectionSection', () => {
338335
fireEvent(metaMetricsSwitch, 'valueChange', true);
339336

340337
await waitFor(() => {
341-
expect(mockMetrics.enable).not.toHaveBeenCalledWith();
342-
expect(mockMetrics.enableSocialLogin).toHaveBeenCalledWith(true);
338+
expect(mockMetrics.enable).toHaveBeenCalled();
343339
expect(mockAlert).not.toHaveBeenCalled();
344340
expect(mockMetrics.addTraitsToUser).toHaveBeenCalled();
345341
expect(mockMetrics.trackEvent).toHaveBeenCalled();
@@ -374,8 +370,7 @@ describe('MetaMetricsAndDataCollectionSection', () => {
374370
await waitFor(() => {
375371
expect(metaMetricsSwitch.props.value).toBe(false);
376372
expect(marketingSwitch.props.value).toBe(false);
377-
expect(mockMetrics.enable).not.toHaveBeenCalledWith(false);
378-
expect(mockMetrics.enableSocialLogin).toHaveBeenCalledWith(false);
373+
expect(mockMetrics.enable).toHaveBeenCalledWith(false);
379374
expect(mockAlert).toHaveBeenCalled();
380375
expect(mockMetrics.addTraitsToUser).not.toHaveBeenCalled();
381376
expect(mockMetrics.trackEvent).not.toHaveBeenCalled();
@@ -410,8 +405,7 @@ describe('MetaMetricsAndDataCollectionSection', () => {
410405
await waitFor(() => {
411406
expect(metaMetricsSwitch.props.value).toBe(true);
412407
expect(marketingSwitch.props.value).toBe(false);
413-
expect(mockMetrics.enable).not.toHaveBeenCalledWith();
414-
expect(mockMetrics.enableSocialLogin).toHaveBeenCalledWith(true);
408+
expect(mockMetrics.enable).toHaveBeenCalled();
415409
expect(mockAlert).not.toHaveBeenCalled();
416410
expect(mockMetrics.addTraitsToUser).toHaveBeenCalled();
417411
expect(mockMetrics.trackEvent).toHaveBeenCalled();
@@ -591,7 +585,6 @@ describe('MetaMetricsAndDataCollectionSection', () => {
591585
expect(marketingSwitch.props.value).toBe(false);
592586
expect(metaMetricsSwitch.props.value).toBe(true);
593587
expect(mockMetrics.enable).not.toHaveBeenCalled();
594-
expect(mockMetrics.enableSocialLogin).not.toHaveBeenCalled();
595588
expect(mockAlert).not.toHaveBeenCalled();
596589
expect(mockMetrics.addTraitsToUser).toHaveBeenCalledTimes(1);
597590
expect(mockMetrics.addTraitsToUser).toHaveBeenCalledWith({

‎app/components/Views/Settings/SecuritySettings/Sections/MetaMetricsAndDataCollectionSection/MetaMetricsAndDataCollectionSection.tsx‎

Lines changed: 5 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -43,14 +43,8 @@ const MetaMetricsAndDataCollectionSection: React.FC<
4343
const theme = useTheme();
4444
const { colors } = theme;
4545
const styles = createStyles(colors);
46-
const {
47-
trackEvent,
48-
enable,
49-
addTraitsToUser,
50-
isEnabled,
51-
createEventBuilder,
52-
enableSocialLogin,
53-
} = useMetrics();
46+
const { trackEvent, enable, addTraitsToUser, isEnabled, createEventBuilder } =
47+
useMetrics();
5448
const [analyticsEnabled, setAnalyticsEnabled] = useState(false);
5549
const dispatch = useDispatch();
5650
const isDataCollectionForMarketingEnabled = useSelector(
@@ -71,11 +65,7 @@ const MetaMetricsAndDataCollectionSection: React.FC<
7165

7266
useEffect(() => {
7367
if (!isBasicFunctionalityEnabled) {
74-
if (isSeedlessOnboardingLoginFlow && enableSocialLogin) {
75-
enableSocialLogin(false);
76-
} else {
77-
enable(false);
78-
}
68+
enable(false);
7969
setAnalyticsEnabled(false);
8070
dispatch(setDataCollectionForMarketing(false));
8171
return;
@@ -99,7 +89,6 @@ const MetaMetricsAndDataCollectionSection: React.FC<
9989
setAnalyticsEnabled,
10090
isEnabled,
10191
enable,
102-
enableSocialLogin,
10392
autoSignIn,
10493
isBasicFunctionalityEnabled,
10594
dispatch,
@@ -112,11 +101,7 @@ const MetaMetricsAndDataCollectionSection: React.FC<
112101
...generateDeviceAnalyticsMetaData(),
113102
...generateUserSettingsAnalyticsMetaData(),
114103
};
115-
if (isSeedlessOnboardingLoginFlow && enableSocialLogin) {
116-
await enableSocialLogin(true);
117-
} else {
118-
await enable();
119-
}
104+
await enable();
120105

121106
setAnalyticsEnabled(true);
122107

@@ -133,11 +118,7 @@ const MetaMetricsAndDataCollectionSection: React.FC<
133118
);
134119
});
135120
} else {
136-
if (isSeedlessOnboardingLoginFlow && enableSocialLogin) {
137-
await enableSocialLogin(false);
138-
} else {
139-
await enable(false);
140-
}
121+
await enable(false);
141122
setAnalyticsEnabled(false);
142123
if (isDataCollectionForMarketingEnabled) {
143124
dispatch(setDataCollectionForMarketing(false));

‎app/components/hooks/useMetrics/useMetrics.test.tsx‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,6 @@ const expectedDataDeleteRegulationId = 'TWV0YU1hc2t1c2Vzbm9wb2ludCE';
3434
const mockMetrics = {
3535
trackEvent: jest.fn(),
3636
enable: jest.fn(() => Promise.resolve()),
37-
enableSocialLogin: jest.fn(() => Promise.resolve()),
3837
addTraitsToUser: jest.fn(() => Promise.resolve()),
3938
createDataDeletionTask: jest.fn(() =>
4039
Promise.resolve(expectedDataDeletionTaskResponse),
@@ -81,7 +80,6 @@ describe('useMetrics', () => {
8180
"createDataDeletionTask": [MockFunction],
8281
"createEventBuilder": [MockFunction],
8382
"enable": [MockFunction],
84-
"enableSocialLogin": [MockFunction],
8583
"getDeleteRegulationCreationDate": [MockFunction],
8684
"getDeleteRegulationId": [MockFunction],
8785
"getMetaMetricsId": [MockFunction],

‎app/components/hooks/useMetrics/useMetrics.ts‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,6 @@ const useMetrics = (): IUseMetricsHook =>
9393
isEnabled: MetaMetrics.getInstance().isEnabled,
9494
getMetaMetricsId: MetaMetrics.getInstance().getMetaMetricsId,
9595
createEventBuilder: MetricsEventBuilder.createEventBuilder,
96-
enableSocialLogin: MetaMetrics.getInstance().enableSocialLogin,
9796
}),
9897
[],
9998
);

‎app/components/hooks/useMetrics/useMetrics.types.ts‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,5 +35,4 @@ export interface IUseMetricsHook {
3535
getMetaMetricsId(): Promise<string | undefined>;
3636
createEventBuilder(event: IMetaMetricsEvent): MetricsEventBuilder;
3737
// Temporary workaround to avoid breaking all the tests that mock the useMetrics hook method
38-
enableSocialLogin?: (enable?: boolean) => Promise<void>;
3938
}

0 commit comments

Comments
 (0)