Skip to content

Commit

Permalink
feat: add account_type/snap_id for buy/send metrics (#28011)
Browse files Browse the repository at this point in the history
## **Description**

Adds more context/info to the Send/Buy events for both Ethereum and
Bitcoin overview screens.

[![Open in GitHub
Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/28011?quickstart=1)

## **Related issues**

Fixes: MetaMask/accounts-planning#636,
MetaMask/accounts-planning#637

## **Manual testing steps**

- Cherry-pick this commit: f938d6d
* This will allow you to use the verbose mode for the local segment
server
- `node development/mock-segment.js -v`
- `yarn start:flask`
- Create a Bitcoin account (enable support from the experimental
settings)
- Select a Bitcoin account
- Click on "Buy" button using your Bitcoin account
- Click on "Send" button using your Bitcoin account
- Create an Ethereum account
- Click on "Buy" button using your Bitcoin account
- Click on "Send" button using your Bitcoin account

You should see those events from the `mock-segment.js` server output:
```
...
[mock-segment]: Events received: Buy Button Clicked {
  "account_type": "bip122:p2wpkh",
  "location": "Home",
  "text": "Buy",
  "chain_id": "bip122:000000000019d6689c085ae165831e93",
  "snap_id": "npm:@metamask/bitcoin-wallet-snap",
  "category": "Navigation",
  "locale": "en",
  "environment_type": "popup"
}
...
[mock-segment]: Events received: Send Button Clicked {
  "account_type": "bip122:p2wpkh",
  "token_symbol": "BTC",
  "location": "Home",
  "text": "Send",
  "chain_id": "bip122:000000000019d6689c085ae165831e93",
  "snap_id": "npm:@metamask/bitcoin-wallet-snap",
  "category": "Navigation",
  "locale": "en",
  "environment_type": "popup"
}
...
[mock-segment]: Events received: Buy Button Clicked {
  "account_type": "eip155:eoa",
  "location": "Home",
  "text": "Buy",
  "chain_id": "0x1",
  "token_symbol": {
    "symbol": "ETH",
    "name": "Ether",
    "address": "0x0000000000000000000000000000000000000000",
    "decimals": 18,
    "iconUrl": "./images/eth_logo.svg",
    "balance": "0",
    "string": "0"
  },
  "category": "Navigation",
  "locale": "en",
  "environment_type": "popup"
}
...
[mock-segment]: Events received: Send Button Clicked {
  "account_type": "eip155:eoa",
  "token_symbol": "ETH",
  "location": "Home",
  "text": "Send",
  "chain_id": "0x1",
  "category": "Navigation",
  "locale": "en",
  "environment_type": "popup"
}
...
```

## **Screenshots/Recordings**

### **Before**

### **After**

## **Pre-merge author checklist**

- [x] I've followed [MetaMask Contributor
Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask
Extension Coding
Standards](https://github.com/MetaMask/metamask-extension/blob/develop/.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-extension/blob/develop/.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
ccharly authored Nov 12, 2024
1 parent e54f555 commit 2b7101d
Show file tree
Hide file tree
Showing 8 changed files with 323 additions and 76 deletions.
90 changes: 90 additions & 0 deletions ui/components/app/wallet-overview/btc-overview.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -11,14 +11,33 @@ import { MultichainNetworks } from '../../../../shared/constants/multichain/netw
import { RampsMetaMaskEntry } from '../../../hooks/ramps/useRamps/useRamps';
import { defaultBuyableChains } from '../../../ducks/ramps/constants';
import { setBackgroundConnection } from '../../../store/background-connection';
import { MetaMetricsContext } from '../../../contexts/metametrics';
import {
MetaMetricsEventCategory,
MetaMetricsEventName,
} from '../../../../shared/constants/metametrics';
import BtcOverview from './btc-overview';

// We need to mock `dispatch` since we use it for `setDefaultHomeActiveTabName`.
const mockDispatch = jest.fn().mockReturnValue(() => jest.fn());
jest.mock('react-redux', () => ({
...jest.requireActual('react-redux'),
useDispatch: () => mockDispatch,
}));

jest.mock('../../../store/actions', () => ({
handleSnapRequest: jest.fn(),
sendMultichainTransaction: jest.fn(),
setDefaultHomeActiveTabName: jest.fn(),
}));

const PORTOFOLIO_URL = 'https://portfolio.test';

const BTC_OVERVIEW_BUY = 'coin-overview-buy';
const BTC_OVERVIEW_BRIDGE = 'coin-overview-bridge';
const BTC_OVERVIEW_RECEIVE = 'coin-overview-receive';
const BTC_OVERVIEW_SWAP = 'token-overview-button-swap';
const BTC_OVERVIEW_SEND = 'coin-overview-send';
const BTC_OVERVIEW_PRIMARY_CURRENCY = 'coin-overview__primary-currency';

const mockMetaMetricsId = 'deadbeef';
Expand Down Expand Up @@ -228,6 +247,39 @@ describe('BtcOverview', () => {
});
});

