Browse Source

Fix portfolio calculation for holdings with same symbol

pull/7664/head
Thomas Kaul 4 days ago
parent
commit
942cdfbf5a
  1. 2
      apps/api/src/app/endpoints/ai/ai.service.ts
  2. 2
      apps/api/src/app/portfolio/calculator/portfolio-calculator.ts
  3. 174
      apps/api/src/app/portfolio/calculator/roai/portfolio-calculator-msft-buy-from-two-data-sources.spec.ts
  4. 31
      apps/api/src/app/portfolio/portfolio.service.spec.ts
  5. 2
      apps/api/src/app/portfolio/portfolio.service.ts
  6. 2
      apps/client/src/app/pages/portfolio/allocations/allocations-page.component.ts

2
apps/api/src/app/endpoints/ai/ai.service.ts

@ -118,7 +118,7 @@ export class AiService {
values: Object.values(AssetSubClass) values: Object.values(AssetSubClass)
}); });
const holdingsTableRows = holdings const holdingsTableRows = [...holdings]
.sort((a, b) => { .sort((a, b) => {
return b.allocationInPercentage - a.allocationInPercentage; return b.allocationInPercentage - a.allocationInPercentage;
}) })

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

@ -1109,7 +1109,7 @@ export abstract class PortfolioCalculator {
const items = lastTransactionPoint?.items ?? []; const items = lastTransactionPoint?.items ?? [];
const newItems = items.filter((item) => { const newItems = items.filter((item) => {
return getAssetProfileIdentifier(item) !== assetProfileIdentifier; return item.dataSource !== dataSource || item.symbol !== symbol;
}); });
newItems.push(currentTransactionPointItem); newItems.push(currentTransactionPointItem);

174
apps/api/src/app/portfolio/calculator/roai/portfolio-calculator-msft-buy-from-two-data-sources.spec.ts

@ -0,0 +1,174 @@
import {
activityDummyData,
assetProfileDummyData,
userDummyData
} from '@ghostfolio/api/app/portfolio/calculator/portfolio-calculator-test-utils';
import { PortfolioCalculatorFactory } from '@ghostfolio/api/app/portfolio/calculator/portfolio-calculator.factory';
import { CurrentRateService } from '@ghostfolio/api/app/portfolio/current-rate.service';
import { CurrentRateServiceMock } from '@ghostfolio/api/app/portfolio/current-rate.service.mock';
import { RedisCacheService } from '@ghostfolio/api/app/redis-cache/redis-cache.service';
import { RedisCacheServiceMock } from '@ghostfolio/api/app/redis-cache/redis-cache.service.mock';
import { ConfigurationService } from '@ghostfolio/api/services/configuration/configuration.service';
import { ExchangeRateDataService } from '@ghostfolio/api/services/exchange-rate-data/exchange-rate-data.service';
import { ExchangeRateDataServiceMock } from '@ghostfolio/api/services/exchange-rate-data/exchange-rate-data.service.mock';
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 { parseDate } from '@ghostfolio/common/helper';
import { Activity } from '@ghostfolio/common/interfaces';
import { PerformanceCalculationType } from '@ghostfolio/common/types/performance-calculation-type.type';
import { Big } from 'big.js';
jest.mock('@ghostfolio/api/app/portfolio/current-rate.service', () => {
return {
CurrentRateService: jest.fn().mockImplementation(() => {
return CurrentRateServiceMock;
})
};
});
jest.mock(
'@ghostfolio/api/services/queues/portfolio-snapshot/portfolio-snapshot.service',
() => {
return {
PortfolioSnapshotService: jest.fn().mockImplementation(() => {
return PortfolioSnapshotServiceMock;
})
};
}
);
jest.mock('@ghostfolio/api/app/redis-cache/redis-cache.service', () => {
return {
RedisCacheService: jest.fn().mockImplementation(() => {
return RedisCacheServiceMock;
})
};
});
jest.mock(
'@ghostfolio/api/services/exchange-rate-data/exchange-rate-data.service',
() => {
return {
ExchangeRateDataService: jest.fn().mockImplementation(() => {
return ExchangeRateDataServiceMock;
})
};
}
);
describe('PortfolioCalculator', () => {
let configurationService: ConfigurationService;
let currentRateService: CurrentRateService;
let exchangeRateDataService: ExchangeRateDataService;
let portfolioCalculatorFactory: PortfolioCalculatorFactory;
let portfolioSnapshotService: PortfolioSnapshotService;
let redisCacheService: RedisCacheService;
beforeEach(() => {
PortfolioSnapshotServiceMock.reset();
RedisCacheServiceMock.reset();
configurationService = new ConfigurationService();
currentRateService = new CurrentRateService(null, null, null, null);
exchangeRateDataService = new ExchangeRateDataService(
null,
null,
null,
null
);
portfolioSnapshotService = new PortfolioSnapshotService(null, null);
redisCacheService = new RedisCacheService(null, null);
portfolioCalculatorFactory = new PortfolioCalculatorFactory(
configurationService,
currentRateService,
exchangeRateDataService,
portfolioSnapshotService,
redisCacheService
);
});
describe('get current positions', () => {
it.only('with MSFT buy from two data sources', async () => {
jest.useFakeTimers().setSystemTime(parseDate('2023-07-10').getTime());
const activities: Activity[] = [
{
...activityDummyData,
assetProfile: {
...assetProfileDummyData,
currency: 'USD',
dataSource: 'YAHOO',
name: 'Microsoft Inc.',
symbol: 'MSFT'
},
date: new Date('2021-11-16'),
feeInAssetProfileCurrency: 0,
feeInBaseCurrency: 0,
quantity: 1,
type: 'BUY',
unitPriceInAssetProfileCurrency: 339.51
},
{
...activityDummyData,
assetProfile: {
...assetProfileDummyData,
currency: 'USD',
dataSource: 'EOD_HISTORICAL_DATA',
name: 'Microsoft Inc.',
symbol: 'MSFT'
},
date: new Date('2021-11-16'),
feeInAssetProfileCurrency: 0,
feeInBaseCurrency: 0,
quantity: 2,
type: 'BUY',
unitPriceInAssetProfileCurrency: 339.51
}
];
const portfolioCalculator = portfolioCalculatorFactory.createCalculator({
activities,
calculationType: PerformanceCalculationType.ROAI,
currency: 'USD',
userId: userDummyData.id
});
const portfolioSnapshot = await portfolioCalculator.computeSnapshot();
// The holdings must not be aggregated, because they belong to two
// different asset profiles
expect(portfolioSnapshot.positions).toEqual([
expect.objectContaining({
activitiesCount: 1,
dataSource: 'EOD_HISTORICAL_DATA',
investment: new Big('679.02'),
marketPrice: 331.83,
quantity: new Big('2'),
symbol: 'MSFT',
valueInBaseCurrency: new Big('663.66')
}),
expect.objectContaining({
activitiesCount: 1,
dataSource: 'YAHOO',
investment: new Big('339.51'),
marketPrice: 331.83,
quantity: new Big('1'),
symbol: 'MSFT',
valueInBaseCurrency: new Big('331.83')
})
]);
expect(portfolioSnapshot.currentValueInBaseCurrency).toEqual(
new Big('995.49')
);
expect(portfolioSnapshot.totalInvestment).toEqual(new Big('1018.53'));
});
});
});

31
apps/api/src/app/portfolio/portfolio.service.spec.ts

@ -9,7 +9,7 @@ import { ConfigurationService } from '@ghostfolio/api/services/configuration/con
import { DataProviderService } from '@ghostfolio/api/services/data-provider/data-provider.service'; import { DataProviderService } from '@ghostfolio/api/services/data-provider/data-provider.service';
import { ExchangeRateDataService } from '@ghostfolio/api/services/exchange-rate-data/exchange-rate-data.service'; import { ExchangeRateDataService } from '@ghostfolio/api/services/exchange-rate-data/exchange-rate-data.service';
import { SymbolProfileService } from '@ghostfolio/api/services/symbol-profile/symbol-profile.service'; import { SymbolProfileService } from '@ghostfolio/api/services/symbol-profile/symbol-profile.service';
import { UNKNOWN_KEY } from '@ghostfolio/common/config'; import { TAG_ID_EMERGENCY_FUND, UNKNOWN_KEY } from '@ghostfolio/common/config';
import { parseDate } from '@ghostfolio/common/helper'; import { parseDate } from '@ghostfolio/common/helper';
import { import {
AssetProfileIdentifier, AssetProfileIdentifier,
@ -214,15 +214,16 @@ describe('PortfolioService', () => {
}); });
describe('getDetails', () => { describe('getDetails', () => {
it('should return cash holdings when the calculator emits cash positions with the exchange-rate data source', async () => { const setUpCashOnlyPortfolio = ({
const accountId = randomUUID(); baseCurrency = 'CHF',
emergencyFund
}: { baseCurrency?: string; emergencyFund?: number } = {}) => {
const cashAccount: AccountWithBalance = { const cashAccount: AccountWithBalance = {
balance: 2000, balance: 2000,
comment: null, comment: null,
createdAt: parseDate('2024-01-01'), createdAt: parseDate('2024-01-01'),
currency: 'USD', currency: 'USD',
id: accountId, id: randomUUID(),
name: 'USD', name: 'USD',
platformId: null, platformId: null,
updatedAt: parseDate('2024-01-01'), updatedAt: parseDate('2024-01-01'),
@ -253,7 +254,8 @@ describe('PortfolioService', () => {
id: userDummyData.id, id: userDummyData.id,
settings: { settings: {
settings: { settings: {
baseCurrency: 'CHF' baseCurrency,
emergencyFund
} }
} }
} as unknown as Awaited<ReturnType<typeof userService.user>>); } as unknown as Awaited<ReturnType<typeof userService.user>>);
@ -320,6 +322,10 @@ describe('PortfolioService', () => {
'getValueOfAccountsAndPlatforms' 'getValueOfAccountsAndPlatforms'
) )
.mockResolvedValue({ accounts: {}, platforms: {} }); .mockResolvedValue({ accounts: {}, platforms: {} });
};
it('should return cash holdings when the calculator emits cash positions with the exchange-rate data source', async () => {
setUpCashOnlyPortfolio();
const { holdings } = await portfolioService.getDetails({ const { holdings } = await portfolioService.getDetails({
filters: [], filters: [],
@ -335,6 +341,19 @@ describe('PortfolioService', () => {
}) })
]); ]);
}); });
it('should replace the existing cash holding instead of adding a second one when filtering by the emergency fund tag', async () => {
setUpCashOnlyPortfolio({ baseCurrency: 'USD', emergencyFund: 1000 });
const { holdings } = await portfolioService.getDetails({
filters: [{ id: TAG_ID_EMERGENCY_FUND, type: 'TAG' }],
userId: userDummyData.id
});
expect(holdings).toHaveLength(1);
expect(holdings[0].assetProfile.symbol).toBe('USD');
expect(holdings[0].valueInBaseCurrency).toBe(1000);
});
}); });
describe('getSummary', () => { describe('getSummary', () => {

2
apps/api/src/app/portfolio/portfolio.service.ts

@ -1765,7 +1765,7 @@ export class PortfolioService {
assetClass: AssetClass.LIQUIDITY, assetClass: AssetClass.LIQUIDITY,
assetSubClass: AssetSubClass.CASH, assetSubClass: AssetSubClass.CASH,
countries: [], countries: [],
dataSource: undefined, dataSource: this.dataProviderService.getDataSourceForExchangeRates(),
holdings: [], holdings: [],
name: currency, name: currency,
sectors: [], sectors: [],

2
apps/client/src/app/pages/portfolio/allocations/allocations-page.component.ts

@ -516,6 +516,8 @@ export class GfAllocationsPageComponent implements OnInit {
if (symbolData) { if (symbolData) {
// Aggregate holdings with the same symbol from different data sources // Aggregate holdings with the same symbol from different data sources
symbolData.dataSource = undefined;
symbolData.isClickable = false;
symbolData.value += value; symbolData.value += value;
} else { } else {
this.symbols[symbol] = { this.symbols[symbol] = {

Loading…
Cancel
Save