fix: rc errors on importing and sending fbt - #876
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/screens/SendConfirmScreen.js (1)
241-250: ⚡ Quick winUse server-provided native token symbol in fee display UI.
This UI still formats fee with a library constant symbol. Please source the symbol from
serverInfo.native_token.symboland keep the constant only as fallback.Based on learnings: Do not hardcode native token name/symbol in user-facing UI; use dynamic server info (
serverInfo.native_token.symbol), with constant UID only as temporary fallback.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/screens/SendConfirmScreen.js` around lines 241 - 250, The fee UI currently uses the constant nativeSymbol; update renderNetworkFeeValue to first read the symbol from serverInfo.native_token.symbol and use that in the displayed fee, falling back to the existing nativeSymbol constant only when serverInfo or serverInfo.native_token.symbol is missing or falsy; modify the JSX inside renderNetworkFeeValue (and any helper renderValue calls if they accept a symbol param) to pass/display the dynamic symbol instead of the hardcoded constant.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/sagas/tokenImport.js`:
- Line 55: fetchTokenDetails can return objects missing the required version
field (via the stored-token early return or UID fallback), which causes
registerToken and the batch import to fail; update fetchTokenDetails to ensure
it always returns an importable token object with a defined version (or
explicitly skip/refetch when version is absent) by: validating the returned
object before any early returns in fetchTokenDetails, adding logic to refetch or
derive version when missing (or return a sentinel to skip), and ensuring callers
like registerToken only receive objects with a defined version (or handle the
explicit skip sentinel) so the batch import won’t abort unexpectedly.
---
Nitpick comments:
In `@src/screens/SendConfirmScreen.js`:
- Around line 241-250: The fee UI currently uses the constant nativeSymbol;
update renderNetworkFeeValue to first read the symbol from
serverInfo.native_token.symbol and use that in the displayed fee, falling back
to the existing nativeSymbol constant only when serverInfo or
serverInfo.native_token.symbol is missing or falsy; modify the JSX inside
renderNetworkFeeValue (and any helper renderValue calls if they accept a symbol
param) to pass/display the dynamic symbol instead of the hardcoded constant.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 30f6fb7b-639f-480b-8ce7-cbb8dd5d976c
📒 Files selected for processing (7)
locale/da/texts.polocale/pt-br/texts.polocale/ru-ru/texts.polocale/texts.potsrc/sagas/tokenImport.jssrc/screens/SendAmountInput.jssrc/screens/SendConfirmScreen.js
| const unsubscribe = navigation.addListener('beforeRemove', async (e) => { | ||
| if (sentSuccessfullyRef.current || releasedRef.current || !sendTx) { | ||
| return; | ||
| } | ||
| e.preventDefault(); | ||
| releasedRef.current = true; | ||
| try { await sendTx.releaseUtxos(); } catch (err) { console.error(err); } | ||
| navigation.dispatch(e.data.action); | ||
| }); | ||
| return unsubscribe; | ||
| }, [sendTx, navigation]); |
There was a problem hiding this comment.
Thinking about this release here. After the user types the pin and the tx is sending, is not possible for the user to go back. But if the user taps the physical android back button, maybe this would release the utxos AND send the tx.
I don't think it's critical to change anything in this PR and @tuliomir has a task to review this physical back button handling, so maybe we should just mention this there.
There was a problem hiding this comment.
Referencing the #791 here for better visibility
github-project-automation
Bot
moved this from In Progress (Done)
to In Review (WIP)
in Hathor Network
github-project-automation
Bot
moved this from In Review (Done)
to Waiting to be deployed
in Hathor Network
3 participants
Acceptance Criteria
Security Checklist
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Localization