it('sends an event when clicking the Buy button', () => {
const storeWithBtcBuyable = getStore({
ramps: {
buyableChains: mockBuyableChainsWithBtc,
},
});

const mockTrackEvent = jest.fn();
const { queryByTestId } = renderWithProvider(
<MetaMetricsContext.Provider value={mockTrackEvent}>
<BtcOverview />
</MetaMetricsContext.Provider>,
storeWithBtcBuyable,
);

const buyButton = queryByTestId(BTC_OVERVIEW_BUY);
expect(buyButton).toBeInTheDocument();
expect(buyButton).not.toBeDisabled();
fireEvent.click(buyButton as HTMLElement);

expect(mockTrackEvent).toHaveBeenCalledWith({
event: MetaMetricsEventName.NavBuyButtonClicked,
category: MetaMetricsEventCategory.Navigation,
properties: {
account_type: mockNonEvmAccount.type,
chain_id: MultichainNetworks.BITCOIN,
location: 'Home',
snap_id: mockNonEvmAccount.metadata.snap.id,
text: 'Buy',
},
});
});

it('always show the Receive button', () => {
const { queryByTestId } = renderWithProvider(<BtcOverview />, getStore());
const receiveButton = queryByTestId(BTC_OVERVIEW_RECEIVE);
Expand Down Expand Up @@ -263,4 +315,42 @@ describe('BtcOverview', () => {
expect(buyButton).toBeInTheDocument();
expect(buyButton).toBeDisabled();
});

it('always show the Send button', () => {
const { queryByTestId } = renderWithProvider(<BtcOverview />, getStore());
const sendButton = queryByTestId(BTC_OVERVIEW_SEND);
expect(sendButton).toBeInTheDocument();
expect(sendButton).not.toBeDisabled();
});

it('sends an event when clicking the Send button', () => {
const mockTrackEvent = jest.fn();
const { queryByTestId } = renderWithProvider(
<MetaMetricsContext.Provider value={mockTrackEvent}>
<BtcOverview />
</MetaMetricsContext.Provider>,
getStore(),
);

const sendButton = queryByTestId(BTC_OVERVIEW_SEND);
expect(sendButton).toBeInTheDocument();
expect(sendButton).not.toBeDisabled();
fireEvent.click(sendButton as HTMLElement);

expect(mockTrackEvent).toHaveBeenCalledWith(
{
event: MetaMetricsEventName.NavSendButtonClicked,
category: MetaMetricsEventCategory.Navigation,
properties: {
account_type: mockNonEvmAccount.type,
chain_id: MultichainNetworks.BITCOIN,
location: 'Home',
snap_id: mockNonEvmAccount.metadata.snap.id,
text: 'Send',
token_symbol: 'BTC',
},
},
expect.any(Object),
);
});
});
7 changes: 4 additions & 3 deletions ui/components/app/wallet-overview/btc-overview.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,9 @@ import {
} from '../../../selectors/multichain';
///: BEGIN:ONLY_INCLUDE_IF(build-main,build-beta,build-flask)
import { getIsBitcoinBuyable } from '../../../ducks/ramps';
import { getSelectedInternalAccount } from '../../../selectors';
import { useMultichainSelector } from '../../../hooks/useMultichainSelector';
///: END:ONLY_INCLUDE_IF
import { getSelectedInternalAccount } from '../../../selectors';
import { CoinOverview } from './coin-overview';

