damus icon indicating copy to clipboard operation
damus copied to clipboard

wallet: Add fiat pricing to Wallet View

Open ericholguin opened this issue 7 months ago • 2 comments

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: or Fixes: 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.]

ericholguin avatar Apr 26 '25 00:04 ericholguin

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 @.***>

jb55 avatar Apr 26 '25 00:04 jb55

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:

  1. 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.

  2. 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.

jb55 avatar Apr 26 '25 00:04 jb55

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.

danieldaquino avatar Oct 29 '25 18:10 danieldaquino