Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Bug: get fiat value for CUC fails with 404 #1649

Closed
lisabaut opened this issue Oct 21, 2022 · 6 comments · Fixed by #1906
Closed

Bug: get fiat value for CUC fails with 404 #1649

lisabaut opened this issue Oct 21, 2022 · 6 comments · Fixed by #1906
Assignees
@mviswanathsai
Labels
bug Something isn't working good first issue Good for newcomers hacktoberfest

Comments

@lisabaut
Copy link
Contributor

Describe the bug

While working on this PR #1645
I saw that a conversion for sats to fiat for the currency "CUC" fails.

Sidenote:
In the PR above I added a try/catch block which was missing.

Screenshots of the changes [optional]

From the PR mentioned above

404

To Reproduce

Steps to reproduce the behavior:

  1. Open the settings page
  2. Change the currency to CUC
  3. watch the console for the error

Expected behavior

No error is thrown or the currency can not be selected

Alby information

  • Alby Version: current master
  • Alby installed through manually
  • Wallet connected with Alby

Device information [optional]

  • OS: Mac
  • Browser FF
  • Browser Version 105

Are you working on this issue?

no
@lisabaut lisabaut mentioned this issue Oct 21, 2022
Merged
3 tasks
@lisabaut lisabaut removed their assignment Oct 21, 2022
@lisabaut
Copy link
Contributor Author

As discussed:

Go over the currency list on the Settings Screen and get an overview which currency with which provider does not work / throws an error.
Remove these currencies from the list accordingly.

@lisabaut lisabaut added good first issue Good for newcomers bug Something isn't working labels Oct 21, 2022
@bumi
Copy link
Collaborator

bumi commented Dec 13, 2022

https://twitter.com/NicolasDorier/status/1602465746591928320

@mviswanathsai
Copy link
Contributor

Hey! I would like to work on this, what prior knowledge do I need to contribute to Alby?

@escapedcat
Copy link
Contributor

Hey! I would like to work on this, what prior knowledge do I need to contribute to Alby?

@mviswanathsai you would need to now TS/JS mainly.
But let us first check if removing broken ones for now is a good first iteration.
@bumi @reneaaron wdyt? Is removing broken/unsupported ones ok for now?
I would say yes.
I think Nicolas' solution would also require the Alby proxy to handle different APIs internally, right? I don't think that's worth the effort currently.

@mviswanathsai
Copy link
Contributor

BYN CUC MRU SSP STN VES throw 404 on Alby
CLP FKP MZN TJS YER on Coindesk
Should I remove these currencies from constants.ts?

@mviswanathsai
Copy link
Contributor

I have opened a PR here #1906

@escapedcat escapedcat mentioned this issue Dec 21, 2022
Merged
3 tasks
bumi added a commit that referenced this issue Dec 22, 2022
@bumi
Merge branch 'master' into fix/lndhub-add-invoice-params
9e75ac2 
* master: (570 commits)
  chore: remove lock file changes #1649
  added yarn.lock
  feat(nostr): add link to more info about nostr keys (#1908)
  Removed yarn.lock file
  fix: webbtc should behave like webln
  Bug: Removed currencies which throw a 404
  Update axios to version 1.2.1
  docs: fix translations link
  chore: add webbtc.request()
  v1.21.0
  fix: remove duplication of height, width
  fix: comment
  fix: separate getPosition func, bugfixes
  fix: added routermc
  prettier
  Update SETUP.md
  Added Vida to "Website" screen (#1887)
  Log in: two words when it's a verb. (#1890)
  fix: ts
  fix: ensure toLowerCase can be called
  ...
bumi added a commit that referenced this issue Dec 23, 2022
@bumi
Merge branch 'master' into fix/local-storage-string-orion
e4c5de1 
* master: (66 commits)
  v1.21.1
  Update @headlessui/react to version 1.7.7
  chore: always prompt for zero amount invoices
  chore: remove lock file changes #1649
  added yarn.lock
  feat(nostr): add link to more info about nostr keys (#1908)
  Removed yarn.lock file
  fix: webbtc should behave like webln
  Bug: Removed currencies which throw a 404
  Update axios to version 1.2.1
  fix: align confirmOrCancel with bottom and align form-submit handling #1902
  fix: align confirmOrCancel with bottom in forms #1902
  style: import sort order #1902
  docs: fix translations link
  chore: add webbtc.request()
  v1.21.0
  fix: remove duplication of height, width
  fix: comment
  fix: separate getPosition func, bugfixes
  fix: added routermc
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@mviswanathsai mviswanathsai

Labels
bug Something isn't working good first issue Good for newcomers hacktoberfest
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Bug: Removed currencies which throw a 404
4 participants
@bumi @escapedcat @lisabaut @mviswanathsai