type BtcOverviewProps = {
Expand All @@ -21,17 +21,18 @@ type BtcOverviewProps = {
const BtcOverview = ({ className }: BtcOverviewProps) => {
const { chainId } = useSelector(getMultichainProviderConfig);
const balance = useSelector(getMultichainSelectedAccountCachedBalance);
const account = useSelector(getSelectedInternalAccount);
///: BEGIN:ONLY_INCLUDE_IF(build-main,build-beta,build-flask)
const selectedAccount = useSelector(getSelectedInternalAccount);
const isBtcMainnetAccount = useMultichainSelector(
getMultichainIsMainnet,
selectedAccount,
account,
);
const isBtcBuyable = useSelector(getIsBitcoinBuyable);
///: END:ONLY_INCLUDE_IF

return (
<CoinOverview
account={account}
balance={balance}
// We turn this off to avoid having that asterisk + the "Balance maybe be outdated" message for now
balanceIsCached={false}
Expand Down
4 changes: 4 additions & 0 deletions ui/components/app/wallet-overview/coin-buttons.stories.js
Original file line number Diff line number Diff line change
@@ -1,9 +1,13 @@
import React from 'react';
import testData from '../../../../.storybook/test-data';
import CoinButtons from './coin-buttons';

const { accounts, selectedAccount } = testData.metamask.internalAccounts;

export default {
title: 'Components/App/WalletOverview/CoinButtons',
args: {
account: accounts[selectedAccount],
chainId: '1',
trackingLocation: 'home',
isSwapsChain: true,
Expand Down
96 changes: 67 additions & 29 deletions ui/components/app/wallet-overview/coin-buttons.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ import {
} from '@metamask/utils';

///: BEGIN:ONLY_INCLUDE_IF(build-flask)
import { BtcAccountType } from '@metamask/keyring-api';
import { BtcAccountType, InternalAccount } from '@metamask/keyring-api';
///: END:ONLY_INCLUDE_IF
///: BEGIN:ONLY_INCLUDE_IF(build-main,build-beta,build-flask)
import { ChainId } from '../../../../shared/constants/network';
Expand All @@ -51,7 +51,6 @@ import {
getCurrentKeyring,
///: END:ONLY_INCLUDE_IF
getUseExternalServices,
getSelectedAccount,
///: BEGIN:ONLY_INCLUDE_IF(build-flask)
getMemoizedUnapprovedTemplatedConfirmations,
///: END:ONLY_INCLUDE_IF
Expand Down Expand Up @@ -90,8 +89,29 @@ import {
} from '../../../store/actions';
import { BITCOIN_WALLET_SNAP_ID } from '../../../../shared/lib/accounts/bitcoin-wallet-snap';
///: END:ONLY_INCLUDE_IF
import {
getMultichainIsEvm,
getMultichainNativeCurrency,
} from '../../../selectors/multichain';
import { useMultichainSelector } from '../../../hooks/useMultichainSelector';

type CoinButtonsProps = {
account: InternalAccount;
chainId: `0x${string}` | CaipChainId | number;
trackingLocation: string;
isSwapsChain: boolean;
isSigningEnabled: boolean;
///: BEGIN:ONLY_INCLUDE_IF(build-main,build-beta,build-flask)
isBridgeChain: boolean;
isBuyableChain: boolean;
defaultSwapsToken?: SwapsEthToken;
///: END:ONLY_INCLUDE_IF
classPrefix?: string;
iconButtonClassName?: string;
};

const CoinButtons = ({
account,
chainId,
trackingLocation,
isSwapsChain,
Expand All @@ -103,26 +123,13 @@ const CoinButtons = ({
///: END:ONLY_INCLUDE_IF
classPrefix = 'coin',
iconButtonClassName = '',
}: {
chainId: `0x${string}` | CaipChainId | number;
trackingLocation: string;
isSwapsChain: boolean;
isSigningEnabled: boolean;
///: BEGIN:ONLY_INCLUDE_IF(build-main,build-beta,build-flask)
isBridgeChain: boolean;
isBuyableChain: boolean;
defaultSwapsToken?: SwapsEthToken;
///: END:ONLY_INCLUDE_IF
classPrefix?: string;
iconButtonClassName?: string;
}) => {
}: CoinButtonsProps) => {
const t = useContext(I18nContext);
const dispatch = useDispatch();

const trackEvent = useContext(MetaMetricsContext);
const [showReceiveModal, setShowReceiveModal] = useState(false);

const account = useSelector(getSelectedAccount);
const { address: selectedAddress } = account;
const history = useHistory();
///: BEGIN:ONLY_INCLUDE_IF(build-main,build-beta,build-flask)
Expand All @@ -131,6 +138,16 @@ const CoinButtons = ({
const usingHardwareWallet = isHardwareKeyring(keyring?.type);
///: END:ONLY_INCLUDE_IF

// Initially, those events were using a "ETH" as `token_symbol`, so we keep this behavior
// for EVM, no matter the currently selected native token (e.g. SepoliaETH if you are on Sepolia
// network).
const isEvm = useMultichainSelector(getMultichainIsEvm, account);
const multichainNativeToken = useMultichainSelector(
getMultichainNativeCurrency,
account,
);
const nativeToken = isEvm ? 'ETH' : multichainNativeToken;

const isExternalServicesEnabled = useSelector(getUseExternalServices);

const buttonTooltips = {
Expand Down Expand Up @@ -180,6 +197,23 @@ const CoinButtons = ({
};
///: END:ONLY_INCLUDE_IF

const getSnapAccountMetaMetricsPropertiesIfAny = (
internalAccount: InternalAccount,
): { snap_id?: string } => {
// Some accounts might be Snap accounts, in this case we add some extra properties
// to the metrics:
const snapId = internalAccount.metadata.snap?.id;
if (snapId) {
return {
snap_id: snapId,
};
}

// If the account is not a Snap account or that we could not get the Snap ID for
// some reason, we don't add any extra property.
return {};
};

///: BEGIN:ONLY_INCLUDE_IF(build-mmi)
const mmiPortfolioEnabled = useSelector(getMmiPortfolioEnabled);
const mmiPortfolioUrl = useSelector(getMmiPortfolioUrl);
Expand Down Expand Up @@ -276,6 +310,21 @@ const CoinButtons = ({
///: END:ONLY_INCLUDE_IF

const handleSendOnClick = useCallback(async () => {
trackEvent(
{
event: MetaMetricsEventName.NavSendButtonClicked,
category: MetaMetricsEventCategory.Navigation,
properties: {
account_type: account.type,
token_symbol: nativeToken,
location: 'Home',
text: 'Send',
chain_id: chainId,
...getSnapAccountMetaMetricsPropertiesIfAny(account),
},
},
{ excludeMetaMetricsId: false },
);
switch (account.type) {
///: BEGIN:ONLY_INCLUDE_IF(build-flask)
case BtcAccountType.P2wpkh: {
Expand All @@ -291,19 +340,6 @@ const CoinButtons = ({
}
///: END:ONLY_INCLUDE_IF
default: {
trackEvent(
{
event: MetaMetricsEventName.NavSendButtonClicked,
category: MetaMetricsEventCategory.Navigation,
properties: {
token_symbol: 'ETH',
location: 'Home',
text: 'Send',
chain_id: chainId,
},
},
{ excludeMetaMetricsId: false },
);
await dispatch(startNewDraftTransaction({ type: AssetType.native }));
history.push(SEND_ROUTE);
}
Expand Down Expand Up @@ -358,10 +394,12 @@ const CoinButtons = ({
event: MetaMetricsEventName.NavBuyButtonClicked,
category: MetaMetricsEventCategory.Navigation,
properties: {
account_type: account.type,
location: 'Home',
text: 'Buy',
chain_id: chainId,
token_symbol: defaultSwapsToken,
...getSnapAccountMetaMetricsPropertiesIfAny(account),
},
});
}, [chainId, defaultSwapsToken]);
Expand Down
9 changes: 5 additions & 4 deletions ui/components/app/wallet-overview/coin-overview.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import { zeroAddress } from 'ethereumjs-util';
import { CaipChainId } from '@metamask/utils';
import type { Hex } from '@metamask/utils';

import { InternalAccount } from '@metamask/keyring-api';
import {
Box,
ButtonIcon,
Expand Down Expand Up @@ -45,7 +46,6 @@ import UserPreferencedCurrencyDisplay from '../user-preferenced-currency-display
import { PRIMARY } from '../../../helpers/constants/common';
import {
getPreferences,
getSelectedAccount,
getShouldHideZeroBalanceTokens,
getTokensMarketData,
getIsTestnet,
Expand Down Expand Up @@ -74,6 +74,7 @@ import CoinButtons from './coin-buttons';
import { AggregatedPercentageOverview } from './aggregated-percentage-overview';

export type CoinOverviewProps = {
account: InternalAccount;
balance: string;
balanceIsCached: boolean;
className?: string;
Expand All @@ -90,6 +91,7 @@ export type CoinOverviewProps = {
};

export const CoinOverview = ({
account,
balance,
balanceIsCached,
className,
Expand Down Expand Up @@ -121,7 +123,6 @@ export const CoinOverview = ({

///: END:ONLY_INCLUDE_IF

const account = useSelector(getSelectedAccount);
const showNativeTokenAsMainBalanceRoute = getSpecificSettingsRoute(
t,
t('general'),
Expand All @@ -135,12 +136,11 @@ export const CoinOverview = ({
const { showFiatInTestnets, privacyMode, showNativeTokenAsMainBalance } =
useSelector(getPreferences);

const selectedAccount = useSelector(getSelectedAccount);
const shouldHideZeroBalanceTokens = useSelector(
getShouldHideZeroBalanceTokens,
);
const { totalFiatBalance, loading } = useAccountTotalFiatBalance(
selectedAccount,
account,
shouldHideZeroBalanceTokens,
);

Expand Down Expand Up @@ -368,6 +368,7 @@ export const CoinOverview = ({
buttons={
<CoinButtons
{...{
account,
trackingLocation: 'home',
chainId,
isSwapsChain,
Expand Down
Loading

0 comments on commit 2b7101d

Please sign in to comment.