From 0bca8897d69e8b88966195955f08a00d4e6c8c32 Mon Sep 17 00:00:00 2001 From: gizmodus Date: Sun, 10 Mar 2024 09:35:47 +0100 Subject: [PATCH] Fix average price calculation by only considering BUY transactions (#3125) * Fix average price calculation by only considering buy transactions * Update changelog --- CHANGELOG.md | 6 + ...aln-buy-and-sell-in-two-activities.spec.ts | 166 ++++++++++++++++++ .../src/app/portfolio/portfolio-calculator.ts | 60 +++---- 3 files changed, 199 insertions(+), 33 deletions(-) create mode 100644 apps/api/src/app/portfolio/portfolio-calculator-baln-buy-and-sell-in-two-activities.spec.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index e4f689445..33baa68c4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,12 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## Unreleased + +### Fixed + +- Fixed an issue in the performance calculation caused by multiple `SELL` activities on the same day + ## 2.62.0 - 2024-03-09 ### Changed diff --git a/apps/api/src/app/portfolio/portfolio-calculator-baln-buy-and-sell-in-two-activities.spec.ts b/apps/api/src/app/portfolio/portfolio-calculator-baln-buy-and-sell-in-two-activities.spec.ts new file mode 100644 index 000000000..703931846 --- /dev/null +++ b/apps/api/src/app/portfolio/portfolio-calculator-baln-buy-and-sell-in-two-activities.spec.ts @@ -0,0 +1,166 @@ +import { CurrentRateService } from '@ghostfolio/api/app/portfolio/current-rate.service'; +import { ExchangeRateDataService } from '@ghostfolio/api/services/exchange-rate-data/exchange-rate-data.service'; +import { parseDate } from '@ghostfolio/common/helper'; + +import Big from 'big.js'; + +import { CurrentRateServiceMock } from './current-rate.service.mock'; +import { PortfolioCalculator } from './portfolio-calculator'; + +jest.mock('@ghostfolio/api/app/portfolio/current-rate.service', () => { + return { + // eslint-disable-next-line @typescript-eslint/naming-convention + CurrentRateService: jest.fn().mockImplementation(() => { + return CurrentRateServiceMock; + }) + }; +}); + +describe('PortfolioCalculator', () => { + let currentRateService: CurrentRateService; + let exchangeRateDataService: ExchangeRateDataService; + + beforeEach(() => { + currentRateService = new CurrentRateService(null, null, null, null); + + exchangeRateDataService = new ExchangeRateDataService( + null, + null, + null, + null + ); + }); + + describe('get current positions', () => { + it.only('with BALN.SW buy and sell in two activities', async () => { + const portfolioCalculator = new PortfolioCalculator({ + currentRateService, + exchangeRateDataService, + currency: 'CHF', + orders: [ + { + currency: 'CHF', + date: '2021-11-22', + dataSource: 'YAHOO', + fee: new Big(1.55), + name: 'Bâloise Holding AG', + quantity: new Big(2), + symbol: 'BALN.SW', + type: 'BUY', + unitPrice: new Big(142.9) + }, + { + currency: 'CHF', + date: '2021-11-30', + dataSource: 'YAHOO', + fee: new Big(1.65), + name: 'Bâloise Holding AG', + quantity: new Big(1), + symbol: 'BALN.SW', + type: 'SELL', + unitPrice: new Big(136.6) + }, + { + currency: 'CHF', + date: '2021-11-30', + dataSource: 'YAHOO', + fee: new Big(0), + name: 'Bâloise Holding AG', + quantity: new Big(1), + symbol: 'BALN.SW', + type: 'SELL', + unitPrice: new Big(136.6) + } + ] + }); + + portfolioCalculator.computeTransactionPoints(); + + const spy = jest + .spyOn(Date, 'now') + .mockImplementation(() => parseDate('2021-12-18').getTime()); + + const chartData = await portfolioCalculator.getChartData({ + start: parseDate('2021-11-22') + }); + + const currentPositions = await portfolioCalculator.getCurrentPositions( + parseDate('2021-11-22') + ); + + const investments = portfolioCalculator.getInvestments(); + + const investmentsByMonth = portfolioCalculator.getInvestmentsByGroup({ + data: chartData, + groupBy: 'month' + }); + + spy.mockRestore(); + + expect(currentPositions).toEqual({ + currentValueInBaseCurrency: new Big('0'), + errors: [], + grossPerformance: new Big('-12.6'), + grossPerformancePercentage: new Big('-0.04408677396780965649'), + grossPerformancePercentageWithCurrencyEffect: new Big( + '-0.04408677396780965649' + ), + grossPerformanceWithCurrencyEffect: new Big('-12.6'), + hasErrors: false, + netPerformance: new Big('-15.8'), + netPerformancePercentage: new Big('-0.05528341497550734703'), + netPerformancePercentageWithCurrencyEffect: new Big( + '-0.05528341497550734703' + ), + netPerformanceWithCurrencyEffect: new Big('-15.8'), + positions: [ + { + averagePrice: new Big('0'), + currency: 'CHF', + dataSource: 'YAHOO', + dividend: new Big('0'), + dividendInBaseCurrency: new Big('0'), + fee: new Big('3.2'), + firstBuyDate: '2021-11-22', + grossPerformance: new Big('-12.6'), + grossPerformancePercentage: new Big('-0.04408677396780965649'), + grossPerformancePercentageWithCurrencyEffect: new Big( + '-0.04408677396780965649' + ), + grossPerformanceWithCurrencyEffect: new Big('-12.6'), + investment: new Big('0'), + investmentWithCurrencyEffect: new Big('0'), + netPerformance: new Big('-15.8'), + netPerformancePercentage: new Big('-0.05528341497550734703'), + netPerformancePercentageWithCurrencyEffect: new Big( + '-0.05528341497550734703' + ), + netPerformanceWithCurrencyEffect: new Big('-15.8'), + marketPrice: 148.9, + marketPriceInBaseCurrency: 148.9, + quantity: new Big('0'), + symbol: 'BALN.SW', + timeWeightedInvestment: new Big('285.80000000000000396627'), + timeWeightedInvestmentWithCurrencyEffect: new Big( + '285.80000000000000396627' + ), + transactionCount: 3, + valueInBaseCurrency: new Big('0') + } + ], + totalInvestment: new Big('0'), + totalInvestmentWithCurrencyEffect: new Big('0') + }); + + expect(investments).toEqual([ + { date: '2021-11-22', investment: new Big('285.8') }, + { date: '2021-11-30', investment: new Big('0') } + ]); + + expect(investmentsByMonth).toEqual([ + { date: '2021-11-01', investment: 0 }, + { date: '2021-12-01', investment: 0 } + ]); + }); + }); +}); diff --git a/apps/api/src/app/portfolio/portfolio-calculator.ts b/apps/api/src/app/portfolio/portfolio-calculator.ts index 1521787e7..faf954bbd 100644 --- a/apps/api/src/app/portfolio/portfolio-calculator.ts +++ b/apps/api/src/app/portfolio/portfolio-calculator.ts @@ -893,13 +893,10 @@ export class PortfolioCalculator { } = {}; let totalInvestment = new Big(0); + let totalInvestmentFromBuyTransactions = new Big(0); + let totalInvestmentFromBuyTransactionsWithCurrencyEffect = new Big(0); let totalInvestmentWithCurrencyEffect = new Big(0); - let totalInvestmentWithGrossPerformanceFromSell = new Big(0); - - let totalInvestmentWithGrossPerformanceFromSellWithCurrencyEffect = new Big( - 0 - ); - + let totalQuantityFromBuyTransactions = new Big(0); let totalUnits = new Big(0); let valueAtStartDate: Big; let valueAtStartDateWithCurrencyEffect: Big; @@ -1138,9 +1135,21 @@ export class PortfolioCalculator { transactionInvestment = order.quantity .mul(order.unitPriceInBaseCurrency) .mul(getFactor(order.type)); + transactionInvestmentWithCurrencyEffect = order.quantity .mul(order.unitPriceInBaseCurrencyWithCurrencyEffect) .mul(getFactor(order.type)); + + totalQuantityFromBuyTransactions = + totalQuantityFromBuyTransactions.plus(order.quantity); + + totalInvestmentFromBuyTransactions = + totalInvestmentFromBuyTransactions.plus(transactionInvestment); + + totalInvestmentFromBuyTransactionsWithCurrencyEffect = + totalInvestmentFromBuyTransactionsWithCurrencyEffect.plus( + transactionInvestmentWithCurrencyEffect + ); } else if (order.type === 'SELL') { if (totalUnits.gt(0)) { transactionInvestment = totalInvestment @@ -1245,35 +1254,21 @@ export class PortfolioCalculator { grossPerformanceFromSellWithCurrencyEffect ); - totalInvestmentWithGrossPerformanceFromSell = - totalInvestmentWithGrossPerformanceFromSell - .plus(transactionInvestment) - .plus(grossPerformanceFromSell); - - totalInvestmentWithGrossPerformanceFromSellWithCurrencyEffect = - totalInvestmentWithGrossPerformanceFromSellWithCurrencyEffect - .plus(transactionInvestmentWithCurrencyEffect) - .plus(grossPerformanceFromSellWithCurrencyEffect); - - lastAveragePrice = totalUnits.eq(0) + lastAveragePrice = totalQuantityFromBuyTransactions.eq(0) ? new Big(0) - : totalInvestmentWithGrossPerformanceFromSell.div(totalUnits); + : totalInvestmentFromBuyTransactions.div( + totalQuantityFromBuyTransactions + ); - lastAveragePriceWithCurrencyEffect = totalUnits.eq(0) + lastAveragePriceWithCurrencyEffect = totalQuantityFromBuyTransactions.eq( + 0 + ) ? new Big(0) - : totalInvestmentWithGrossPerformanceFromSellWithCurrencyEffect.div( - totalUnits + : totalInvestmentFromBuyTransactionsWithCurrencyEffect.div( + totalQuantityFromBuyTransactions ); if (PortfolioCalculator.ENABLE_LOGGING) { - console.log( - 'totalInvestmentWithGrossPerformanceFromSell', - totalInvestmentWithGrossPerformanceFromSell.toNumber() - ); - console.log( - 'totalInvestmentWithGrossPerformanceFromSellWithCurrencyEffect', - totalInvestmentWithGrossPerformanceFromSellWithCurrencyEffect.toNumber() - ); console.log( 'grossPerformanceFromSells', grossPerformanceFromSells.toNumber() @@ -1319,11 +1314,10 @@ export class PortfolioCalculator { orderDate, previousOrderDate ); - - // Set to at least 1 day, otherwise the transactions on the same day - // would not be considered in the time weighted calculation if (daysSinceLastOrder <= 0) { - daysSinceLastOrder = 1; + // The time between two activities on the same day is unknown + // -> Set it to the smallest floating point number greater than 0 + daysSinceLastOrder = Number.EPSILON; } // Sum up the total investment days since the start date to calculate