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.

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

Portfolio value change calculation is broken when using Mina values #204

Merged
merged 3 commits into from
Aug 31, 2024

Conversation

aliraza556
Copy link
Contributor

Problem:

  • When using Mina values on Dashboard, the daily change +0.02 (24h) is broken, displaying pretty much the same value all the time.

closes: #198

Issue ticket number and link:

Acceptance Criteria

  • The value reflects an actual fiat change, but in Mina.

@aliraza556
Copy link
Contributor Author

Hi @mrcnk, Please review this PR.

Copy link

deepsource-io bot commented Aug 21, 2024

Here's the code health analysis summary for commits ee3f2ee..dc6ed65. View details on DeepSource ↗.

Analysis Summary

AnalyzerStatusSummaryLink
DeepSource JavaScript LogoJavaScript✅ SuccessView Check ↗

💡 If you’re a repository administrator, you can configure the quality gates from the settings.

@mrcnk
Copy link
Member

mrcnk commented Aug 27, 2024

Hey, just checked and:

  • First there's an additional, unnecessary minus for negative values.
  • Value is still fixed at -0.03, no matter of portfolio size.
    CleanShot 2024-08-27 at 11 57 27

@aliraza556
Copy link
Contributor Author

Hey, just checked and:

  • First there's an additional, unnecessary minus for negative values.
  • Value is still fixed at -0.03, no matter of portfolio size.
    CleanShot 2024-08-27 at 11 57 27

@mrcnk Please review this PR: I have fixed
image

@mrcnk
Copy link
Member

mrcnk commented Aug 30, 2024

@aliraza556 It's now displaying fiat values both for fiat and Mina. For Mina it should display Mina value, so if for example the price of Mina is 50c, it should be:

Mina Change Value = Fiat Change Value / Fiat Price
In example: Fiat Change Value = 100 USD, Fiat Price = 50c
Mina Change Value = 100 USD / 0.5
Mina Change Value = 200 Mina

@aliraza556 aliraza556 force-pushed the Portfolio-value-change-calculation branch from d19a53a to 9cd94c6 Compare August 31, 2024 08:01
@aliraza556
Copy link
Contributor Author

@aliraza556 It's now displaying fiat values both for fiat and Mina. For Mina it should display Mina value, so if for example the price of Mina is 50c, it should be:

Mina Change Value = Fiat Change Value / Fiat Price
In example: Fiat Change Value = 100 USD, Fiat Price = 50c
Mina Change Value = 100 USD / 0.5
Mina Change Value = 200 Mina

@mrcnk Please review it i have fixed

@mrcnk
Copy link
Member

mrcnk commented Aug 31, 2024

@aliraza556 it's working fine now, can you just format the code and push again?

@aliraza556
Copy link
Contributor Author

@aliraza556 it's working fine now, can you just format the code and push again?

@mrcnk Ok

@aliraza556
Copy link
Contributor Author

@mrcnk Done

@mrcnk mrcnk merged commit bbe3ec7 into palladians:main Aug 31, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Portfolio value change calculation is broken when using Mina values
2 participants