From 28287c5dc59af02246b1b6f13075d3401cdd32a8 Mon Sep 17 00:00:00 2001 From: Rene Turcios <95189389+CloudCorpRecords@users.noreply.github.com> Date: Sat, 17 Oct 2026 14:27:37 -0700 Subject: [PATCH] Bugfix/exclude crypto and cash from market and continent distribution (#8141) The Market distribution listed crypto and cash (via the unknown bucket) and the Continent distribution listed crypto allocations, although neither can be assigned to a specific market or continent. - getAggregatedMarkets now skips cash and cryptocurrency positions so they no longer appear in the market distribution; percentages are guarded against a zero total - the allocations page now excludes cryptocurrencies (like cash) from the geographic analysis (continents, countries, sectors) Signed-off-by: Rene Turcios <95189389+CloudCorpRecords@users.noreply.github.com> --- .../app/portfolio/portfolio.service.spec.ts | 141 +++++++++++++++++- .../src/app/portfolio/portfolio.service.ts | 82 +++++++--- .../allocations/allocations-page.component.ts | 19 ++- 3 files changed, 211 insertions(+), 31 deletions(-) diff --git a/apps/api/src/app/portfolio/portfolio.service.spec.ts b/apps/api/src/app/portfolio/portfolio.service.spec.ts index 12d2b3147e..9b6025bf6b 100644 --- a/apps/api/src/app/portfolio/portfolio.service.spec.ts +++ b/apps/api/src/app/portfolio/portfolio.service.spec.ts @@ -25,7 +25,7 @@ import { } from '@ghostfolio/common/interfaces'; import { AccountWithBalance, DateRange } from '@ghostfolio/common/types'; -import { AssetClass, DataSource, Prisma } from '@prisma/client'; +import { AssetClass, AssetSubClass, DataSource, Prisma } from '@prisma/client'; import { Big } from 'big.js'; import { endOfDay } from 'date-fns'; import { randomUUID } from 'node:crypto'; @@ -346,7 +346,7 @@ describe('PortfolioService', () => { ).getAggregatedMarkets(holdings); }; - it('should distribute holdings with countries to their market and route holdings without countries (e.g. commodities, cryptocurrencies) to the unknown bucket', () => { + it('should distribute holdings with countries to their market and route holdings without countries (e.g. commodities) to the unknown bucket', () => { const holdings = [ { // Gold (GC=F) @@ -391,6 +391,143 @@ describe('PortfolioService', () => { expect(marketsAdvanced.northAmerica.valueInBaseCurrency).toBe(1000); expect(marketsAdvanced[UNKNOWN_KEY].valueInBaseCurrency).toBe(500); }); + + it('should exclude cash and cryptocurrency holdings from the market distribution', () => { + const holdings = [ + { + // BTC + assetProfile: { + assetSubClass: AssetSubClass.CRYPTOCURRENCY, + countries: [] + }, + markets: { + developedMarkets: 0, + emergingMarkets: 0, + otherMarkets: 0 + }, + marketsAdvanced: { + asiaPacific: 0, + emergingMarkets: 0, + europe: 0, + japan: 0, + northAmerica: 0, + otherMarkets: 0 + }, + valueInBaseCurrency: 500 + }, + { + // Cash (USD) + assetProfile: { + assetSubClass: AssetSubClass.CASH, + countries: [] + }, + markets: { + developedMarkets: 0, + emergingMarkets: 0, + otherMarkets: 0 + }, + marketsAdvanced: { + asiaPacific: 0, + emergingMarkets: 0, + europe: 0, + japan: 0, + northAmerica: 0, + otherMarkets: 0 + }, + valueInBaseCurrency: 250 + }, + { + // MSFT + assetProfile: { countries: [{ code: 'US', weight: 1 }] }, + markets: { + developedMarkets: 1, + emergingMarkets: 0, + otherMarkets: 0 + }, + marketsAdvanced: { + asiaPacific: 0, + emergingMarkets: 0, + europe: 0, + japan: 0, + northAmerica: 1, + otherMarkets: 0 + }, + valueInBaseCurrency: 1000 + } + ]; + + const { markets, marketsAdvanced } = getAggregatedMarkets(holdings); + + expect(markets.developedMarkets.valueInBaseCurrency).toBe(1000); + expect(markets[UNKNOWN_KEY].valueInBaseCurrency).toBe(0); + + expect(markets.developedMarkets.valueInPercentage).toBeCloseTo(1); + expect(markets[UNKNOWN_KEY].valueInPercentage).toBe(0); + + expect(marketsAdvanced.northAmerica.valueInBaseCurrency).toBe(1000); + expect(marketsAdvanced[UNKNOWN_KEY].valueInBaseCurrency).toBe(0); + expect(marketsAdvanced.northAmerica.valueInPercentage).toBeCloseTo(1); + expect(marketsAdvanced[UNKNOWN_KEY].valueInPercentage).toBe(0); + }); + + it('should not produce NaN percentages when the portfolio only holds cash and cryptocurrencies', () => { + const holdings = [ + { + // BTC + assetProfile: { + assetSubClass: AssetSubClass.CRYPTOCURRENCY, + countries: [] + }, + markets: { + developedMarkets: 0, + emergingMarkets: 0, + otherMarkets: 0 + }, + marketsAdvanced: { + asiaPacific: 0, + emergingMarkets: 0, + europe: 0, + japan: 0, + northAmerica: 0, + otherMarkets: 0 + }, + valueInBaseCurrency: 500 + }, + { + // Cash (USD) + assetProfile: { + assetSubClass: AssetSubClass.CASH, + countries: [] + }, + markets: { + developedMarkets: 0, + emergingMarkets: 0, + otherMarkets: 0 + }, + marketsAdvanced: { + asiaPacific: 0, + emergingMarkets: 0, + europe: 0, + japan: 0, + northAmerica: 0, + otherMarkets: 0 + }, + valueInBaseCurrency: 250 + } + ]; + + const { markets, marketsAdvanced } = getAggregatedMarkets(holdings); + + for (const bucket of Object.values(markets)) { + expect(bucket.valueInBaseCurrency).toBe(0); + expect(bucket.valueInPercentage).toBe(0); + } + + for (const bucket of Object.values(marketsAdvanced)) { + expect(bucket.valueInBaseCurrency).toBe(0); + expect(bucket.valueInPercentage).toBe(0); + } + }); }); describe('getCashSymbolProfiles', () => { diff --git a/apps/api/src/app/portfolio/portfolio.service.ts b/apps/api/src/app/portfolio/portfolio.service.ts index a8a3fc6740..e0bbaa516c 100644 --- a/apps/api/src/app/portfolio/portfolio.service.ts +++ b/apps/api/src/app/portfolio/portfolio.service.ts @@ -45,6 +45,7 @@ import { getAssetProfileIdentifier, getSum, isAccountExcluded, + isCashPosition, isDraftActivity, parseDate, resetHours, @@ -1682,6 +1683,14 @@ export class PortfolioService { for (const position of holdings) { const value = position.valueInBaseCurrency; + if ( + isCashPosition(position.assetProfile) || + position.assetProfile.assetSubClass === AssetSubClass.CRYPTOCURRENCY + ) { + // Cash and cryptocurrencies do not belong to any specific market + continue; + } + if (position.assetProfile.countries.length > 0) { markets.developedMarkets.valueInBaseCurrency += position.markets.developedMarkets * value; @@ -1714,14 +1723,28 @@ export class PortfolioService { }) ).toNumber(); - markets.developedMarkets.valueInPercentage = - markets.developedMarkets.valueInBaseCurrency / marketsTotalInBaseCurrency; - markets.emergingMarkets.valueInPercentage = - markets.emergingMarkets.valueInBaseCurrency / marketsTotalInBaseCurrency; - markets.otherMarkets.valueInPercentage = - markets.otherMarkets.valueInBaseCurrency / marketsTotalInBaseCurrency; - markets[UNKNOWN_KEY].valueInPercentage = - markets[UNKNOWN_KEY].valueInBaseCurrency / marketsTotalInBaseCurrency; + // The total can be zero when the portfolio only holds cash and / or + // cryptocurrencies, which are not assigned to any market + const toPercentage = (valueInBaseCurrency: number, total: number) => { + return total ? valueInBaseCurrency / total : 0; + }; + + markets.developedMarkets.valueInPercentage = toPercentage( + markets.developedMarkets.valueInBaseCurrency, + marketsTotalInBaseCurrency + ); + markets.emergingMarkets.valueInPercentage = toPercentage( + markets.emergingMarkets.valueInBaseCurrency, + marketsTotalInBaseCurrency + ); + markets.otherMarkets.valueInPercentage = toPercentage( + markets.otherMarkets.valueInBaseCurrency, + marketsTotalInBaseCurrency + ); + markets[UNKNOWN_KEY].valueInPercentage = toPercentage( + markets[UNKNOWN_KEY].valueInBaseCurrency, + marketsTotalInBaseCurrency + ); const marketsAdvancedTotal = marketsAdvanced.asiaPacific.valueInBaseCurrency + @@ -1732,21 +1755,34 @@ export class PortfolioService { marketsAdvanced.otherMarkets.valueInBaseCurrency + marketsAdvanced[UNKNOWN_KEY].valueInBaseCurrency; - marketsAdvanced.asiaPacific.valueInPercentage = - marketsAdvanced.asiaPacific.valueInBaseCurrency / marketsAdvancedTotal; - marketsAdvanced.emergingMarkets.valueInPercentage = - marketsAdvanced.emergingMarkets.valueInBaseCurrency / - marketsAdvancedTotal; - marketsAdvanced.europe.valueInPercentage = - marketsAdvanced.europe.valueInBaseCurrency / marketsAdvancedTotal; - marketsAdvanced.japan.valueInPercentage = - marketsAdvanced.japan.valueInBaseCurrency / marketsAdvancedTotal; - marketsAdvanced.northAmerica.valueInPercentage = - marketsAdvanced.northAmerica.valueInBaseCurrency / marketsAdvancedTotal; - marketsAdvanced.otherMarkets.valueInPercentage = - marketsAdvanced.otherMarkets.valueInBaseCurrency / marketsAdvancedTotal; - marketsAdvanced[UNKNOWN_KEY].valueInPercentage = - marketsAdvanced[UNKNOWN_KEY].valueInBaseCurrency / marketsAdvancedTotal; + marketsAdvanced.asiaPacific.valueInPercentage = toPercentage( + marketsAdvanced.asiaPacific.valueInBaseCurrency, + marketsAdvancedTotal + ); + marketsAdvanced.emergingMarkets.valueInPercentage = toPercentage( + marketsAdvanced.emergingMarkets.valueInBaseCurrency, + marketsAdvancedTotal + ); + marketsAdvanced.europe.valueInPercentage = toPercentage( + marketsAdvanced.europe.valueInBaseCurrency, + marketsAdvancedTotal + ); + marketsAdvanced.japan.valueInPercentage = toPercentage( + marketsAdvanced.japan.valueInBaseCurrency, + marketsAdvancedTotal + ); + marketsAdvanced.northAmerica.valueInPercentage = toPercentage( + marketsAdvanced.northAmerica.valueInBaseCurrency, + marketsAdvancedTotal + ); + marketsAdvanced.otherMarkets.valueInPercentage = toPercentage( + marketsAdvanced.otherMarkets.valueInBaseCurrency, + marketsAdvancedTotal + ); + marketsAdvanced[UNKNOWN_KEY].valueInPercentage = toPercentage( + marketsAdvanced[UNKNOWN_KEY].valueInBaseCurrency, + marketsAdvancedTotal + ); return { markets, marketsAdvanced }; } diff --git a/apps/client/src/app/pages/portfolio/allocations/allocations-page.component.ts b/apps/client/src/app/pages/portfolio/allocations/allocations-page.component.ts index 915c6833ab..a4f883c0ec 100644 --- a/apps/client/src/app/pages/portfolio/allocations/allocations-page.component.ts +++ b/apps/client/src/app/pages/portfolio/allocations/allocations-page.component.ts @@ -388,7 +388,7 @@ export class GfAllocationsPageComponent implements OnInit { [name: string]: { name: string; value: number }; } = {}; - let totalValueExcludingCashPositions = 0; + let totalValueExcludingCashAndCryptoPositions = 0; let totalValueInFunds = 0; for (const position of this.portfolioDetails.holdings) { @@ -415,11 +415,18 @@ export class GfAllocationsPageComponent implements OnInit { : (position.valueInBaseCurrency ?? 0) }; - if (!isCashPosition(position.assetProfile)) { + const isExcludedFromGeographicAnalysis = + isCashPosition(position.assetProfile) || + position.assetProfile.assetSubClass === AssetSubClass.CRYPTOCURRENCY; + + if (!isExcludedFromGeographicAnalysis) { // Prepare analysis data by continents, countries, holdings and sectors - // except for cash + // except for cash and cryptocurrencies, which cannot be assigned to a + // specific continent - totalValueExcludingCashPositions += this.extractValue(position); + totalValueExcludingCashAndCryptoPositions += this.extractValue( + position + ); if (position.assetProfile.countries.length > 0) { for (const country of position.assetProfile.countries) { @@ -541,11 +548,11 @@ export class GfAllocationsPageComponent implements OnInit { if (this.showValuesInPercentage()) { // The values are percentages of the whole portfolio, but the analysis - // data does not contain the cash positions + // data does not contain the cash and cryptocurrency positions for (const values of [this.continents, this.countries, this.sectors]) { convertValuesToPercentagesOfTotal({ values, - total: totalValueExcludingCashPositions + total: totalValueExcludingCashAndCryptoPositions }); } }