Decouple Transfer from FiatConnect and auto-enable asset on buy#1837
Conversation
Transfer was importing FiatConnect solely to create FiatSceneViewModel in sheet handlers, making it impossible to inject AssetsEnabler without adding package dependencies or stub types. - Move AmountNavigationView to Gem/Navigation/Transfer - Extract ConfirmTransferScene sheets into ConfirmTransferNavigationView - Remove FiatConnect dependency from Transfer - Move @entry fiatService from FiatConnect to Gem/Types/Environment
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request refactors the transfer and fiat connect modules, notably moving sheet presentation logic from ConfirmTransferScene to a new ConfirmTransferNavigationView and updating FiatSceneViewModel to support asset enabling via the AssetsEnabler service. Additionally, many types and view models within the Transfer feature have been made public. Feedback identifies a critical compilation error in FiatSceneViewModel where the walletId property is not initialized, and suggests implementing more robust error logging for the asset enabling process to ensure visibility in production environments.
| self.wallet = wallet | ||
| self.assetsEnabler = assetsEnabler | ||
| self.type = type | ||
| let walletId = wallet.walletId | ||
| self.assetQuery = ObservableQuery(AssetRequest(walletId: walletId, assetId: assetAddress.asset.id), initialValue: .with(asset: assetAddress.asset)) |
There was a problem hiding this comment.
The class property walletId is not being initialized in the init method, which will cause a compilation error. You should initialize it from the wallet parameter. The local walletId variable then becomes redundant.
Please also ensure that wallet and assetsEnabler are declared as properties of FiatSceneViewModel.
| self.wallet = wallet | |
| self.assetsEnabler = assetsEnabler | |
| self.type = type | |
| let walletId = wallet.walletId | |
| self.assetQuery = ObservableQuery(AssetRequest(walletId: walletId, assetId: assetAddress.asset.id), initialValue: .with(asset: assetAddress.asset)) | |
| self.wallet = wallet | |
| self.assetsEnabler = assetsEnabler | |
| self.type = type | |
| self.walletId = wallet.walletId | |
| self.assetQuery = ObservableQuery(AssetRequest(walletId: self.walletId, assetId: assetAddress.asset.id), initialValue: .with(asset: assetAddress.asset)) |
| } catch { | ||
| debugLog("FiatSceneViewModel enableAsset error: \(error)") | ||
| } |
There was a problem hiding this comment.
The error from enableAssets is only logged to the debug console. This means that in release builds, any failure to enable the asset will be silent. Consider using a more robust logging mechanism to track these errors in production.
gemdev111
force-pushed
the
fiat-auto-detect
branch
from
749bab3 to
8e4e03b
Compare
3 participants
Refactor Transfer/FiatConnect dependency and improve fiat buy flow.
Close: #1820