damus
damus copied to clipboard
wallet: Add fiat pricing to Wallet View
Summary
This PR adds the fiat price to user's wallet balance. Using Coinbase API to get BTC current price and the user's locale (region) to determine their currency.
Changelog-Added: Added fiat pricing to wallet balance
Checklist
- [x] I have read (or I am familiar with) the Contribution Guidelines
- [x] I have tested the changes in this PR
- [x] I have opened or referred to an existing github issue related to this change.
- [x] My PR is either small, or I have split it into smaller logical commits that are easier to review
- [x] I have added the signoff line to all my commits. See Signing off your work
- [x] I have added appropriate changelog entries for the changes in this PR. See Adding changelog entries
- [ ] I do not need to add a changelog entry. Reason: [Please provide a reason]
- [x] I have added appropriate
Closes:orFixes:tags in the commit messages wherever applicable, or made sure those are not needed. See Submitting patches
Test report
Please provide a test report for the changes in this PR. You can use the template below, but feel free to modify it as needed.
Device: [Please specify the device you used for testing] iPhone 16 Pro
iOS: [Please specify the iOS version you used for testing] 18.3.2
Damus: [Please specify the Damus version or commit hash you used for testing] 1.14 (1) 54d6161a
Setup: [Please provide a brief description of the setup you used for testing, if applicable]
XCode Simulator / Local Device
Steps: [Please provide a list of steps you took to test the changes in this PR]
Open Wallet Balance verify there is a fiat price below the wallet balance and verify it is in your regional currency.
Results:
- [x] PASS
- [ ] Partial PASS
- Details: [Please provide details of the partial pass]
Other notes
[Please provide any other information that you think is relevant to this PR.]
On Fri, Apr 25, 2025 at 09:23:56AM -0600, ericholguin wrote:
This PR adds the fiat price to user's wallet balance. Using Coinbase API to get BTC current price and the user's locale (region) to determine their currency.
Changelog-Added: Added fiat pricing to wallet balance
Signed-off-by: ericholguin @.***>
Closes: https://github.com/damus-io/damus/pull/3001
LGTM
Reviewed-by: William Casarin @.***>
o1 review:
From: @.*** To: @.*** Subject: Review for PR wallet: Add fiat pricing to Wallet View #3001
https://github.com/damus-io/damus/pull/3001
Review for "wallet: Add fiat pricing to Wallet View" #3001 by ericholguin
Here are a few observations about potential issues in this PR:
-
Locale.currency force-unwrap: The code uses 'Locale.current.currency!.identifier', which could lead to a crash if the locale lacks a recognized currency. It might be safer to handle cases where this could be nil.
-
Error handling for fetch: In CoinbaseModel, if the network call fails, there's only a printed error. You may want to provide a user-facing fallback or more robust error handling.
Due to a high volume of PRs, I am closing older inactive PRs to make our list more manageable. Please feel free to reopen when needed.
Thank you for your understanding.