Skip to content

Commit

Permalink
fix: trackevent enabled is undefined (#12180)
Browse files Browse the repository at this point in the history
<!--
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**

> [!IMPORTANT]
> The issue is due to the fact that `MetaMetrics.trackEvent` method has
multiple signatures to handle the backward compatibility with old ways
to call it.
> After discussion (see the change history of this description), we
decided to remove backward compatibility and the multiple signature
system. We are going to simplify and then fix all the `trackEvent`
calls.
Asside from the changes in `MetaMetrics` and `useMetrics` hook (and
tests) all the changes should be on the calls of `trackEvent`.

### Current changes in this PR
- removes multiple signature `MetaMetrics.trackEvent`
- updates `MetaMetrics` unit tests
- updates `useMetrics` hook
- updates `useMetrics` hook unit tests
- deletes now useless legacy compatibility utils
- updates all `trackEvent` calls
- updates all unit tests that test `trackEvent` calls

## **Related issues**

Fixes #12117 

## **Manual testing steps**

1. navigate the app
2. check trackEvent is called (check app logs)

## **Screenshots/Recordings**



https://github.com/user-attachments/assets/058f6607-eda1-4eb5-bcae-a774deb13648



https://github.com/user-attachments/assets/fa0816cf-5459-4825-a162-a9a2e87c86ac



### **Before**

N/A

### **After**

N/A

## **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.

---------

Co-authored-by: Frank von Hoven <[email protected]>
Co-authored-by: Frank von Hoven <[email protected]>
  • Loading branch information
3 people authored Nov 25, 2024
1 parent a74aa71 commit 2506358
Show file tree
Hide file tree
Showing 194 changed files with 3,388 additions and 2,514 deletions.
6 changes: 3 additions & 3 deletions app/actions/onboarding/index.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
import { IMetaMetricsEvent } from '../../core/Analytics';
import { ITrackingEvent } from '../../core/Analytics/MetaMetrics.types';

export const SAVE_EVENT = 'SAVE_EVENT';
export const CLEAR_EVENTS = 'CLEAR_EVENTS';

interface SaveEventAction {
type: typeof SAVE_EVENT;
event: [IMetaMetricsEvent];
event: [ITrackingEvent];
}

interface ClearEventsAction {
Expand All @@ -15,7 +15,7 @@ interface ClearEventsAction {
export type OnboardingActionTypes = SaveEventAction | ClearEventsAction;

export function saveOnboardingEvent(
eventArgs: [IMetaMetricsEvent],
eventArgs: [ITrackingEvent],
): SaveEventAction {
return {
type: SAVE_EVENT,
Expand Down
24 changes: 18 additions & 6 deletions app/component-library/components/Navigation/TabBar/TabBar.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ import OnboardingWizard from '../../../../components/UI/OnboardingWizard';

const TabBar = ({ state, descriptors, navigation }: TabBarProps) => {
const { colors } = useTheme();
const { trackEvent } = useMetrics();
const { trackEvent, createEventBuilder } = useMetrics();
const { bottom: bottomInset } = useSafeAreaInsets();
const { styles } = useStyles(styleSheet, { bottomInset });
const chainId = useSelector(selectChainId);
Expand Down Expand Up @@ -73,10 +73,14 @@ const TabBar = ({ state, descriptors, navigation }: TabBarProps) => {
navigation.navigate(Routes.MODAL.ROOT_MODAL_FLOW, {
screen: Routes.MODAL.WALLET_ACTIONS,
});
trackEvent(MetaMetricsEvents.ACTIONS_BUTTON_CLICKED, {
text: '',
chain_id: getDecimalChainId(chainId),
});
trackEvent(
createEventBuilder(MetaMetricsEvents.ACTIONS_BUTTON_CLICKED)
.addProperties({
text: '',
chain_id: getDecimalChainId(chainId),
})
.build(),
);
break;
case Routes.BROWSER_VIEW:
navigation.navigate(Routes.BROWSER.HOME, {
Expand Down Expand Up @@ -124,7 +128,15 @@ const TabBar = ({ state, descriptors, navigation }: TabBarProps) => {
/>
);
},
[state, descriptors, navigation, colors, chainId, trackEvent],
[
state,
descriptors,
navigation,
colors,
chainId,
trackEvent,
createEventBuilder,
],
);

const renderTabBarItems = useCallback(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import { MetaMetricsEvents } from '../../../core/Analytics';
import { backgroundState } from '../../../util/test/initial-root-state';
import { render } from '@testing-library/react-native';
import { useMetrics } from '../../../components/hooks/useMetrics';
import { MetricsEventBuilder } from '../../../core/Analytics/MetricsEventBuilder';

jest.mock('../../Views/confirmations/hooks/useApprovalRequest');
jest.mock('../../../components/hooks/useMetrics');
Expand Down Expand Up @@ -68,22 +69,23 @@ const mockSelectorState = (state: any) => {

const mockTrackEvent = jest.fn();

(useMetrics as jest.MockedFn<typeof useMetrics>).mockReturnValue({
trackEvent: mockTrackEvent,
createEventBuilder: MetricsEventBuilder.createEventBuilder,
enable: jest.fn(),
addTraitsToUser: jest.fn(),
createDataDeletionTask: jest.fn(),
checkDataDeleteStatus: jest.fn(),
getDeleteRegulationCreationDate: jest.fn(),
getDeleteRegulationId: jest.fn(),
isDataRecorded: jest.fn(),
isEnabled: jest.fn(),
getMetaMetricsId: jest.fn(),
});

describe('PermissionApproval', () => {
beforeEach(() => {
jest.resetAllMocks();
(useMetrics as jest.MockedFn<typeof useMetrics>).mockReturnValue({
trackEvent: mockTrackEvent,
createEventBuilder: jest.fn(),
enable: jest.fn(),
addTraitsToUser: jest.fn(),
createDataDeletionTask: jest.fn(),
checkDataDeleteStatus: jest.fn(),
getDeleteRegulationCreationDate: jest.fn(),
getDeleteRegulationId: jest.fn(),
isDataRecorded: jest.fn(),
isEnabled: jest.fn(),
getMetaMetricsId: jest.fn(),
});
jest.clearAllMocks();
});

it('navigates', async () => {
Expand Down Expand Up @@ -143,14 +145,17 @@ describe('PermissionApproval', () => {

render(<PermissionApproval navigation={navigationMock} />);

expect(mockTrackEvent).toHaveBeenCalledTimes(1);
expect(mockTrackEvent).toHaveBeenCalledWith(
const expectedEvent = MetricsEventBuilder.createEventBuilder(
MetaMetricsEvents.CONNECT_REQUEST_STARTED,
{
)
.addProperties({
number_of_accounts: 3,
source: 'PERMISSION SYSTEM',
},
);
})
.build();

expect(mockTrackEvent).toHaveBeenCalledTimes(1);
expect(mockTrackEvent).toHaveBeenCalledWith(expectedEvent);
});

it('does not navigate if no approval request', async () => {
Expand Down
22 changes: 16 additions & 6 deletions app/components/Approvals/PermissionApproval/PermissionApproval.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ export interface PermissionApprovalProps {
}

const PermissionApproval = (props: PermissionApprovalProps) => {
const { trackEvent } = useMetrics();
const { trackEvent, createEventBuilder } = useMetrics();
const { approvalRequest } = useApprovalRequest();
const totalAccounts = useSelector(selectAccountsLength);
const isProcessing = useRef<boolean>(false);
Expand All @@ -38,18 +38,28 @@ const PermissionApproval = (props: PermissionApprovalProps) => {

isProcessing.current = true;

trackEvent(MetaMetricsEvents.CONNECT_REQUEST_STARTED, {
number_of_accounts: totalAccounts,
source: 'PERMISSION SYSTEM',
});
trackEvent(
createEventBuilder(MetaMetricsEvents.CONNECT_REQUEST_STARTED)
.addProperties({
number_of_accounts: totalAccounts,
source: 'PERMISSION SYSTEM',
})
.build(),
);

props.navigation.navigate(
...createAccountConnectNavDetails({
hostInfo: requestData,
permissionRequestId: id,
}),
);
}, [approvalRequest, totalAccounts, props.navigation, trackEvent]);
}, [
approvalRequest,
totalAccounts,
props.navigation,
trackEvent,
createEventBuilder,
]);

return null;
};
Expand Down
44 changes: 30 additions & 14 deletions app/components/Nav/Main/MainNavigator.js
Original file line number Diff line number Diff line change
Expand Up @@ -400,7 +400,7 @@ const SettingsFlow = () => (
);

const HomeTabs = () => {
const { trackEvent } = useMetrics();
const { trackEvent, createEventBuilder } = useMetrics();
const drawerRef = useRef(null);
const [isKeyboardHidden, setIsKeyboardHidden] = useState(true);

Expand Down Expand Up @@ -439,10 +439,14 @@ const HomeTabs = () => {
home: {
tabBarIconKey: TabBarIconKey.Wallet,
callback: () => {
trackEvent(MetaMetricsEvents.WALLET_OPENED, {
number_of_accounts: accountsLength,
chain_id: getDecimalChainId(chainId),
});
trackEvent(
createEventBuilder(MetaMetricsEvents.WALLET_OPENED)
.addProperties({
number_of_accounts: accountsLength,
chain_id: getDecimalChainId(chainId),
})
.build(),
);
},
rootScreenName: Routes.WALLET_VIEW,
},
Expand All @@ -453,27 +457,39 @@ const HomeTabs = () => {
browser: {
tabBarIconKey: TabBarIconKey.Browser,
callback: () => {
trackEvent(MetaMetricsEvents.BROWSER_OPENED, {
number_of_accounts: accountsLength,
chain_id: getDecimalChainId(chainId),
source: 'Navigation Tab',
active_connected_dapp: activeConnectedDapp,
number_of_open_tabs: amountOfBrowserOpenTabs,
});
trackEvent(
createEventBuilder(MetaMetricsEvents.BROWSER_OPENED)
.addProperties({
number_of_accounts: accountsLength,
chain_id: getDecimalChainId(chainId),
source: 'Navigation Tab',
active_connected_dapp: activeConnectedDapp,
number_of_open_tabs: amountOfBrowserOpenTabs,
})
.build(),
);
},
rootScreenName: Routes.BROWSER_VIEW,
},
activity: {
tabBarIconKey: TabBarIconKey.Activity,
callback: () => {
trackEvent(MetaMetricsEvents.NAVIGATION_TAPS_TRANSACTION_HISTORY);
trackEvent(
createEventBuilder(
MetaMetricsEvents.NAVIGATION_TAPS_TRANSACTION_HISTORY,
).build(),
);
},
rootScreenName: Routes.TRANSACTIONS_VIEW,
},
settings: {
tabBarIconKey: TabBarIconKey.Setting,
callback: () => {
trackEvent(MetaMetricsEvents.NAVIGATION_TAPS_SETTINGS);
trackEvent(
createEventBuilder(
MetaMetricsEvents.NAVIGATION_TAPS_SETTINGS,
).build(),
);
},
rootScreenName: Routes.SETTINGS_VIEW,
unmountOnBlur: true,
Expand Down
32 changes: 25 additions & 7 deletions app/components/Nav/Main/RootRPCMethodsUI.js
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ export const useSwapConfirmedEvent = ({ trackSwaps }) => {
};

const RootRPCMethodsUI = (props) => {
const { trackEvent } = useMetrics();
const { trackEvent, createEventBuilder } = useMetrics();
const [transactionModalType, setTransactionModalType] = useState(undefined);
const tokenList = useSelector(selectTokenList);
const setTransactionObject = props.setTransactionObject;
Expand Down Expand Up @@ -244,15 +244,28 @@ const RootRPCMethodsUI = (props) => {

Logger.log('Swaps', 'Sending metrics event', event);

trackEvent(event, { sensitiveProperties: { ...parameters } });
trackEvent(
createEventBuilder(event)
.addSensitiveProperties({ ...parameters })
.build(),
);
} catch (e) {
Logger.error(e, MetaMetricsEvents.SWAP_TRACKING_FAILED);
trackEvent(MetaMetricsEvents.SWAP_TRACKING_FAILED, {
error: e,
});
trackEvent(
createEventBuilder(MetaMetricsEvents.SWAP_TRACKING_FAILED)
.addProperties({
error: e,
})
.build(),
);
}
},
[props.selectedAddress, props.shouldUseSmartTransaction, trackEvent],
[
props.selectedAddress,
props.shouldUseSmartTransaction,
trackEvent,
createEventBuilder,
],
);

const { addTransactionMetaIdForListening } = useSwapConfirmedEvent({
Expand Down Expand Up @@ -326,7 +339,11 @@ const RootRPCMethodsUI = (props) => {
);
Logger.error(error, 'error while trying to send transaction (Main)');
} else {
trackEvent(MetaMetricsEvents.QR_HARDWARE_TRANSACTION_CANCELED);
trackEvent(
createEventBuilder(
MetaMetricsEvents.QR_HARDWARE_TRANSACTION_CANCELED,
).build(),
);
}
}
},
Expand All @@ -336,6 +353,7 @@ const RootRPCMethodsUI = (props) => {
trackEvent,
swapsTransactions,
addTransactionMetaIdForListening,
createEventBuilder,
],
);

Expand Down
Loading

0 comments on commit 2506358

Please sign in to comment.