From 82b243f9ed92bb11cda285fd8c9fc6d45471014c Mon Sep 17 00:00:00 2001 From: Thomas Kaul <4159106+dtslvr@users.noreply.github.com> Date: Sun, 30 Aug 2026 19:33:10 +0200 Subject: [PATCH] Bugfix/handling of cash positions (#7660) * Fix handling of cash positions * Update changelog --- CHANGELOG.md | 5 + .../app/endpoints/public/public.service.ts | 20 +- .../src/app/portfolio/portfolio.controller.ts | 49 +---- apps/api/src/helper/portfolio.helper.ts | 49 +++++ .../allocations/allocations-page.component.ts | 203 ++++++++++-------- .../app/pages/public/public-page.component.ts | 53 +++-- libs/common/src/lib/helper.ts | 24 +++ .../public-portfolio-response.interface.ts | 1 - libs/ui/src/lib/services/data.service.ts | 6 - 9 files changed, 234 insertions(+), 176 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 269d81671..07dec9f69 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Upgraded `undici` from version `8.5.0` to `8.10.0` - Upgraded `uuid` from version `14.0.1` to `14.0.2` +### Fixed + +- Fixed the cash positions being included in the by continent, by country and by sector charts on the allocations page and the public page +- Fixed the allocations in percentage exceeding 100% in the restricted view + ## 3.64.0 - 2026-08-30 ### Added diff --git a/apps/api/src/app/endpoints/public/public.service.ts b/apps/api/src/app/endpoints/public/public.service.ts index d86f1f0d8..c32df6d82 100644 --- a/apps/api/src/app/endpoints/public/public.service.ts +++ b/apps/api/src/app/endpoints/public/public.service.ts @@ -6,18 +6,14 @@ import { ConfigurationService } from '@ghostfolio/api/services/configuration/con import { ExchangeRateDataService } from '@ghostfolio/api/services/exchange-rate-data/exchange-rate-data.service'; import { DEFAULT_CURRENCY } from '@ghostfolio/common/config'; import { SubscriptionType } from '@ghostfolio/common/enums'; -import { getSum } from '@ghostfolio/common/helper'; +import { getSum, isCashPosition } from '@ghostfolio/common/helper'; import { AccessSettings, PublicPortfolioResponse } from '@ghostfolio/common/interfaces'; import { HttpException, Injectable } from '@nestjs/common'; -import { - AssetClass, - AssetSubClass, - Type as ActivityType -} from '@prisma/client'; +import { Type as ActivityType } from '@prisma/client'; import { Big } from 'big.js'; import { StatusCodes, getReasonPhrase } from 'http-status-codes'; @@ -173,23 +169,19 @@ export class PublicService { assetProfile: { ...holding.assetProfile, assetClass: - hasDetails || - holding.assetProfile.assetClass === AssetClass.LIQUIDITY + hasDetails || isCashPosition(holding.assetProfile) ? holding.assetProfile.assetClass : undefined, assetClassLabel: - hasDetails || - holding.assetProfile.assetClass === AssetClass.LIQUIDITY + hasDetails || isCashPosition(holding.assetProfile) ? holding.assetProfile.assetClassLabel : undefined, assetSubClass: - hasDetails || - holding.assetProfile.assetSubClass === AssetSubClass.CASH + hasDetails || isCashPosition(holding.assetProfile) ? holding.assetProfile.assetSubClass : undefined, assetSubClassLabel: - hasDetails || - holding.assetProfile.assetSubClass === AssetSubClass.CASH + hasDetails || isCashPosition(holding.assetProfile) ? holding.assetProfile.assetSubClassLabel : undefined, holdings: holding.assetProfile.holdings?.map( diff --git a/apps/api/src/app/portfolio/portfolio.controller.ts b/apps/api/src/app/portfolio/portfolio.controller.ts index 02b847932..431a8da20 100644 --- a/apps/api/src/app/portfolio/portfolio.controller.ts +++ b/apps/api/src/app/portfolio/portfolio.controller.ts @@ -6,6 +6,7 @@ import { hasNotDefinedValuesInObject, nullifyValuesInObject } from '@ghostfolio/api/helper/object.helper'; +import { convertValuesToPercentages } from '@ghostfolio/api/helper/portfolio.helper'; import { PerformanceLoggingInterceptor } from '@ghostfolio/api/interceptors/performance-logging/performance-logging.interceptor'; import { RedactValuesInResponseInterceptor } from '@ghostfolio/api/interceptors/redact-values-in-response/redact-values-in-response.interceptor'; import { TransformDataSourceInRequestInterceptor } from '@ghostfolio/api/interceptors/transform-data-source-in-request/transform-data-source-in-request.interceptor'; @@ -15,6 +16,7 @@ import { ConfigurationService } from '@ghostfolio/api/services/configuration/con import { getIntervalFromDateRange } from '@ghostfolio/common/calculation-helper'; import { UNKNOWN_KEY } from '@ghostfolio/common/config'; import { SubscriptionType } from '@ghostfolio/common/enums'; +import { isCashPosition } from '@ghostfolio/common/helper'; import { PortfolioDetails, PortfolioDividendsResponse, @@ -44,7 +46,7 @@ import { Version } from '@nestjs/common'; import { REQUEST } from '@nestjs/core'; -import { AssetClass, AssetSubClass, DataSource } from '@prisma/client'; +import { DataSource } from '@prisma/client'; import { Big } from 'big.js'; import { StatusCodes, getReasonPhrase } from 'http-status-codes'; @@ -128,38 +130,7 @@ export class PortfolioController { !hasScope(impersonationScopes, scopes.portfolioReadValues) || isRestrictedView(this.request.user) ) { - const totalInvestment = holdings - .map(({ investment }) => { - return investment; - }) - .reduce((a, b) => a + b, 0); - - const totalValue = holdings - .filter(({ assetProfile }) => { - return ( - assetProfile.assetClass !== AssetClass.LIQUIDITY && - assetProfile.assetSubClass !== AssetSubClass.CASH - ); - }) - .map(({ valueInBaseCurrency }) => { - return valueInBaseCurrency; - }) - .reduce((a, b) => { - return a + b; - }, 0); - - for (const holding of holdings) { - holding.investment = holding.investment / totalInvestment; - holding.valueInPercentage = holding.valueInBaseCurrency / totalValue; - } - - for (const [name, { valueInBaseCurrency }] of Object.entries(accounts)) { - accounts[name].valueInPercentage = valueInBaseCurrency / totalValue; - } - - for (const [name, { valueInBaseCurrency }] of Object.entries(platforms)) { - platforms[name].valueInPercentage = valueInBaseCurrency / totalValue; - } + convertValuesToPercentages({ accounts, holdings, platforms }); } if ( @@ -208,23 +179,19 @@ export class PortfolioController { assetProfile: { ...portfolioPosition.assetProfile, assetClass: - hasDetails || - portfolioPosition.assetProfile.assetClass === AssetClass.LIQUIDITY + hasDetails || isCashPosition(portfolioPosition.assetProfile) ? portfolioPosition.assetProfile.assetClass : undefined, assetClassLabel: - hasDetails || - portfolioPosition.assetProfile.assetClass === AssetClass.LIQUIDITY + hasDetails || isCashPosition(portfolioPosition.assetProfile) ? portfolioPosition.assetProfile.assetClassLabel : undefined, assetSubClass: - hasDetails || - portfolioPosition.assetProfile.assetSubClass === AssetSubClass.CASH + hasDetails || isCashPosition(portfolioPosition.assetProfile) ? portfolioPosition.assetProfile.assetSubClass : undefined, assetSubClassLabel: - hasDetails || - portfolioPosition.assetProfile.assetSubClass === AssetSubClass.CASH + hasDetails || isCashPosition(portfolioPosition.assetProfile) ? portfolioPosition.assetProfile.assetSubClassLabel : undefined, ...(hasDetails diff --git a/apps/api/src/helper/portfolio.helper.ts b/apps/api/src/helper/portfolio.helper.ts index 6ebe48d3c..b8152414b 100644 --- a/apps/api/src/helper/portfolio.helper.ts +++ b/apps/api/src/helper/portfolio.helper.ts @@ -1,5 +1,54 @@ +import { PortfolioDetails } from '@ghostfolio/common/interfaces'; + import { Type as ActivityType } from '@prisma/client'; +export function convertValuesToPercentages({ + accounts, + holdings, + platforms +}: { + accounts: PortfolioDetails['accounts']; + holdings: PortfolioDetails['holdings']; + platforms: PortfolioDetails['platforms']; +}) { + const totalInvestment = holdings + .map(({ investment }) => { + return investment; + }) + .reduce((a, b) => { + return a + b; + }, 0); + + const totalValue = holdings + .map(({ valueInBaseCurrency }) => { + return valueInBaseCurrency; + }) + .reduce((a, b) => { + return a + b; + }, 0); + + for (const holding of holdings) { + holding.investment = totalInvestment + ? holding.investment / totalInvestment + : 0; + holding.valueInPercentage = totalValue + ? holding.valueInBaseCurrency / totalValue + : 0; + } + + for (const account of Object.values(accounts)) { + account.valueInPercentage = totalValue + ? account.valueInBaseCurrency / totalValue + : 0; + } + + for (const platform of Object.values(platforms)) { + platform.valueInPercentage = totalValue + ? platform.valueInBaseCurrency / totalValue + : 0; + } +} + export function getFactor(activityType: ActivityType) { let factor: number; 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 341509d0c..ceda1d4c0 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 @@ -8,8 +8,10 @@ import { UserService } from '@ghostfolio/client/services/user/user.service'; import { MAX_TOP_HOLDINGS, UNKNOWN_KEY } from '@ghostfolio/common/config'; import { canOpenHoldingDetail, + convertValuesToPercentagesOfTotal, getAssetProfileIdentifier, - getCountryName + getCountryName, + isCashPosition } from '@ghostfolio/common/helper'; import { HoldingWithParents, @@ -270,6 +272,20 @@ export class GfAllocationsPageComponent implements OnInit { return UNKNOWN_KEY; } + private extractValue({ + valueInBaseCurrency, + valueInPercentage + }: { + valueInBaseCurrency?: PortfolioPosition['valueInBaseCurrency']; + valueInPercentage?: PortfolioPosition['valueInPercentage']; + }) { + return ( + (isNumber(valueInBaseCurrency) + ? valueInBaseCurrency + : valueInPercentage) ?? 0 + ); + } + private fetchPortfolioDetails() { return this.dataService.fetchPortfolioDetails({ filters: this.userService.getFilters(), @@ -373,6 +389,8 @@ export class GfAllocationsPageComponent implements OnInit { }; } + let totalValueExcludingCashPositions = 0; + for (const position of this.portfolioDetails.holdings) { const assetProfileIdentifier = getAssetProfileIdentifier( position.assetProfile @@ -397,111 +415,104 @@ export class GfAllocationsPageComponent implements OnInit { : (position.valueInBaseCurrency ?? 0) }; - // Prepare analysis data by continents, countries, holdings and sectors - - if (position.assetProfile.countries.length > 0) { - for (const country of position.assetProfile.countries) { - const { code, continent, weight } = country; - const value = - (isNumber(position.valueInBaseCurrency) - ? position.valueInBaseCurrency - : position.valueInPercentage) ?? 0; + if (!isCashPosition(position.assetProfile)) { + // Prepare analysis data by continents, countries, holdings and sectors + // except for cash + + totalValueExcludingCashPositions += this.extractValue(position); + + if (position.assetProfile.countries.length > 0) { + for (const country of position.assetProfile.countries) { + const { code, continent, weight } = country; + const value = this.extractValue(position); + + const continentData = this.continents[continent]; + + if (continentData) { + continentData.value += weight * value; + } else { + this.continents[continent] = { + name: translate(continent), + value: weight * value + }; + } + + const countryData = this.countries[code]; + + if (countryData) { + countryData.value += weight * value; + } else { + this.countries[code] = { + name: getCountryName({ code }), + value: weight * value + }; + } + } + } else { + const value = this.extractValue(position); - const continentData = this.continents[continent]; + const continentData = this.continents[UNKNOWN_KEY]; if (continentData) { - continentData.value += weight * value; - } else { - this.continents[continent] = { - name: translate(continent), - value: weight * value - }; + continentData.value += value; } - const countryData = this.countries[code]; + const countryData = this.countries[UNKNOWN_KEY]; if (countryData) { - countryData.value += weight * value; - } else { - this.countries[code] = { - name: getCountryName({ code }), - value: weight * value - }; + countryData.value += value; } } - } else { - const value = - (isNumber(position.valueInBaseCurrency) - ? position.valueInBaseCurrency - : position.valueInPercentage) ?? 0; - - const continentData = this.continents[UNKNOWN_KEY]; - - if (continentData) { - continentData.value += value; - } - - const countryData = this.countries[UNKNOWN_KEY]; - if (countryData) { - countryData.value += value; - } - } - - if (position.assetProfile.holdings.length > 0) { - for (const { - allocationInPercentage, - name, - valueInBaseCurrency - } of position.assetProfile.holdings) { - const normalizedAssetName = this.normalizeAssetName(name); - const value = isNumber(valueInBaseCurrency) - ? valueInBaseCurrency - : allocationInPercentage * (position.valueInPercentage ?? 0); - - const holdingData = this.topHoldingsMap[normalizedAssetName]; - - if (holdingData) { - holdingData.value += value; - } else { - this.topHoldingsMap[normalizedAssetName] = { - name, - value - }; + if (position.assetProfile.holdings.length > 0) { + for (const { + allocationInPercentage, + name, + valueInBaseCurrency + } of position.assetProfile.holdings) { + const normalizedAssetName = this.normalizeAssetName(name); + const value = isNumber(valueInBaseCurrency) + ? valueInBaseCurrency + : allocationInPercentage * (position.valueInPercentage ?? 0); + + const holdingData = this.topHoldingsMap[normalizedAssetName]; + + if (holdingData) { + holdingData.value += value; + } else { + this.topHoldingsMap[normalizedAssetName] = { + name, + value + }; + } } } - } - if (position.assetProfile.sectors.length > 0) { - for (const sector of position.assetProfile.sectors) { - const { name, weight } = sector; - const value = - (isNumber(position.valueInBaseCurrency) - ? position.valueInBaseCurrency - : position.valueInPercentage) ?? 0; + if (position.assetProfile.sectors.length > 0) { + for (const sector of position.assetProfile.sectors) { + const { name, weight } = sector; + const value = this.extractValue(position); + + const sectorData = this.sectors[name]; + + if (sectorData) { + sectorData.value += weight * value; + } else { + this.sectors[name] = { + name: translate(name), + value: weight * value + }; + } + } + } else { + const value = this.extractValue(position); - const sectorData = this.sectors[name]; + const sectorData = this.sectors[UNKNOWN_KEY]; if (sectorData) { - sectorData.value += weight * value; - } else { - this.sectors[name] = { - name: translate(name), - value: weight * value - }; + sectorData.value += value; } } - } else { - const value = - (isNumber(position.valueInBaseCurrency) - ? position.valueInBaseCurrency - : position.valueInPercentage) ?? 0; - - const sectorData = this.sectors[UNKNOWN_KEY]; - - if (sectorData) { - sectorData.value += value; - } } if (this.holdings[assetProfileIdentifier].assetSubClass === 'ETF') { @@ -510,10 +521,7 @@ export class GfAllocationsPageComponent implements OnInit { const symbol = position.assetProfile.symbol; - const value = - (isNumber(position.valueInBaseCurrency) - ? position.valueInBaseCurrency - : position.valueInPercentage) ?? 0; + const value = this.extractValue(position); const symbolData = this.symbols[symbol]; @@ -533,6 +541,17 @@ 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 + for (const values of [this.continents, this.countries, this.sectors]) { + convertValuesToPercentagesOfTotal({ + values, + total: totalValueExcludingCashPositions + }); + } + } + this.markets = this.portfolioDetails.markets; if (this.portfolioDetails.marketsAdvanced) { diff --git a/apps/client/src/app/pages/public/public-page.component.ts b/apps/client/src/app/pages/public/public-page.component.ts index e76154ce0..46606753e 100644 --- a/apps/client/src/app/pages/public/public-page.component.ts +++ b/apps/client/src/app/pages/public/public-page.component.ts @@ -1,7 +1,9 @@ import { UNKNOWN_KEY } from '@ghostfolio/common/config'; import { + convertValuesToPercentagesOfTotal, getAssetProfileIdentifier, - getCountryName + getCountryName, + isCashPosition } from '@ghostfolio/common/helper'; import { InfoItem, @@ -34,9 +36,7 @@ import { MatButtonModule } from '@angular/material/button'; import { MatCardModule } from '@angular/material/card'; import { MatTableDataSource } from '@angular/material/table'; import { ActivatedRoute, Router } from '@angular/router'; -import { AssetClass } from '@prisma/client'; import { StatusCodes } from 'http-status-codes'; -import { isNumber } from 'lodash'; import { DeviceDetectorService } from 'ngx-device-detector'; import { EMPTY } from 'rxjs'; import { catchError } from 'rxjs/operators'; @@ -178,6 +178,8 @@ export class GfPublicPageComponent implements OnInit { } }; + let totalValueExcludingCashPositions = 0; + for (const position of this.publicPortfolioDetails.holdings) { const assetProfileIdentifier = getAssetProfileIdentifier( position.assetProfile @@ -191,39 +193,40 @@ export class GfPublicPageComponent implements OnInit { value: position.allocationInPercentage }; - if (position.assetProfile.assetClass !== AssetClass.LIQUIDITY) { - // Prepare analysis data by continents, countries, holdings and sectors except for liquidity + if (!isCashPosition(position.assetProfile)) { + // Prepare analysis data by continents, countries, holdings and sectors + // except for cash + + const value = position.valueInPercentage ?? 0; + + totalValueExcludingCashPositions += value; if (position.assetProfile.countries.length > 0) { for (const country of position.assetProfile.countries) { const { code, continent, weight } = country; if (this.continents[continent]?.value) { - this.continents[continent].value += - weight * (position.valueInBaseCurrency ?? 0); + this.continents[continent].value += weight * value; } else { this.continents[continent] = { name: translate(continent), - value: weight * (position.valueInBaseCurrency ?? 0) + value: weight * value }; } if (this.countries[code]?.value) { - this.countries[code].value += - weight * (position.valueInBaseCurrency ?? 0); + this.countries[code].value += weight * value; } else { this.countries[code] = { name: getCountryName({ code }), - value: weight * (position.valueInBaseCurrency ?? 0) + value: weight * value }; } } } else { - this.continents[UNKNOWN_KEY].value += - position.valueInBaseCurrency ?? 0; + this.continents[UNKNOWN_KEY].value += value; - this.countries[UNKNOWN_KEY].value += - position.valueInBaseCurrency ?? 0; + this.countries[UNKNOWN_KEY].value += value; } if (position.assetProfile.sectors.length > 0) { @@ -231,25 +234,22 @@ export class GfPublicPageComponent implements OnInit { const { name, weight } = sector; if (this.sectors[name]?.value) { - this.sectors[name].value += - weight * (position.valueInBaseCurrency ?? 0); + this.sectors[name].value += weight * value; } else { this.sectors[name] = { name: translate(name), - value: weight * (position.valueInBaseCurrency ?? 0) + value: weight * value }; } } } else { - this.sectors[UNKNOWN_KEY].value += position.valueInBaseCurrency ?? 0; + this.sectors[UNKNOWN_KEY].value += value; } } const symbol = position.assetProfile.symbol; - const value = isNumber(position.valueInBaseCurrency) - ? position.valueInBaseCurrency - : (position.valueInPercentage ?? 0); + const value = position.valueInPercentage ?? 0; const symbolData = this.symbols[symbol]; @@ -264,5 +264,14 @@ export class GfPublicPageComponent implements OnInit { }; } } + + // The values are percentages of the whole portfolio, but the analysis data + // does not contain the cash positions + for (const values of [this.continents, this.countries, this.sectors]) { + convertValuesToPercentagesOfTotal({ + values, + total: totalValueExcludingCashPositions + }); + } } } diff --git a/libs/common/src/lib/helper.ts b/libs/common/src/lib/helper.ts index 6accde31c..b6b097be9 100644 --- a/libs/common/src/lib/helper.ts +++ b/libs/common/src/lib/helper.ts @@ -232,6 +232,22 @@ export function capitalize(aString: string) { return aString.charAt(0).toUpperCase() + aString.slice(1).toLowerCase(); } +export function convertValuesToPercentagesOfTotal({ + total, + values +}: { + total: number; + values: { [key: string]: { value: number } }; +}) { + if (!total) { + return; + } + + for (const item of Object.values(values)) { + item.value = item.value / total; + } +} + export function downloadAsFile({ content, contentType = 'text/plain', @@ -553,6 +569,14 @@ export function isAccountExcluded(account?: { tags?: { id: string }[] }) { ); } +export function isCashPosition({ + assetSubClass +}: { + assetSubClass?: AssetSubClass; +} = {}) { + return assetSubClass === AssetSubClass.CASH; +} + export function isCurrency(aCurrency: string) { if (!aCurrency) { return false; diff --git a/libs/common/src/lib/interfaces/responses/public-portfolio-response.interface.ts b/libs/common/src/lib/interfaces/responses/public-portfolio-response.interface.ts index ce4bc2817..616dbac99 100644 --- a/libs/common/src/lib/interfaces/responses/public-portfolio-response.interface.ts +++ b/libs/common/src/lib/interfaces/responses/public-portfolio-response.interface.ts @@ -17,7 +17,6 @@ export interface PublicPortfolioResponse extends PublicPortfolioResponseV1 { | 'dateOfFirstActivity' | 'markets' | 'netPerformancePercentWithCurrencyEffect' - | 'valueInBaseCurrency' | 'valueInPercentage' >[]; latestActivities: (Pick< diff --git a/libs/ui/src/lib/services/data.service.ts b/libs/ui/src/lib/services/data.service.ts index fb8a5aea6..5f86b7a32 100644 --- a/libs/ui/src/lib/services/data.service.ts +++ b/libs/ui/src/lib/services/data.service.ts @@ -788,12 +788,6 @@ export class DataService { holding.assetProfile.assetSubClassLabel = translate( holding.assetProfile.assetSubClass ); - - holding.valueInBaseCurrency = isNumber( - holding.valueInBaseCurrency - ) - ? holding.valueInBaseCurrency - : holding.valueInPercentage; } }