Browse Source

Bugfix/prevent double counting of cash in net worth (#6171)

* Prevent double counting of cash in net worth

* Update changelog
pull/6184/head
Kenrick Tandrian 2 days ago
committed by GitHub
parent
commit
645e8ee303
No known key found for this signature in database GPG Key ID: B5690EEEBB952194
  1. 1
      CHANGELOG.md
  2. 49
      apps/api/src/app/order/order.service.ts
  3. 33
      apps/api/src/app/portfolio/calculator/portfolio-calculator.ts
  4. 148
      apps/api/src/app/portfolio/calculator/roai/portfolio-calculator-cash.spec.ts
  5. 6
      apps/api/src/app/portfolio/calculator/roai/portfolio-calculator.ts
  6. 6
      apps/api/src/services/exchange-rate-data/exchange-rate-data.service.mock.ts
  7. 3
      apps/api/src/services/queues/portfolio-snapshot/portfolio-snapshot.processor.ts
  8. 2
      libs/common/src/lib/models/timeline-position.ts

1
CHANGELOG.md

@ -18,6 +18,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
### Fixed ### Fixed
- Fixed the net worth calculation to prevent the double counting of cash positions
- Fixed the filtering by asset class in the endpoint `GET api/v1/portfolio/holdings` - Fixed the filtering by asset class in the endpoint `GET api/v1/portfolio/holdings`
- Fixed the case-insensitive sorting in the accounts table component - Fixed the case-insensitive sorting in the accounts table component
- Fixed the case-insensitive sorting in the benchmark component - Fixed the case-insensitive sorting in the benchmark component

49
apps/api/src/app/order/order.service.ts

@ -743,47 +743,50 @@ export class OrderService {
/** /**
* Retrieves all orders required for the portfolio calculator, including both standard asset orders * Retrieves all orders required for the portfolio calculator, including both standard asset orders
* and synthetic orders representing cash activities. * and optional synthetic orders representing cash activities.
*
* @param filters - Optional filters to apply to the orders.
* @param userCurrency - The base currency of the user.
* @param userId - The ID of the user.
* @returns An object containing the combined list of activities and the total count.
*/ */
@LogPerformance @LogPerformance
public async getOrdersForPortfolioCalculator({ public async getOrdersForPortfolioCalculator({
filters, filters,
userCurrency, userCurrency,
userId userId,
withCash = false
}: { }: {
/** Optional filters to apply to the orders. */
filters?: Filter[]; filters?: Filter[];
/** The base currency of the user. */
userCurrency: string; userCurrency: string;
/** The ID of the user. */
userId: string; userId: string;
/** Whether to include cash activities in the result. */
withCash?: boolean;
}) { }) {
const nonCashOrders = await this.getOrders({ const orders = await this.getOrders({
filters, filters,
userCurrency, userCurrency,
userId, userId,
withExcludedAccountsAndActivities: false // TODO withExcludedAccountsAndActivities: false // TODO
}); });
const cashDetails = await this.accountService.getCashDetails({ if (withCash) {
filters, const cashDetails = await this.accountService.getCashDetails({
userId, filters,
currency: userCurrency userId,
}); currency: userCurrency
});
const cashOrders = await this.getCashOrders({ const cashOrders = await this.getCashOrders({
cashDetails, cashDetails,
filters, filters,
userCurrency, userCurrency,
userId userId
}); });
return { orders.activities.push(...cashOrders.activities);
activities: [...nonCashOrders.activities, ...cashOrders.activities], orders.count += cashOrders.count;
count: nonCashOrders.count + cashOrders.count }
};
return orders;
} }
public async getStatisticsByCurrency( public async getStatisticsByCurrency(

33
apps/api/src/app/portfolio/calculator/portfolio-calculator.ts

@ -39,6 +39,7 @@ import { GroupBy } from '@ghostfolio/common/types';
import { PerformanceCalculationType } from '@ghostfolio/common/types/performance-calculation-type.type'; import { PerformanceCalculationType } from '@ghostfolio/common/types/performance-calculation-type.type';
import { Logger } from '@nestjs/common'; import { Logger } from '@nestjs/common';
import { AssetSubClass } from '@prisma/client';
import { Big } from 'big.js'; import { Big } from 'big.js';
import { plainToClass } from 'class-transformer'; import { plainToClass } from 'class-transformer';
import { import {
@ -389,27 +390,33 @@ export abstract class PortfolioCalculator {
hasAnySymbolMetricsErrors = hasAnySymbolMetricsErrors || hasErrors; hasAnySymbolMetricsErrors = hasAnySymbolMetricsErrors || hasErrors;
valuesBySymbol[item.symbol] = { const includeInTotalAssetValue =
currentValues, item.assetSubClass !== AssetSubClass.CASH;
currentValuesWithCurrencyEffect,
investmentValuesAccumulated, if (includeInTotalAssetValue) {
investmentValuesAccumulatedWithCurrencyEffect, valuesBySymbol[item.symbol] = {
investmentValuesWithCurrencyEffect, currentValues,
netPerformanceValues, currentValuesWithCurrencyEffect,
netPerformanceValuesWithCurrencyEffect, investmentValuesAccumulated,
timeWeightedInvestmentValues, investmentValuesAccumulatedWithCurrencyEffect,
timeWeightedInvestmentValuesWithCurrencyEffect investmentValuesWithCurrencyEffect,
}; netPerformanceValues,
netPerformanceValuesWithCurrencyEffect,
timeWeightedInvestmentValues,
timeWeightedInvestmentValuesWithCurrencyEffect
};
}
positions.push({ positions.push({
feeInBaseCurrency, feeInBaseCurrency,
includeInTotalAssetValue,
timeWeightedInvestment, timeWeightedInvestment,
timeWeightedInvestmentWithCurrencyEffect, timeWeightedInvestmentWithCurrencyEffect,
dividend: totalDividend,
dividendInBaseCurrency: totalDividendInBaseCurrency,
averagePrice: item.averagePrice, averagePrice: item.averagePrice,
currency: item.currency, currency: item.currency,
dataSource: item.dataSource, dataSource: item.dataSource,
dividend: totalDividend,
dividendInBaseCurrency: totalDividendInBaseCurrency,
fee: item.fee, fee: item.fee,
firstBuyDate: item.firstBuyDate, firstBuyDate: item.firstBuyDate,
grossPerformance: !hasErrors ? (grossPerformance ?? null) : null, grossPerformance: !hasErrors ? (grossPerformance ?? null) : null,

148
apps/api/src/app/portfolio/calculator/roai/portfolio-calculator-cash.spec.ts

@ -14,7 +14,7 @@ import { ExchangeRateDataServiceMock } from '@ghostfolio/api/services/exchange-r
import { PortfolioSnapshotService } from '@ghostfolio/api/services/queues/portfolio-snapshot/portfolio-snapshot.service'; import { PortfolioSnapshotService } from '@ghostfolio/api/services/queues/portfolio-snapshot/portfolio-snapshot.service';
import { PortfolioSnapshotServiceMock } from '@ghostfolio/api/services/queues/portfolio-snapshot/portfolio-snapshot.service.mock'; import { PortfolioSnapshotServiceMock } from '@ghostfolio/api/services/queues/portfolio-snapshot/portfolio-snapshot.service.mock';
import { parseDate } from '@ghostfolio/common/helper'; import { parseDate } from '@ghostfolio/common/helper';
import { HistoricalDataItem } from '@ghostfolio/common/interfaces'; import { TimelinePosition } from '@ghostfolio/common/models';
import { PerformanceCalculationType } from '@ghostfolio/common/types/performance-calculation-type.type'; import { PerformanceCalculationType } from '@ghostfolio/common/types/performance-calculation-type.type';
import { DataSource } from '@prisma/client'; import { DataSource } from '@prisma/client';
@ -191,7 +191,8 @@ describe('PortfolioCalculator', () => {
const { activities } = await orderService.getOrdersForPortfolioCalculator( const { activities } = await orderService.getOrdersForPortfolioCalculator(
{ {
userCurrency: 'CHF', userCurrency: 'CHF',
userId: userDummyData.id userId: userDummyData.id,
withCash: true
} }
); );
@ -201,7 +202,14 @@ describe('PortfolioCalculator', () => {
values: [] values: []
}); });
const accountBalanceItems =
await accountBalanceService.getAccountBalanceItems({
userCurrency: 'CHF',
userId: userDummyData.id
});
const portfolioCalculator = portfolioCalculatorFactory.createCalculator({ const portfolioCalculator = portfolioCalculatorFactory.createCalculator({
accountBalanceItems,
activities, activities,
calculationType: PerformanceCalculationType.ROAI, calculationType: PerformanceCalculationType.ROAI,
currency: 'CHF', currency: 'CHF',
@ -210,94 +218,72 @@ describe('PortfolioCalculator', () => {
const portfolioSnapshot = await portfolioCalculator.computeSnapshot(); const portfolioSnapshot = await portfolioCalculator.computeSnapshot();
const historicalData20231231 = portfolioSnapshot.historicalData.find( const position = portfolioSnapshot.positions.find(({ symbol }) => {
({ date }) => { return symbol === 'USD';
return date === '2023-12-31';
}
);
const historicalData20240101 = portfolioSnapshot.historicalData.find(
({ date }) => {
return date === '2024-01-01';
}
);
const historicalData20241231 = portfolioSnapshot.historicalData.find(
({ date }) => {
return date === '2024-12-31';
}
);
/**
* Investment value with currency effect: 1000 USD * 0.85 = 850 CHF
* Total investment: 1000 USD * 0.91 = 910 CHF
* Value (current): 1000 USD * 0.91 = 910 CHF
* Value with currency effect: 1000 USD * 0.85 = 850 CHF
*/
expect(historicalData20231231).toMatchObject({
date: '2023-12-31',
investmentValueWithCurrencyEffect: 850,
netPerformance: 0,
netPerformanceInPercentage: 0,
netPerformanceInPercentageWithCurrencyEffect: 0,
netPerformanceWithCurrencyEffect: 0,
netWorth: 850,
totalAccountBalance: 0,
totalInvestment: 910,
totalInvestmentValueWithCurrencyEffect: 850,
value: 910,
valueWithCurrencyEffect: 850
});
/**
* Net performance with currency effect: (1000 * 0.86) - (1000 * 0.85) = 10 CHF
* Total investment: 1000 USD * 0.91 = 910 CHF
* Total investment value with currency effect: 1000 USD * 0.85 = 850 CHF
* Value (current): 1000 USD * 0.91 = 910 CHF
* Value with currency effect: 1000 USD * 0.86 = 860 CHF
*/
expect(historicalData20240101).toMatchObject({
date: '2024-01-01',
investmentValueWithCurrencyEffect: 0,
netPerformance: 0,
netPerformanceInPercentage: 0,
netPerformanceInPercentageWithCurrencyEffect: 0.011764705882352941,
netPerformanceWithCurrencyEffect: 10,
netWorth: 860,
totalAccountBalance: 0,
totalInvestment: 910,
totalInvestmentValueWithCurrencyEffect: 850,
value: 910,
valueWithCurrencyEffect: 860
}); });
/** /**
* Investment value with currency effect: 1000 USD * 0.90 = 900 CHF * Investment: 2000 USD * 0.91 = 1820 CHF
* Investment value with currency effect: (1000 USD * 0.85) + (1000 USD * 0.90) = 1750 CHF
* Net performance: (1000 USD * 1.0) - (1000 USD * 1.0) = 0 CHF * Net performance: (1000 USD * 1.0) - (1000 USD * 1.0) = 0 CHF
* Net performance with currency effect: (1000 USD * 0.9) - (1000 USD * 0.85) = 50 CHF * Total account balance: 2000 USD * 0.85 = 1700 CHF (using the exchange rate on 2024-12-31)
* Total investment: 2000 USD * 0.91 = 1820 CHF * Value in base currency: 2000 USD * 0.91 = 1820 CHF
* Total investment value with currency effect: (1000 USD * 0.85) + (1000 USD * 0.90) = 1750 CHF
* Value (current): 2000 USD * 0.91 = 1820 CHF
* Value with currency effect: 2000 USD * 0.9 = 1800 CHF
*/ */
expect(historicalData20241231).toMatchObject<HistoricalDataItem>({ expect(position).toMatchObject<TimelinePosition>({
date: '2024-12-31', averagePrice: new Big(1),
investmentValueWithCurrencyEffect: 900, currency: 'USD',
netPerformance: 0, dataSource: DataSource.YAHOO,
netPerformanceInPercentage: 0, dividend: new Big(0),
netPerformanceInPercentageWithCurrencyEffect: 0.058823529411764705, dividendInBaseCurrency: new Big(0),
netPerformanceWithCurrencyEffect: 50, fee: new Big(0),
netWorth: 1800, feeInBaseCurrency: new Big(0),
totalAccountBalance: 0, firstBuyDate: '2023-12-31',
totalInvestment: 1820, grossPerformance: new Big(0),
totalInvestmentValueWithCurrencyEffect: 1750, grossPerformancePercentage: new Big(0),
value: 1820, grossPerformancePercentageWithCurrencyEffect: new Big(
valueWithCurrencyEffect: 1800 '0.08211603004634809014'
),
grossPerformanceWithCurrencyEffect: new Big(70),
includeInTotalAssetValue: false,
investment: new Big(1820),
investmentWithCurrencyEffect: new Big(1750),
marketPrice: null,
marketPriceInBaseCurrency: 0.91,
netPerformance: new Big(0),
netPerformancePercentage: new Big(0),
netPerformancePercentageWithCurrencyEffectMap: {
'1d': new Big('0.01111111111111111111'),
'1y': new Big('0.06937181021989792704'),
'5y': new Big('0.0818817546090273363'),
max: new Big('0.0818817546090273363'),
mtd: new Big('0.01111111111111111111'),
wtd: new Big('-0.05517241379310344828'),
ytd: new Big('0.01111111111111111111')
},
netPerformanceWithCurrencyEffectMap: {
'1d': new Big(20),
'1y': new Big(60),
'5y': new Big(70),
max: new Big(70),
mtd: new Big(20),
wtd: new Big(-80),
ytd: new Big(20)
},
quantity: new Big(2000),
symbol: 'USD',
timeWeightedInvestment: new Big('912.47956403269754768392'),
timeWeightedInvestmentWithCurrencyEffect: new Big(
'852.45231607629427792916'
),
transactionCount: 2,
valueInBaseCurrency: new Big(1820)
}); });
expect(portfolioSnapshot).toMatchObject({ expect(portfolioSnapshot).toMatchObject({
hasErrors: false, hasErrors: false,
totalFeesWithCurrencyEffect: new Big('0'), totalFeesWithCurrencyEffect: new Big(0),
totalInterestWithCurrencyEffect: new Big('0'), totalInterestWithCurrencyEffect: new Big(0),
totalLiabilitiesWithCurrencyEffect: new Big('0') totalLiabilitiesWithCurrencyEffect: new Big(0)
}); });
}); });
}); });

6
apps/api/src/app/portfolio/calculator/roai/portfolio-calculator.ts

@ -34,7 +34,11 @@ export class RoaiPortfolioCalculator extends PortfolioCalculator {
let totalTimeWeightedInvestment = new Big(0); let totalTimeWeightedInvestment = new Big(0);
let totalTimeWeightedInvestmentWithCurrencyEffect = new Big(0); let totalTimeWeightedInvestmentWithCurrencyEffect = new Big(0);
for (const currentPosition of positions) { for (const currentPosition of positions.filter(
({ includeInTotalAssetValue }) => {
return includeInTotalAssetValue;
}
)) {
if (currentPosition.feeInBaseCurrency) { if (currentPosition.feeInBaseCurrency) {
totalFeesWithCurrencyEffect = totalFeesWithCurrencyEffect.plus( totalFeesWithCurrencyEffect = totalFeesWithCurrencyEffect.plus(
currentPosition.feeInBaseCurrency currentPosition.feeInBaseCurrency

6
apps/api/src/services/exchange-rate-data/exchange-rate-data.service.mock.ts

@ -1,5 +1,7 @@
export const ExchangeRateDataServiceMock = { import { ExchangeRateDataService } from './exchange-rate-data.service';
getExchangeRatesByCurrency: ({ targetCurrency }): Promise<any> => {
export const ExchangeRateDataServiceMock: Partial<ExchangeRateDataService> = {
getExchangeRatesByCurrency: ({ targetCurrency }) => {
if (targetCurrency === 'CHF') { if (targetCurrency === 'CHF') {
return Promise.resolve({ return Promise.resolve({
CHFCHF: { CHFCHF: {

3
apps/api/src/services/queues/portfolio-snapshot/portfolio-snapshot.processor.ts

@ -50,7 +50,8 @@ export class PortfolioSnapshotProcessor {
await this.orderService.getOrdersForPortfolioCalculator({ await this.orderService.getOrdersForPortfolioCalculator({
filters: job.data.filters, filters: job.data.filters,
userCurrency: job.data.userCurrency, userCurrency: job.data.userCurrency,
userId: job.data.userId userId: job.data.userId,
withCash: true
}); });
const accountBalanceItems = const accountBalanceItems =

2
libs/common/src/lib/models/timeline-position.ts

@ -50,6 +50,8 @@ export class TimelinePosition {
@Type(() => Big) @Type(() => Big)
grossPerformanceWithCurrencyEffect: Big; grossPerformanceWithCurrencyEffect: Big;
includeInTotalAssetValue?: boolean;
@Transform(transformToBig, { toClassOnly: true }) @Transform(transformToBig, { toClassOnly: true })
@Type(() => Big) @Type(() => Big)
investment: Big; investment: Big;

Loading…
Cancel
Save