Conversation
WalkthroughAdds guards and validations in Buy.tsx to depend on walletPortfolioData, enforce a $2 minimum stable balance, and require at least $1 native-token balance for gas (minGasFee). Updates UI error handling accordingly. Tests are rewritten to assert immediate warning states on mount without debounced waits or input interactions. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant BuyComponent as Buy.tsx
participant Portfolio as walletPortfolioData
participant UI as Error/State UI
User->>BuyComponent: Mount / Refresh
BuyComponent->>Portfolio: Read balances
alt walletPortfolioData missing
BuyComponent->>BuyComponent: Warn & exit early
BuyComponent->>UI: Show data-missing state
else Has portfolio data
BuyComponent->>BuyComponent: Compute stable balances
alt Max stable < $2
BuyComponent->>BuyComponent: setMinimumStableBalance(true)
BuyComponent->>UI: Show min-stable warning
else Stable OK
BuyComponent->>BuyComponent: Find native token on target chain
alt Native not found OR USD < $1
BuyComponent->>BuyComponent: setMinGasFee(true)
BuyComponent->>UI: Show min-gas warning
else Gas OK
BuyComponent->>BuyComponent: setMinGasFee(false)
BuyComponent->>UI: Enable actions
end
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Deploying x with
|
| Latest commit: |
3eef4fe
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://6712d522.x-e62.pages.dev |
| Branch Preview URL: | https://pro-3691-re-warning-message.x-e62.pages.dev |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
src/apps/pulse/components/Buy/tests/Buy.test.tsx (2)
449-527: Add negative test case for when both conditions are satisfied.While the tests verify warnings appear when conditions are violated, there's no test confirming warnings DON'T appear when stable balance >= $2 AND gas >= $1. This would ensure the validation logic doesn't have false positives.
Add a test case like this:
it('does not show warnings when stable balance and gas are sufficient', async () => { const sufficientBalanceData = { ...mockWalletPortfolioData, result: { ...mockWalletPortfolioData.result, data: { ...mockWalletPortfolioData.result.data, total_wallet_balance: 2000, assets: [ // USDC with sufficient balance (>= $2) { asset: { id: 2, symbol: 'USDC', name: 'USD Coin', logo: 'usdc-logo.png', decimals: ['6'], contracts: [], blockchains: [], }, contracts_balances: [ { address: '0xA0b86991c6218b36c1d19D4a2e9Eb0cE3606eB48', chainId: 'evm:1', balance: 100, balanceRaw: '100000000', decimals: 6, }, ], cross_chain_balances: {}, price_change_24h: 0, estimated_balance: 100, price: 1, token_balance: 100, allocation: 0.95, wallets: ['0x1234567890123456789012345678901234567890'], }, // ETH with sufficient balance for gas (>= $1) { asset: { id: 3, symbol: 'ETH', name: 'Ethereum', logo: 'eth-logo.png', decimals: ['18'], contracts: [], blockchains: [], }, contracts_balances: [ { address: '0x0000000000000000000000000000000000000000', chainId: 'evm:1', balance: 0.5, balanceRaw: '500000000000000000', decimals: 18, }, ], cross_chain_balances: {}, price_change_24h: 0, estimated_balance: 1500, price: 3000, token_balance: 0.5, allocation: 0.05, wallets: ['0x1234567890123456789012345678901234567890'], }, ], }, }, }; renderWithProviders({ walletPortfolioData: sufficientBalanceData }); await waitFor(() => { expect(screen.queryByText('You need $2 USDC to trade, deposit USDC')).not.toBeInTheDocument(); expect(screen.queryByText(/Min\. \$1 .* required on/)).not.toBeInTheDocument(); }); });
449-527: Consider testing warning priority behavior.The PR description mentions fixing "priorities" of warning messages. It would be valuable to have a test that verifies when both low stable balance AND low gas conditions exist, only the stable balance warning is shown (since the code returns early at line 194).
Add a test case:
it('shows stable balance warning with priority over gas warning', async () => { const bothLowBalanceData = { ...mockWalletPortfolioData, result: { ...mockWalletPortfolioData.result, data: { ...mockWalletPortfolioData.result.data, total_wallet_balance: 1.3, assets: [ // USDC with balance less than $2 { asset: { id: 2, symbol: 'USDC', name: 'USD Coin', logo: 'usdc-logo.png', decimals: ['6'], contracts: [], blockchains: [], }, contracts_balances: [ { address: '0xA0b86991c6218b36c1d19D4a2e9Eb0cE3606eB48', chainId: 'evm:1', balance: 1, balanceRaw: '1000000', decimals: 6, }, ], cross_chain_balances: {}, price_change_24h: 0, estimated_balance: 1, price: 1, token_balance: 1, allocation: 0.77, wallets: ['0x1234567890123456789012345678901234567890'], }, // ETH with insufficient balance for gas (< $1) { asset: { id: 3, symbol: 'ETH', name: 'Ethereum', logo: 'eth-logo.png', decimals: ['18'], contracts: [], blockchains: [], }, contracts_balances: [ { address: '0x0000000000000000000000000000000000000000', chainId: 'evm:1', balance: 0.0001, balanceRaw: '100000000000000', decimals: 18, }, ], cross_chain_balances: {}, price_change_24h: 0, estimated_balance: 0.3, price: 3000, token_balance: 0.0001, allocation: 0.23, wallets: ['0x1234567890123456789012345678901234567890'], }, ], }, }, }; renderWithProviders({ walletPortfolioData: bothLowBalanceData }); await waitFor(() => { // Should show stable balance warning (higher priority) expect(screen.getByText('You need $2 USDC to trade, deposit USDC')).toBeInTheDocument(); // Should NOT show gas warning since stable balance check returns early expect(screen.queryByText(/Min\. \$1 .* required on/)).not.toBeInTheDocument(); }); });Also applies to: 529-607
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/apps/pulse/components/Buy/Buy.tsx(3 hunks)src/apps/pulse/components/Buy/tests/Buy.test.tsx(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/apps/pulse/components/Buy/Buy.tsx (2)
src/apps/pulse/utils/constants.ts (1)
getChainId(28-49)src/apps/pulse/utils/blockchain.ts (3)
isNativeToken(17-18)NativeSymbols(20-27)ChainNames(29-36)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: unit-tests
- GitHub Check: lint
- GitHub Check: build
🔇 Additional comments (5)
src/apps/pulse/components/Buy/Buy.tsx (3)
175-178: LGTM! Good defensive guard.The early return when
walletPortfolioDatais missing prevents downstream errors and aligns with the PR objective of showing warnings immediately when balance data is unavailable.
212-218: Verify native token balance calculation.The calculation
(nativeToken.balance ?? 0) * (nativeToken.price ?? 0)assumes both fields exist and are correctly populated. If either is undefined or zero when it shouldn't be, the gas fee check could produce false positives.Consider adding debug logging temporarily to verify the balance calculation is working as expected:
const nativeTokenUSDBalance = (nativeToken.balance ?? 0) * (nativeToken.price ?? 0); +console.log('[Gas Check]', { + symbol: nativeToken.symbol, + balance: nativeToken.balance, + price: nativeToken.price, + usdBalance: nativeTokenUSDBalance, + chainId: chainIdOfMaxStableBalance +}); if (!nativeTokenUSDBalance || nativeTokenUSDBalance < 1) { setMinGasFee(true); return; }You can remove this logging once verified in production.
202-206: UsechainNameToChainIdTokensDatafor chain ID comparison instead oft.chainId.The
Tokentype has ablockchain: string, not a numericchainId. Replace:- Number(getChainId(t.blockchain as MobulaChainNames)) === chainIdOfMaxStableBalance + chainNameToChainIdTokensData(t.blockchain) === chainIdOfMaxStableBalanceto ensure a correct numeric comparison and avoid NaN from comma-separated IDs.
Likely an incorrect or invalid review comment.
src/apps/pulse/components/Buy/tests/Buy.test.tsx (2)
521-526: LGTM! Test correctly verifies immediate warning behavior.The test now properly asserts that the minimum stable balance warning appears immediately when the component mounts with low balance data, which aligns with the PR objective.
601-606: LGTM! Gas fee warning test matches new behavior.The test correctly verifies that the gas fee warning appears immediately on mount with insufficient gas, matching the new immediate-warning implementation.
| if (maxStableBalance < 2) { | ||
| setMinimumStableBalance(true); | ||
| return; | ||
| } | ||
| setMinimumStableBalance(false); |
There was a problem hiding this comment.
Early return prevents clearing minGasFee state.
If maxStableBalance < 2, the function returns without ever reaching lines 197-219 where minGasFee state is managed. This means if a user previously had low gas (minGasFee=true) but then acquires stable coins while still having low gas, the minGasFee state won't be updated on subsequent renders.
Consider setting minGasFee(false) before the early return or restructure to always update all warning states:
if (maxStableBalance < 2) {
setMinimumStableBalance(true);
+ setMinGasFee(false); // Reset since stable balance takes priority
return;
}
setMinimumStableBalance(false);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (maxStableBalance < 2) { | |
| setMinimumStableBalance(true); | |
| return; | |
| } | |
| setMinimumStableBalance(false); | |
| if (maxStableBalance < 2) { | |
| setMinimumStableBalance(true); | |
| setMinGasFee(false); // Reset since stable balance takes priority | |
| return; | |
| } | |
| setMinimumStableBalance(false); |
🤖 Prompt for AI Agents
In src/apps/pulse/components/Buy/Buy.tsx around lines 192 to 196, the early
return when maxStableBalance < 2 sets minimumStableBalance but prevents later
logic (lines ~197-219) from updating minGasFee, leaving minGasFee stale; ensure
minGasFee is updated before returning (call setMinGasFee(false) or the
appropriate value) or refactor so warnings (minimumStableBalance and minGasFee)
are computed and set unconditionally (remove the early return and use
conditional branching for only the returnable behavior) so both states are
always updated.
| } else if (minGasFee) { | ||
| message = `Min. $1 ${NativeSymbols[maxStableCoinBalance.chainId]} required on ${ChainNames[maxStableCoinBalance.chainId]}`; |
There was a problem hiding this comment.
Add fallback for missing chainId in symbol/name lookups.
If maxStableCoinBalance.chainId is not in the NativeSymbols or ChainNames mappings (e.g., for a new chain), the message will display "Min. $1 undefined required on undefined".
Apply this diff to add fallback values:
-message = `Min. $1 ${NativeSymbols[maxStableCoinBalance.chainId]} required on ${ChainNames[maxStableCoinBalance.chainId]}`;
+message = `Min. $1 ${NativeSymbols[maxStableCoinBalance.chainId] || 'native token'} required on ${ChainNames[maxStableCoinBalance.chainId] || `chain ${maxStableCoinBalance.chainId}`}`;Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/apps/pulse/components/Buy/Buy.tsx around lines 666-667, the message
construction uses NativeSymbols[maxStableCoinBalance.chainId] and
ChainNames[maxStableCoinBalance.chainId] without fallbacks; update the code so
lookups use safe access with defaults (e.g., const symbol =
NativeSymbols[maxStableCoinBalance.chainId] || 'TOKEN'; const chainName =
ChainNames[maxStableCoinBalance.chainId] || 'Unknown chain';) and then build the
message using those variables so it never renders "undefined" for unknown
chains.
2 participants
Description
How Has This Been Tested?
Screenshots (if appropriate):
Types of changes
Summary by CodeRabbit
New Features
Bug Fixes
Tests