achannarasappa / ticker

Terminal stock ticker with live updates and position tracking
GNU General Public License v3.0
5.01k stars 273 forks source link

total change wrong with disable-unit-cost-conversion #241

Closed jeremyrgreen closed 1 year ago

jeremyrgreen commented 1 year ago

Describe the bug When currency-disable-unit-cost-conversion is set to true, the average unit cost is treated correctly as being in my local currency. However, the total change in value of the holding is computed as if this were in the foreign currency.

To Reproduce Here's a minimal example config file showing the outcome, using a JPY->EUR conversion to make the effect clear:

show-tags: true
show-holdings: true
currency: EUR
currency-disable-unit-cost-conversion: true
watchlist:
lots:
  - symbol: 7974.T
    quantity: 1
    unit_cost: 37

Expected behavior In this example, the unit cost of 37 EUR is close to the market price, so there should be a small total change. However, it is treated as 37 JPY and re-converted to EUR, so the total change shown is almost as large as the current value of the holding.

Environment

jeremyrgreen commented 1 year ago

One possible fix is to replace the last function in ticker/internal/asset/currency.go with the following:

func convertAssetHoldingCurrency(currencyRateByUse currency.CurrencyRateByUse, holding c.Holding) c.Holding {
    value := holding.Value * currencyRateByUse.QuotePrice
    cost := holding.Cost * currencyRateByUse.PositionCost
    totalChangeAmount := value - cost
    totalChangePercent := (totalChangeAmount / cost) * 100
    return c.Holding{
        Value:     value,
        Cost:      cost,
        Quantity:  holding.Quantity,
        UnitValue: holding.UnitValue * currencyRateByUse.QuotePrice,
        UnitCost:  holding.UnitCost * currencyRateByUse.PositionCost,
        DayChange: c.HoldingChange{
            Amount:  holding.DayChange.Amount * currencyRateByUse.QuotePrice,
            Percent: holding.DayChange.Percent,
        },
        TotalChange: c.HoldingChange{
            Amount:  totalChangeAmount,
            Percent: totalChangePercent,
        },
        Weight: holding.Weight,
    }
}

This simply replaces the original calculation of the total change with the correct one. However, I don't know whether it makes sense to compute it once incorrectly (without the currency conversion) then replace it — if the cost and value are in different currencies, the total change can only make sense after a currency conversion has been applied.

achannarasappa commented 1 year ago

Thanks for reporting this and sharing the suggested solution

The latest release should fix this issue but please reopen if you still see an issue