Skip to content

Commit 50dbb45

Browse files
PatrykLuckaPatrykLucka
authored
refactor: enhance NFT details tracking with source parameter (MetaMask#23110)
## **Description** **What is the reason for the change?** The "NFT Details Opened" Segment event was not tracking the source/origin of where users navigate to NFT details from. This made it impossible to differentiate between users clicking NFTs from the wallet homepage versus the dedicated NFT list page, limiting our ability to analyze user behavior patterns. **What is the improvement/solution?** Added an optional `source` parameter to the NFT Details navigation flow and analytics tracking: - Added `source` property to `NftDetailsParams` and `CollectibleModalParams` types with values: - `'mobile-nft-list'` - when navigating from homepage NFT tab - `'mobile-nft-list-page'` - when navigating from full NFT list page - Updated `NftGrid` component to pass appropriate source based on `isFullView` prop - Updated `NftGridItem` to propagate source parameter - Modified analytics tracking in `NftDetails` and `CollectibleModal` to include source in the "NFT Details Opened" event - Added comprehensive unit tests to verify correct source values are passed and tracked The implementation maintains backward compatibility - the source parameter is optional and existing navigation flows continue to work without it. ## **Changelog** CHANGELOG entry: null ## **Related issues** Fixes: [TMCU-208](https://consensyssoftware.atlassian.net/browse/TMCU-208) ## **Manual testing steps** ```gherkin Feature: NFT Details Source Tracking Scenario: user navigates to NFT details from homepage Given user is on wallet homepage with NFTs displayed When user taps on an NFT in the homepage NFT tab Then NFT Details Opened event is tracked with source: "mobile-nft-list" Scenario: user navigates to NFT details from full NFT list page Given user has tapped "View All NFTs" from homepage And user is on the full NFT list page When user taps on an NFT Then NFT Details Opened event is tracked with source: "mobile-nft-list-page" ``` ## **Screenshots/Recordings** <!-- If applicable, add screenshots and/or recordings to visualize the before and after of your change. --> ### **Before** <!-- [screenshots/recordings] --> ### **After** <!-- [screenshots/recordings] --> ## **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. [TMCU-208]: https://consensyssoftware.atlassian.net/browse/TMCU-208?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ <!-- CURSOR_SUMMARY --> --- > [!NOTE] > Adds an optional `source` param passed from NFT list views to details/modal and includes it in the NFT details opened analytics event, with corresponding tests. > > - **Analytics**: > - Include optional `source` in `MetaMetricsEvents.NFT_DETAILS_OPENED` for `NftDetails` and `CollectibleModal`; effect depends on `chainId` and `source`. > - **Navigation/Types**: > - Add `source?` to `NftDetailsParams` and `CollectibleModalParams`. > - Propagate `source` from navigation: `NftGrid` determines (`mobile-nft-list` vs `mobile-nft-list-page`) and `NftGridItem` passes it to `NftDetails`. > - **UI**: > - `CollectibleModal` reads `source` via `useParams` and tracks it. > - Minor refactor in `NftGrid` to centralize content rendering. > - **Tests**: > - Add/extend tests in `CollectibleModal.test.tsx`, `NftGrid.test.tsx`, and `NftDetails.test.ts` to assert `source` propagation and event properties; update snapshots. > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit 169b734. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY -->
1 parent 99b3e05 commit 50dbb45

11 files changed

Lines changed: 361 additions & 51 deletions

File tree

‎app/components/UI/CollectibleModal/CollectibleModal.test.tsx‎

Lines changed: 136 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import {
1212
} from '../../../selectors/preferencesController';
1313
import { useSelector } from 'react-redux';
1414
import { selectSendRedesignFlags } from '../../../selectors/featureFlagController/confirmations';
15+
import { selectChainId } from '../../../selectors/networkController';
1516
import { MOCK_ACCOUNTS_CONTROLLER_STATE } from '../../../util/test/accountsControllerTestUtils';
1617
import { mockNetworkState } from '../../../util/test/network';
1718

@@ -42,35 +43,58 @@ jest.mock('react-redux', () => ({
4243
...jest.requireActual('react-redux'),
4344
useSelector: jest.fn(),
4445
}));
46+
47+
const mockTrackEvent = jest.fn();
48+
const mockCreateEventBuilder = jest.fn(() => ({
49+
addProperties: jest.fn().mockReturnThis(),
50+
build: jest.fn().mockReturnValue({
51+
properties: { chain_id: 1 },
52+
}),
53+
}));
54+
55+
jest.mock('../../hooks/useMetrics', () => ({
56+
useMetrics: () => ({
57+
trackEvent: mockTrackEvent,
58+
createEventBuilder: mockCreateEventBuilder,
59+
}),
60+
}));
61+
4562
const mockedNavigate = jest.fn();
4663
const mockedReplace = jest.fn();
64+
const mockUseRoute = jest.fn();
4765

48-
jest.mock('@react-navigation/native', () => {
49-
const actualNav = jest.requireActual('@react-navigation/native');
50-
51-
const navigation = {
52-
params: {
53-
contractName: 'Opensea',
54-
collectible: { name: 'Leopard', tokenId: 6904, address: '0x123' },
55-
},
56-
};
57-
return {
58-
...actualNav,
59-
useNavigation: () => ({
60-
navigate: mockedNavigate,
61-
replace: mockedReplace,
62-
}),
63-
useRoute: jest.fn(() => ({ params: navigation.params })),
64-
};
65-
});
66+
jest.mock('@react-navigation/native', () => ({
67+
...jest.requireActual('@react-navigation/native'),
68+
useNavigation: () => ({
69+
navigate: mockedNavigate,
70+
replace: mockedReplace,
71+
}),
72+
useRoute: () => mockUseRoute(),
73+
}));
6674

6775
describe('CollectibleModal', () => {
76+
beforeEach(() => {
77+
jest.clearAllMocks();
78+
// Default route params without source
79+
mockUseRoute.mockReturnValue({
80+
params: {
81+
contractName: 'Opensea',
82+
collectible: { name: 'Leopard', tokenId: 6904, address: '0x123' },
83+
},
84+
});
85+
});
86+
6887
afterEach(() => {
6988
(useSelector as jest.Mock).mockClear();
7089
});
71-
it('should render correctly', async () => {
90+
it('renders correctly', async () => {
7291
(useSelector as jest.Mock).mockImplementation((selector) => {
92+
if (selector === collectiblesSelector) return collectibles;
93+
if (selector === selectIsIpfsGatewayEnabled) return false;
94+
if (selector === selectDisplayNftMedia) return false;
7395
if (selector === selectSendRedesignFlags) return { enabled: false };
96+
if (selector === selectChainId) return '0x1';
97+
return undefined;
7498
});
7599
const { toJSON } = renderWithProvider(<CollectibleModal />, {
76100
state: mockInitialState,
@@ -79,12 +103,14 @@ describe('CollectibleModal', () => {
79103
expect(toJSON()).toMatchSnapshot();
80104
});
81105

82-
it('should render the correct token name and ID', async () => {
106+
it('renders the correct token name and ID', async () => {
83107
(useSelector as jest.Mock).mockImplementation((selector) => {
84108
if (selector === collectiblesSelector) return collectibles;
85109
if (selector === selectIsIpfsGatewayEnabled) return true;
86110
if (selector === selectDisplayNftMedia) return true;
87111
if (selector === selectSendRedesignFlags) return { enabled: false };
112+
if (selector === selectChainId) return '0x1';
113+
return undefined;
88114
});
89115

90116
const { findAllByText } = renderWithProvider(<CollectibleModal />, {
@@ -94,4 +120,94 @@ describe('CollectibleModal', () => {
94120
expect(await findAllByText('#6904')).toBeDefined();
95121
expect(await findAllByText('Leopard')).toBeDefined();
96122
});
123+
124+
it('tracks NFT Details Opened event', () => {
125+
(useSelector as jest.Mock).mockImplementation((selector) => {
126+
if (selector === collectiblesSelector) return collectibles;
127+
if (selector === selectIsIpfsGatewayEnabled) return false;
128+
if (selector === selectDisplayNftMedia) return false;
129+
if (selector === selectSendRedesignFlags) return { enabled: false };
130+
if (selector === selectChainId) return '0x1';
131+
return undefined;
132+
});
133+
134+
renderWithProvider(<CollectibleModal />, {
135+
state: mockInitialState,
136+
});
137+
138+
expect(mockCreateEventBuilder).toHaveBeenCalled();
139+
expect(mockTrackEvent).toHaveBeenCalled();
140+
});
141+
142+
it('tracks NFT Details Opened event with mobile-nft-list source', () => {
143+
const mockAddProperties = jest.fn().mockReturnThis();
144+
const mockBuild = jest.fn();
145+
mockCreateEventBuilder.mockReturnValue({
146+
addProperties: mockAddProperties,
147+
build: mockBuild,
148+
});
149+
150+
mockUseRoute.mockReturnValue({
151+
params: {
152+
contractName: 'Opensea',
153+
collectible: { name: 'Leopard', tokenId: 6904, address: '0x123' },
154+
source: 'mobile-nft-list',
155+
},
156+
});
157+
158+
(useSelector as jest.Mock).mockImplementation((selector) => {
159+
if (selector === collectiblesSelector) return collectibles;
160+
if (selector === selectIsIpfsGatewayEnabled) return false;
161+
if (selector === selectDisplayNftMedia) return false;
162+
if (selector === selectSendRedesignFlags) return { enabled: false };
163+
if (selector === selectChainId) return '0x1';
164+
return undefined;
165+
});
166+
167+
renderWithProvider(<CollectibleModal />, {
168+
state: mockInitialState,
169+
});
170+
171+
expect(mockAddProperties).toHaveBeenCalledWith(
172+
expect.objectContaining({
173+
source: 'mobile-nft-list',
174+
}),
175+
);
176+
});
177+
178+
it('tracks NFT Details Opened event with mobile-nft-list-page source', () => {
179+
const mockAddProperties = jest.fn().mockReturnThis();
180+
const mockBuild = jest.fn();
181+
mockCreateEventBuilder.mockReturnValue({
182+
addProperties: mockAddProperties,
183+
build: mockBuild,
184+
});
185+
186+
mockUseRoute.mockReturnValue({
187+
params: {
188+
contractName: 'Opensea',
189+
collectible: { name: 'Leopard', tokenId: 6904, address: '0x123' },
190+
source: 'mobile-nft-list-page',
191+
},
192+
});
193+
194+
(useSelector as jest.Mock).mockImplementation((selector) => {
195+
if (selector === collectiblesSelector) return collectibles;
196+
if (selector === selectIsIpfsGatewayEnabled) return false;
197+
if (selector === selectDisplayNftMedia) return false;
198+
if (selector === selectSendRedesignFlags) return { enabled: false };
199+
if (selector === selectChainId) return '0x1';
200+
return undefined;
201+
});
202+
203+
renderWithProvider(<CollectibleModal />, {
204+
state: mockInitialState,
205+
});
206+
207+
expect(mockAddProperties).toHaveBeenCalledWith(
208+
expect.objectContaining({
209+
source: 'mobile-nft-list-page',
210+
}),
211+
);
212+
});
97213
});

‎app/components/UI/CollectibleModal/CollectibleModal.tsx‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,8 @@ const CollectibleModal = () => {
3838
const { trackEvent, createEventBuilder } = useMetrics();
3939
const chainId = useSelector(selectChainId);
4040

41-
const { contractName, collectible } = useParams<CollectibleModalParams>();
41+
const { contractName, collectible, source } =
42+
useParams<CollectibleModalParams>();
4243

4344
const modalRef = useRef<ReusableModalRef>(null);
4445

@@ -73,13 +74,16 @@ const CollectibleModal = () => {
7374
useEffect(() => {
7475
trackEvent(
7576
createEventBuilder(MetaMetricsEvents.NFT_DETAILS_OPENED)
76-
.addProperties({ chain_id: getDecimalChainId(chainId) })
77+
.addProperties({
78+
chain_id: getDecimalChainId(chainId),
79+
...(source && { source }),
80+
})
7781
.build(),
7882
);
7983
// The linter wants `trackEvent` to be added as a dependency,
8084
// But the event fires twice if I do that.
8185
// eslint-disable-next-line react-hooks/exhaustive-deps
82-
}, [chainId]);
86+
}, [chainId, source]);
8387

8488
const onSend = useCallback(async () => {
8589
dispatch(newAssetTransaction({ contractName, ...collectible }));

‎app/components/UI/CollectibleModal/CollectibleModal.types.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import { Nft } from '@metamask/assets-controllers';
33
export interface CollectibleModalParams {
44
contractName: string;
55
collectible: Nft;
6+
source?: 'mobile-nft-list' | 'mobile-nft-list-page';
67
}
78

89
export interface ReusableModalRef {

‎app/components/UI/CollectibleModal/__snapshots__/CollectibleModal.test.tsx.snap‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
// Jest Snapshot v1, https://goo.gl/fbAQLP
22

3-
exports[`CollectibleModal should render correctly 1`] = `
3+
exports[`CollectibleModal renders correctly 1`] = `
44
<View
55
style={
66
{

‎app/components/UI/NftGrid/NftGrid.test.tsx‎

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -586,6 +586,65 @@ describe('NftGrid', () => {
586586

587587
expect(mockNavigate).toHaveBeenCalledWith('NftDetails', {
588588
collectible: mockNft,
589+
source: 'mobile-nft-list',
590+
});
591+
});
592+
});
593+
594+
it('passes mobile-nft-list source when navigating from homepage view', async () => {
595+
const mockCollectibles = { '0x1': [mockNft] };
596+
mockUseSelector
597+
.mockReturnValueOnce(false) // isNftFetchingProgress
598+
.mockReturnValueOnce(false) // selectHomepageRedesignV1Enabled
599+
.mockReturnValueOnce(mockCollectibles); // multichainCollectiblesByEnabledNetworksSelector
600+
const store = mockStore(initialState);
601+
602+
const { getByTestId } = render(
603+
<Provider store={store}>
604+
<NftGrid isFullView={false} />
605+
</Provider>,
606+
);
607+
608+
act(() => {
609+
jest.advanceTimersByTime(100);
610+
});
611+
612+
await waitFor(() => {
613+
const nftItem = getByTestId('collectible-Test NFT-456');
614+
fireEvent.press(nftItem);
615+
616+
expect(mockNavigate).toHaveBeenCalledWith('NftDetails', {
617+
collectible: mockNft,
618+
source: 'mobile-nft-list',
619+
});
620+
});
621+
});
622+
623+
it('passes mobile-nft-list-page source when navigating from full view', async () => {
624+
const mockCollectibles = { '0x1': [mockNft] };
625+
mockUseSelector
626+
.mockReturnValueOnce(false) // isNftFetchingProgress
627+
.mockReturnValueOnce(false) // selectHomepageRedesignV1Enabled
628+
.mockReturnValueOnce(mockCollectibles); // multichainCollectiblesByEnabledNetworksSelector
629+
const store = mockStore(initialState);
630+
631+
const { getByTestId } = render(
632+
<Provider store={store}>
633+
<NftGrid isFullView />
634+
</Provider>,
635+
);
636+
637+
act(() => {
638+
jest.advanceTimersByTime(100);
639+
});
640+
641+
await waitFor(() => {
642+
const nftItem = getByTestId('collectible-Test NFT-456');
643+
fireEvent.press(nftItem);
644+
645+
expect(mockNavigate).toHaveBeenCalledWith('NftDetails', {
646+
collectible: mockNft,
647+
source: 'mobile-nft-list-page',
589648
});
590649
});
591650
});

0 commit comments

Comments
 (0)