Skip to content

Commit 0c32be7

Browse files
authored
feat: add NFT detection to wallet balance refresh (MetaMask#27272)
<!-- 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** include nfts on manual refresh <!-- 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: include nfts on manual refresh ## **Related issues** Fixes: ## **Manual testing steps** ```gherkin Feature: my feature name Scenario: user [verb for user action] Given [describe expected initial app state] When user [verb for user action] Then [describe expected outcome] ``` ## **Screenshots/Recordings** <!-- If applicable, add screenshots and/or recordings to visualize the before and after of your change. --> ### **Before** <!-- [screenshots/recordings] --> https://github.com/user-attachments/assets/e4e47867-490d-40a9-b22c-8dd9b6ff5239 ### **After** <!-- [screenshots/recordings] --> https://github.com/user-attachments/assets/ad697185-85a5-4902-80c5-f819516d66fb ## **Pre-merge author checklist** - [ ] 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). - [ ] I've completed the PR template to the best of my ability - [ ] I've included tests if applicable - [ ] I've documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [ ] 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] > **Medium Risk** > Adds an extra network/data-fetch operation to the balance refresh path, which could affect refresh latency and controller load if the gating conditions are misconfigured. > > **Overview** > Manual wallet refresh now optionally triggers NFT discovery by invoking `NftDetectionController.detectNfts` (with `firstPageOnly: true`) alongside existing token/balance/rate refresh work. > > NFT detection is gated behind both the homepage sections v1 feature flag and the user’s NFT detection preference (`selectUseNftDetection`), and tests were expanded to cover the new call and gating behavior. > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit accdd10. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY -->
1 parent 583ebf0 commit 0c32be7

2 files changed

Lines changed: 87 additions & 0 deletions

File tree

app/components/Views/Wallet/hooks/useBalanceRefresh.test.ts

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,10 @@ jest.mock('../../../../selectors/networkEnablementController', () => ({
3838
selectEVMEnabledNetworks: jest.fn(() => []),
3939
}));
4040

41+
jest.mock('../../../../selectors/preferencesController', () => ({
42+
selectUseNftDetection: jest.fn(() => true),
43+
}));
44+
4145
jest.mock('../../../../core/Engine', () => ({
4246
context: {
4347
AccountTrackerController: {
@@ -52,6 +56,9 @@ jest.mock('../../../../core/Engine', () => ({
5256
TokenBalancesController: {
5357
updateBalances: jest.fn(() => Promise.resolve()),
5458
},
59+
NftDetectionController: {
60+
detectNfts: jest.fn(() => Promise.resolve()),
61+
},
5562
},
5663
}));
5764

@@ -64,6 +71,18 @@ describe('useBalanceRefresh', () => {
6471
beforeEach(() => {
6572
jest.clearAllMocks();
6673
mockPopularEvmNetworks = ['0x1', '0x89'];
74+
const { selectHomepageSectionsV1Enabled } = jest.requireMock(
75+
'../../../../selectors/featureFlagController/homepage',
76+
);
77+
const { selectEVMEnabledNetworks } = jest.requireMock(
78+
'../../../../selectors/networkEnablementController',
79+
);
80+
const { selectUseNftDetection } = jest.requireMock(
81+
'../../../../selectors/preferencesController',
82+
);
83+
(selectHomepageSectionsV1Enabled as jest.Mock).mockReturnValue(true);
84+
(selectEVMEnabledNetworks as jest.Mock).mockReturnValue([]);
85+
(selectUseNftDetection as jest.Mock).mockReturnValue(true);
6786
(
6887
Engine.context.AccountTrackerController.refresh as jest.Mock
6988
).mockResolvedValue(undefined);
@@ -76,6 +95,9 @@ describe('useBalanceRefresh', () => {
7695
(
7796
Engine.context.TokenBalancesController.updateBalances as jest.Mock
7897
).mockResolvedValue(undefined);
98+
(
99+
Engine.context.NftDetectionController.detectNfts as jest.Mock
100+
).mockResolvedValue(undefined);
79101
});
80102

81103
it('returns refreshBalance, handleRefresh, and refreshing', () => {
@@ -125,6 +147,58 @@ describe('useBalanceRefresh', () => {
125147
).toHaveBeenCalledWith({ chainIds: ['0x1', '0x89'] });
126148
});
127149

150+
describe('NftDetectionController', () => {
151+
it('calls detectNfts with popular chain IDs and firstPageOnly when sections v1 is enabled', async () => {
152+
const { result } = renderHook(() => useBalanceRefresh());
153+
154+
await act(async () => {
155+
await result.current.refreshBalance();
156+
});
157+
158+
expect(
159+
Engine.context.NftDetectionController.detectNfts,
160+
).toHaveBeenCalledWith(['0x1', '0x89'], { firstPageOnly: true });
161+
});
162+
163+
it('does not call detectNfts when sections v1 is disabled', async () => {
164+
const { selectHomepageSectionsV1Enabled } = jest.requireMock(
165+
'../../../../selectors/featureFlagController/homepage',
166+
);
167+
const { selectEVMEnabledNetworks } = jest.requireMock(
168+
'../../../../selectors/networkEnablementController',
169+
);
170+
(selectHomepageSectionsV1Enabled as jest.Mock).mockReturnValue(false);
171+
(selectEVMEnabledNetworks as jest.Mock).mockReturnValue(['0x1']);
172+
173+
const { result } = renderHook(() => useBalanceRefresh());
174+
175+
await act(async () => {
176+
await result.current.refreshBalance();
177+
});
178+
179+
expect(
180+
Engine.context.NftDetectionController.detectNfts,
181+
).not.toHaveBeenCalled();
182+
});
183+
184+
it('does not call detectNfts when user has disabled NFT detection in settings', async () => {
185+
const { selectUseNftDetection } = jest.requireMock(
186+
'../../../../selectors/preferencesController',
187+
);
188+
(selectUseNftDetection as jest.Mock).mockReturnValue(false);
189+
190+
const { result } = renderHook(() => useBalanceRefresh());
191+
192+
await act(async () => {
193+
await result.current.refreshBalance();
194+
});
195+
196+
expect(
197+
Engine.context.NftDetectionController.detectNfts,
198+
).not.toHaveBeenCalled();
199+
});
200+
});
201+
128202
it('calls CurrencyRateController.updateExchangeRate with native currencies', async () => {
129203
const { result } = renderHook(() => useBalanceRefresh());
130204

app/components/Views/Wallet/hooks/useBalanceRefresh.ts

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import {
99
import { useNetworkEnablement } from '../../../hooks/useNetworkEnablement/useNetworkEnablement';
1010
import { selectHomepageSectionsV1Enabled } from '../../../../selectors/featureFlagController/homepage';
1111
import { selectEVMEnabledNetworks } from '../../../../selectors/networkEnablementController';
12+
import { selectUseNftDetection } from '../../../../selectors/preferencesController';
1213

1314
const REFRESH_TIMEOUT_MS = 5000;
1415
const REFRESH_TIMEOUT_ERROR_MESSAGE = 'Balance refresh timed out';
@@ -32,6 +33,8 @@ export const useBalanceRefresh = () => {
3233
selectHomepageSectionsV1Enabled,
3334
);
3435

36+
const isNftDetectionEnabled = useSelector(selectUseNftDetection);
37+
3538
const evmEnabledChainIds = useSelector(selectEVMEnabledNetworks);
3639

3740
const evmNetworkConfigurations = useSelector(
@@ -62,6 +65,7 @@ export const useBalanceRefresh = () => {
6265
CurrencyRateController,
6366
TokenBalancesController,
6467
TokenDetectionController,
68+
NftDetectionController,
6569
} = Engine.context;
6670
const networkClientIds = Object.values(evmNetworkConfigurationsFiltered)
6771
.map(
@@ -81,6 +85,13 @@ export const useBalanceRefresh = () => {
8185
TokenBalancesController.updateBalances({
8286
chainIds: evmChainIdsForRefresh,
8387
}),
88+
...(isHomepageSectionsV1Enabled && isNftDetectionEnabled
89+
? [
90+
NftDetectionController.detectNfts(evmChainIdsForRefresh, {
91+
firstPageOnly: true,
92+
}),
93+
]
94+
: []),
8495
]),
8596
new Promise((_, reject) =>
8697
setTimeout(
@@ -101,6 +112,8 @@ export const useBalanceRefresh = () => {
101112
evmNetworkConfigurationsFiltered,
102113
evmChainIdsForRefresh,
103114
nativeCurrencies,
115+
isHomepageSectionsV1Enabled,
116+
isNftDetectionEnabled,
104117
]);
105118

106119
const handleRefresh = useCallback(async () => {

0 commit comments

Comments
 (0)