palladians / pallad

Progressive Mina Protocol Wallet
https://pallad.co/
Apache License 2.0
26 stars 10 forks source link

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

Closed aliraza556 closed 2 months ago

aliraza556 commented 2 months ago

Problem:

closes: #198

Issue ticket number and link:

Acceptance Criteria

aliraza556 commented 2 months ago

Hi @mrcnk, Please review this PR.

deepsource-io[bot] commented 2 months ago

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 commented 2 months ago

Hey, just checked and:

aliraza556 commented 2 months ago

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 commented 2 months ago

@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 commented 2 months ago

@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 commented 2 months ago

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

aliraza556 commented 2 months ago

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

@mrcnk Ok

aliraza556 commented 2 months ago

@mrcnk Done