Conversation
- Enable asset automatically when user presses Continue on buy screen - Inject AssetsEnabler into FiatSceneViewModel, fire-and-forget on continue - Update all call sites to pass Wallet instead of WalletId - Add BalanceService dependency to FiatConnect package
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the user experience by automating the activation of newly purchased cryptocurrency assets. Previously, users might have needed to manually enable an asset after buying it. Now, the system automatically activates the asset in the background as soon as the user proceeds with the purchase, ensuring the asset is immediately visible and usable without any additional steps. This change streamlines the user journey for buying crypto across all entry points and leverages existing asset management patterns for consistency. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
![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 FiatConnect feature by replacing direct usage of WalletId with a Wallet object and introducing an AssetsEnabler service. This change allows the FiatSceneViewModel to manage asset enabling, specifically by calling enableAsset() after a successful fiat buy operation. The review comments suggest an improvement to prevent potential UI blocking by ensuring the enableAsset() logic is executed on a background thread using Task.detached.
| } | ||
|
|
||
| urlState = .data(()) | ||
| Task { await enableAsset() } |
There was a problem hiding this comment.
To avoid potential UI blocking, it's safer to run the asset enabling logic on a background thread. The current implementation with Task inherits the main actor context, which could lead to UI freezes if enableAsset() performs synchronous I/O operations.
I'm suggesting a change to the enableAsset() function to make it synchronous and launch a Task.detached for background execution. Consequently, this call site should be updated to a direct synchronous call.
| Task { await enableAsset() } | |
| enableAsset() |
| private func enableAsset() async { | ||
| guard type == .buy else { return } | ||
| do { | ||
| try await assetsEnabler.enableAssets(wallet: wallet, assetIds: [asset.id], enabled: true) | ||
| } catch { | ||
| debugLog("FiatSceneViewModel enableAsset error: \(error)") | ||
| } | ||
| } |
There was a problem hiding this comment.
To prevent potential UI blocking from synchronous I/O operations within assetsEnabler.enableAssets, this logic should be executed on a background thread.
I recommend changing this function to be synchronous and using Task.detached to perform the asset enabling work in the background. This ensures the main thread remains responsive.
private func enableAsset() {
guard type == .buy else { return }
// Capture properties to use in the detached task
let wallet = self.wallet
let asset = self.asset
let assetsEnabler = self.assetsEnabler
Task.detached {
do {
try await assetsEnabler.enableAssets(wallet: wallet, assetIds: [asset.id], enabled: true)
} catch {
debugLog("FiatSceneViewModel enableAsset error: \(error)")
}
}
}
1 participant
closes #1820
Summary
AssetsEnablerpattern used inReceiveViewModelChanges
Wallet+AssetsEnablerintoFiatSceneViewModelenableAsset()as fire-and-forgetTaskinonSelectContinue()AmountSheetType/ConfirmTransferSheetTypeto carryWalletinstead ofWalletIdViewModelFactory, navigation stacks, scenes)BalanceServicedependency toFiatConnectpackage