search

alecsloan / cryptolio

A simple yet powerful dashboard to track your digital assets
https://cryptolio.app
MIT License
9 stars 1 forks source link

Bug: Coingecko API #33

Open AppsCDN opened 3 years ago

AppsCDN commented 3 years ago

When the data source is coingecko, a bug occurs when adding some currencies that have the same "symbol" in the CoinGecko API. An example of this is the "Cardano (ADA)" currency. When I add it to the portfolio, another currency is added simultaneously to "Binance-Peg Cardano (ADA)". I believe that requesting data for a currency in the CoinGecko API should be done by "id" which in the API unique for each currency

alecsloan commented 3 years ago

Hey @AppsCDN, thanks for using my project and contributing this issue! You're totally right, I've run into this issue with some portfolio charting features I'm working on and have already converted to using CoinGecko's id system instead of symbols in my dev branch. I'll push the fix commit to master tonight!

alecsloan commented 3 years ago

@AppsCDN This issue should now be resolved in master. Please note you'll have to go into your dev tools, application, localstorage and drop the data row. If you have portfolio data you don't want to lose you can download the json in the settings panel then re-upload it after dropping the localstorage. Let me know if you have anymore issue!

AppsCDN commented 3 years ago

Okay, I'm going to sleep now, but tomorrow morning I'm going to test the update, thank you very much for your excellent work on this very useful project.

AppsCDN commented 3 years ago

Hi, some currencies are not being added to the portfolio when using the CoinGecko API, such as "Shiba Inu (Shib)" and THETA (THETA).

alecsloan commented 3 years ago

I see, this is a bit of a tricky situation caused by merging the two datasource asset lists. https://github.com/alecsloan/cryptolio/blob/1b0e741d0de2e13949934ac77ddd98a695876477/src/App.js#L193

If I remove the requirement for names to match we'll see the issue you described above such as"Binance-Peg Cardano (ADA)". However if I leave the name match check it won't merge things like THETA which has a name of THETA Network on CoinGecko. I didn't have much spare time to look into this today, but I'll have to dedicate some time to come up with a solution.

Thanks for your patience.

alecsloan commented 3 years ago

I've found a fix just by removing the Binance-Peg assets from Coingecko. Not sure why they have these as separate assets and not just market entries. You can find the fix in #35 which will be release v3.5.0 pending successful deployment to netlify. Also in the update comes a portfolio donut, historical portfolio performance stacked line chart, along with historical data endpoints.

AppsCDN commented 3 years ago

Ok, as soon as possible I will test the update, thanks!