From eb75be8535f2b40e6f2a2ab57031840f866bce41 Mon Sep 17 00:00:00 2001 From: Thomas Kaul <4159106+dtslvr@users.noreply.github.com> Date: Sat, 9 Mar 2024 19:56:26 +0100 Subject: [PATCH] Optimize details endpoint (#3123) * Make summary optional * Introduce dedicated holdings endpoint * Update changelog --- CHANGELOG.md | 2 + .../src/app/portfolio/portfolio.controller.ts | 62 +++++++++++------ .../src/app/portfolio/portfolio.service.ts | 69 ++++++++++++------- .../redact-values-in-response.interceptor.ts | 2 - .../account-detail-dialog.component.ts | 8 +-- .../portfolio-summary.component.html | 2 +- .../allocations/allocations-page.component.ts | 1 - .../allocations/allocations-page.html | 9 +-- .../holdings/holdings-page.component.ts | 30 ++------ apps/client/src/app/services/data.service.ts | 41 +++++++++++ libs/common/src/lib/helper.ts | 6 +- libs/common/src/lib/interfaces/index.ts | 2 + .../interfaces/portfolio-details.interface.ts | 5 +- .../interfaces/portfolio-summary.interface.ts | 4 +- .../portfolio-holdings-response.interface.ts | 5 ++ 15 files changed, 159 insertions(+), 89 deletions(-) create mode 100644 libs/common/src/lib/interfaces/responses/portfolio-holdings-response.interface.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 7c1e7a219..9918a4420 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed +- Optimized the calculation of the accounts table +- Optimized the calculation of the portfolio holdings - Integrated dividend into the transaction point concept in the portfolio service - Removed the environment variable `WEB_AUTH_RP_ID` diff --git a/apps/api/src/app/portfolio/portfolio.controller.ts b/apps/api/src/app/portfolio/portfolio.controller.ts index 63e26e512..748850204 100644 --- a/apps/api/src/app/portfolio/portfolio.controller.ts +++ b/apps/api/src/app/portfolio/portfolio.controller.ts @@ -20,6 +20,7 @@ import { import { PortfolioDetails, PortfolioDividends, + PortfolioHoldingsResponse, PortfolioInvestments, PortfolioPerformanceResponse, PortfolioPublicDetails, @@ -95,21 +96,15 @@ export class PortfolioController { filterByTags }); - const { - accounts, - filteredValueInBaseCurrency, - filteredValueInPercentage, - hasErrors, - holdings, - platforms, - summary, - totalValueInBaseCurrency - } = await this.portfolioService.getDetails({ - dateRange, - filters, - impersonationId, - userId: this.request.user.id - }); + const { accounts, hasErrors, holdings, platforms, summary } = + await this.portfolioService.getDetails({ + dateRange, + filters, + impersonationId, + userId: this.request.user.id, + withLiabilities: true, + withSummary: true + }); if (hasErrors || hasNotDefinedValuesInObject(holdings)) { hasError = true; @@ -164,19 +159,21 @@ export class PortfolioController { 'currentGrossPerformanceWithCurrencyEffect', 'currentNetPerformance', 'currentNetPerformanceWithCurrencyEffect', + 'currentNetWorth', 'currentValue', 'dividendInBaseCurrency', 'emergencyFund', 'excludedAccountsAndActivities', 'fees', + 'filteredValueInBaseCurrency', 'fireWealth', 'interest', 'items', 'liabilities', - 'netWorth', 'totalBuy', 'totalInvestment', - 'totalSell' + 'totalSell', + 'totalValueInBaseCurrency' ]); } @@ -203,12 +200,9 @@ export class PortfolioController { return { accounts, - filteredValueInBaseCurrency, - filteredValueInPercentage, hasError, holdings, platforms, - totalValueInBaseCurrency, summary: portfolioSummary }; } @@ -279,6 +273,33 @@ export class PortfolioController { return { dividends }; } + @Get('holdings') + @UseGuards(AuthGuard('jwt'), HasPermissionGuard) + @UseInterceptors(RedactValuesInResponseInterceptor) + @UseInterceptors(TransformDataSourceInResponseInterceptor) + public async getHoldings( + @Headers(HEADER_KEY_IMPERSONATION.toLowerCase()) impersonationId: string, + @Query('accounts') filterByAccounts?: string, + @Query('assetClasses') filterByAssetClasses?: string, + @Query('query') filterBySearchQuery?: string, + @Query('tags') filterByTags?: string + ): Promise { + const filters = this.apiService.buildFiltersFromQueryParams({ + filterByAccounts, + filterByAssetClasses, + filterBySearchQuery, + filterByTags + }); + + const { holdings } = await this.portfolioService.getDetails({ + filters, + impersonationId, + userId: this.request.user.id + }); + + return { holdings: Object.values(holdings) }; + } + @Get('investments') @UseGuards(AuthGuard('jwt'), HasPermissionGuard) public async getInvestments( @@ -502,7 +523,6 @@ export class PortfolioController { } const { holdings } = await this.portfolioService.getDetails({ - dateRange: 'max', filters: [{ id: 'EQUITY', type: 'ASSET_CLASS' }], impersonationId: access.userId, userId: user.id diff --git a/apps/api/src/app/portfolio/portfolio.service.ts b/apps/api/src/app/portfolio/portfolio.service.ts index f03e6ec84..9880abcc4 100644 --- a/apps/api/src/app/portfolio/portfolio.service.ts +++ b/apps/api/src/app/portfolio/portfolio.service.ts @@ -24,7 +24,12 @@ import { MAX_CHART_ITEMS, UNKNOWN_KEY } from '@ghostfolio/common/config'; -import { DATE_FORMAT, getSum, parseDate } from '@ghostfolio/common/helper'; +import { + DATE_FORMAT, + getAllActivityTypes, + getSum, + parseDate +} from '@ghostfolio/common/helper'; import { Accounts, EnhancedSymbolProfile, @@ -141,7 +146,8 @@ export class PortfolioService { filters, withExcludedAccounts, impersonationId: userId, - userId: this.request.user.id + userId: this.request.user.id, + withLiabilities: true }) ]); @@ -332,13 +338,17 @@ export class PortfolioService { filters, impersonationId, userId, - withExcludedAccounts = false + withExcludedAccounts = false, + withLiabilities = false, + withSummary = false }: { dateRange?: DateRange; filters?: Filter[]; impersonationId: string; userId: string; withExcludedAccounts?: boolean; + withLiabilities?: boolean; + withSummary?: boolean; }): Promise { userId = await this.getUserId(impersonationId, userId); const user = await this.userService.user({ id: userId }); @@ -352,7 +362,12 @@ export class PortfolioService { await this.getTransactionPoints({ filters, userId, - withExcludedAccounts + withExcludedAccounts, + types: withLiabilities + ? undefined + : getAllActivityTypes().filter((activityType) => { + return activityType !== 'LIABILITY'; + }) }); const portfolioCalculator = new PortfolioCalculator({ @@ -625,29 +640,29 @@ export class PortfolioService { }; } - const summary = await this.getSummary({ - holdings, - impersonationId, - userCurrency, - userId, - balanceInBaseCurrency: cashDetails.balanceInBaseCurrency, - emergencyFundPositionsValueInBaseCurrency: - this.getEmergencyFundPositionsValueInBaseCurrency({ - holdings - }) - }); + let summary: PortfolioSummary; + + if (withSummary) { + summary = await this.getSummary({ + filteredValueInBaseCurrency, + holdings, + impersonationId, + userCurrency, + userId, + balanceInBaseCurrency: cashDetails.balanceInBaseCurrency, + emergencyFundPositionsValueInBaseCurrency: + this.getEmergencyFundPositionsValueInBaseCurrency({ + holdings + }) + }); + } return { accounts, holdings, platforms, summary, - filteredValueInBaseCurrency: filteredValueInBaseCurrency.toNumber(), - filteredValueInPercentage: summary.netWorth - ? filteredValueInBaseCurrency.div(summary.netWorth).toNumber() - : 0, - hasErrors: currentPositions.hasErrors, - totalValueInBaseCurrency: summary.netWorth + hasErrors: currentPositions.hasErrors }; } @@ -1705,6 +1720,7 @@ export class PortfolioService { private async getSummary({ balanceInBaseCurrency, emergencyFundPositionsValueInBaseCurrency, + filteredValueInBaseCurrency, holdings, impersonationId, userCurrency, @@ -1712,6 +1728,7 @@ export class PortfolioService { }: { balanceInBaseCurrency: number; emergencyFundPositionsValueInBaseCurrency: number; + filteredValueInBaseCurrency: Big; holdings: PortfolioDetails['holdings']; impersonationId: string; userCurrency: string; @@ -1893,7 +1910,6 @@ export class PortfolioService { interest, items, liabilities, - netWorth, totalBuy, totalSell, committedFunds: committedFunds.toNumber(), @@ -1905,12 +1921,17 @@ export class PortfolioService { .toNumber(), total: emergencyFund.toNumber() }, + filteredValueInBaseCurrency: filteredValueInBaseCurrency.toNumber(), + filteredValueInPercentage: netWorth + ? filteredValueInBaseCurrency.div(netWorth).toNumber() + : undefined, fireWealth: new Big(performanceInformation.performance.currentValue) .minus(emergencyFundPositionsValueInBaseCurrency) .toNumber(), ordersCount: activities.filter(({ type }) => { return type === 'BUY' || type === 'SELL'; - }).length + }).length, + totalValueInBaseCurrency: netWorth }; } @@ -1943,7 +1964,7 @@ export class PortfolioService { private async getTransactionPoints({ filters, includeDrafts = false, - types = ['BUY', 'DIVIDEND', 'ITEM', 'LIABILITY', 'SELL'], + types = getAllActivityTypes(), userId, withExcludedAccounts = false }: { diff --git a/apps/api/src/interceptors/redact-values-in-response.interceptor.ts b/apps/api/src/interceptors/redact-values-in-response.interceptor.ts index b1889cf9d..78ae918d2 100644 --- a/apps/api/src/interceptors/redact-values-in-response.interceptor.ts +++ b/apps/api/src/interceptors/redact-values-in-response.interceptor.ts @@ -49,7 +49,6 @@ export class RedactValuesInResponseInterceptor 'dividendInBaseCurrency', 'fee', 'feeInBaseCurrency', - 'filteredValueInBaseCurrency', 'grossPerformance', 'grossPerformanceWithCurrencyEffect', 'investment', @@ -58,7 +57,6 @@ export class RedactValuesInResponseInterceptor 'quantity', 'symbolMapping', 'totalBalanceInBaseCurrency', - 'totalValueInBaseCurrency', 'unitPrice', 'value', 'valueInBaseCurrency' diff --git a/apps/client/src/app/components/account-detail-dialog/account-detail-dialog.component.ts b/apps/client/src/app/components/account-detail-dialog/account-detail-dialog.component.ts index 2cd48a561..760f8081b 100644 --- a/apps/client/src/app/components/account-detail-dialog/account-detail-dialog.component.ts +++ b/apps/client/src/app/components/account-detail-dialog/account-detail-dialog.component.ts @@ -115,7 +115,7 @@ export class AccountDetailDialog implements OnDestroy, OnInit { ); this.dataService - .fetchPortfolioDetails({ + .fetchPortfolioHoldings({ filters: [ { type: 'ACCOUNT', @@ -125,11 +125,7 @@ export class AccountDetailDialog implements OnDestroy, OnInit { }) .pipe(takeUntil(this.unsubscribeSubject)) .subscribe(({ holdings }) => { - this.holdings = []; - - for (const [symbol, holding] of Object.entries(holdings)) { - this.holdings.push(holding); - } + this.holdings = holdings; this.changeDetectorRef.markForCheck(); }); diff --git a/apps/client/src/app/components/portfolio-summary/portfolio-summary.component.html b/apps/client/src/app/components/portfolio-summary/portfolio-summary.component.html index 096271926..347767011 100644 --- a/apps/client/src/app/components/portfolio-summary/portfolio-summary.component.html +++ b/apps/client/src/app/components/portfolio-summary/portfolio-summary.component.html @@ -282,7 +282,7 @@ [isCurrency]="true" [locale]="locale" [unit]="baseCurrency" - [value]="isLoading ? undefined : summary?.netWorth" + [value]="isLoading ? undefined : summary?.totalValueInBaseCurrency" /> 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 330ae4227..67ad82316 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 @@ -281,7 +281,6 @@ export class AllocationsPageComponent implements OnDestroy, OnInit { this.platforms = {}; this.portfolioDetails = { accounts: {}, - filteredValueInPercentage: 0, holdings: {}, platforms: {}, summary: undefined diff --git a/apps/client/src/app/pages/portfolio/allocations/allocations-page.html b/apps/client/src/app/pages/portfolio/allocations/allocations-page.html index 312061775..04bf96f39 100644 --- a/apps/client/src/app/pages/portfolio/allocations/allocations-page.html +++ b/apps/client/src/app/pages/portfolio/allocations/allocations-page.html @@ -18,7 +18,7 @@ [value]=" isLoading ? undefined - : portfolioDetails?.filteredValueInPercentage + : portfolioDetails?.summary?.filteredValueInPercentage " /> @@ -26,10 +26,11 @@ diff --git a/apps/client/src/app/pages/portfolio/holdings/holdings-page.component.ts b/apps/client/src/app/pages/portfolio/holdings/holdings-page.component.ts index 6c4a058b7..6635393f5 100644 --- a/apps/client/src/app/pages/portfolio/holdings/holdings-page.component.ts +++ b/apps/client/src/app/pages/portfolio/holdings/holdings-page.component.ts @@ -3,11 +3,7 @@ import { PositionDetailDialog } from '@ghostfolio/client/components/position/pos import { DataService } from '@ghostfolio/client/services/data.service'; import { ImpersonationStorageService } from '@ghostfolio/client/services/impersonation-storage.service'; import { UserService } from '@ghostfolio/client/services/user/user.service'; -import { - PortfolioDetails, - PortfolioPosition, - User -} from '@ghostfolio/common/interfaces'; +import { PortfolioPosition, User } from '@ghostfolio/common/interfaces'; import { hasPermission, permissions } from '@ghostfolio/common/permissions'; import { ChangeDetectorRef, Component, OnDestroy, OnInit } from '@angular/core'; @@ -28,8 +24,6 @@ export class HoldingsPageComponent implements OnDestroy, OnInit { public hasImpersonationId: boolean; public hasPermissionToCreateOrder: boolean; public holdings: PortfolioPosition[]; - public isLoading = false; - public portfolioDetails: PortfolioDetails; public user: User; private unsubscribeSubject = new Subject(); @@ -83,12 +77,10 @@ export class HoldingsPageComponent implements OnDestroy, OnInit { this.holdings = undefined; - this.fetchPortfolioDetails() + this.fetchHoldings() .pipe(takeUntil(this.unsubscribeSubject)) - .subscribe((portfolioDetails) => { - this.portfolioDetails = portfolioDetails; - - this.initialize(); + .subscribe(({ holdings }) => { + this.holdings = holdings; this.changeDetectorRef.markForCheck(); }); @@ -103,22 +95,12 @@ export class HoldingsPageComponent implements OnDestroy, OnInit { this.unsubscribeSubject.complete(); } - private fetchPortfolioDetails() { - return this.dataService.fetchPortfolioDetails({ + private fetchHoldings() { + return this.dataService.fetchPortfolioHoldings({ filters: this.userService.getFilters() }); } - private initialize() { - this.holdings = []; - - for (const [symbol, holding] of Object.entries( - this.portfolioDetails.holdings - )) { - this.holdings.push(holding); - } - } - private openPositionDialog({ dataSource, symbol diff --git a/apps/client/src/app/services/data.service.ts b/apps/client/src/app/services/data.service.ts index 6555a964d..7741fd601 100644 --- a/apps/client/src/app/services/data.service.ts +++ b/apps/client/src/app/services/data.service.ts @@ -27,6 +27,7 @@ import { OAuthResponse, PortfolioDetails, PortfolioDividends, + PortfolioHoldingsResponse, PortfolioInvestments, PortfolioPerformanceResponse, PortfolioPublicDetails, @@ -434,6 +435,46 @@ export class DataService { ); } + public fetchPortfolioHoldings({ + filters + }: { + filters?: Filter[]; + } = {}) { + return this.http + .get('/api/v1/portfolio/holdings', { + params: this.buildFiltersAsQueryParams({ filters }) + }) + .pipe( + map((response) => { + if (response.holdings) { + for (const symbol of Object.keys(response.holdings)) { + response.holdings[symbol].assetClassLabel = translate( + response.holdings[symbol].assetClass + ); + + response.holdings[symbol].assetSubClassLabel = translate( + response.holdings[symbol].assetSubClass + ); + + response.holdings[symbol].dateOfFirstActivity = response.holdings[ + symbol + ].dateOfFirstActivity + ? parseISO(response.holdings[symbol].dateOfFirstActivity) + : undefined; + + response.holdings[symbol].value = isNumber( + response.holdings[symbol].value + ) + ? response.holdings[symbol].value + : response.holdings[symbol].valueInPercentage; + } + } + + return response; + }) + ); + } + public fetchPortfolioPerformance({ filters, range, diff --git a/libs/common/src/lib/helper.ts b/libs/common/src/lib/helper.ts index e55800628..c52ec9ca5 100644 --- a/libs/common/src/lib/helper.ts +++ b/libs/common/src/lib/helper.ts @@ -1,6 +1,6 @@ import * as currencies from '@dinero.js/currencies'; import { NumberParser } from '@internationalized/number'; -import { DataSource, MarketData } from '@prisma/client'; +import { DataSource, MarketData, Type as ActivityType } from '@prisma/client'; import Big from 'big.js'; import { getDate, @@ -138,6 +138,10 @@ export function extractNumberFromString({ } } +export function getAllActivityTypes(): ActivityType[] { + return ['BUY', 'DIVIDEND', 'FEE', 'ITEM', 'LIABILITY', 'SELL']; +} + export function getAssetProfileIdentifier({ dataSource, symbol }: UniqueAsset) { return `${dataSource}-${symbol}`; } diff --git a/libs/common/src/lib/interfaces/index.ts b/libs/common/src/lib/interfaces/index.ts index 7d77826d0..dba1ac79a 100644 --- a/libs/common/src/lib/interfaces/index.ts +++ b/libs/common/src/lib/interfaces/index.ts @@ -40,6 +40,7 @@ import type { BenchmarkResponse } from './responses/benchmark-response.interface import type { ResponseError } from './responses/errors.interface'; import type { ImportResponse } from './responses/import-response.interface'; import type { OAuthResponse } from './responses/oauth-response.interface'; +import type { PortfolioHoldingsResponse } from './responses/portfolio-holdings-response.interface'; import type { PortfolioPerformanceResponse } from './responses/portfolio-performance-response.interface'; import type { ScraperConfiguration } from './scraper-configuration.interface'; import type { Statistics } from './statistics.interface'; @@ -81,6 +82,7 @@ export { PortfolioChart, PortfolioDetails, PortfolioDividends, + PortfolioHoldingsResponse, PortfolioInvestments, PortfolioItem, PortfolioOverview, diff --git a/libs/common/src/lib/interfaces/portfolio-details.interface.ts b/libs/common/src/lib/interfaces/portfolio-details.interface.ts index 565c17c7d..611ed8056 100644 --- a/libs/common/src/lib/interfaces/portfolio-details.interface.ts +++ b/libs/common/src/lib/interfaces/portfolio-details.interface.ts @@ -13,8 +13,6 @@ export interface PortfolioDetails { valueInPercentage?: number; }; }; - filteredValueInBaseCurrency?: number; - filteredValueInPercentage: number; holdings: { [symbol: string]: PortfolioPosition }; platforms: { [id: string]: { @@ -25,6 +23,5 @@ export interface PortfolioDetails { valueInPercentage?: number; }; }; - summary: PortfolioSummary; - totalValueInBaseCurrency?: number; + summary?: PortfolioSummary; } diff --git a/libs/common/src/lib/interfaces/portfolio-summary.interface.ts b/libs/common/src/lib/interfaces/portfolio-summary.interface.ts index aaf80c0cd..de04dc24c 100644 --- a/libs/common/src/lib/interfaces/portfolio-summary.interface.ts +++ b/libs/common/src/lib/interfaces/portfolio-summary.interface.ts @@ -13,13 +13,15 @@ export interface PortfolioSummary extends PortfolioPerformance { }; excludedAccountsAndActivities: number; fees: number; + filteredValueInBaseCurrency?: number; + filteredValueInPercentage?: number; fireWealth: number; firstOrderDate: Date; interest: number; items: number; liabilities: number; - netWorth: number; ordersCount: number; totalBuy: number; totalSell: number; + totalValueInBaseCurrency?: number; } diff --git a/libs/common/src/lib/interfaces/responses/portfolio-holdings-response.interface.ts b/libs/common/src/lib/interfaces/responses/portfolio-holdings-response.interface.ts new file mode 100644 index 000000000..d2cf38f55 --- /dev/null +++ b/libs/common/src/lib/interfaces/responses/portfolio-holdings-response.interface.ts @@ -0,0 +1,5 @@ +import { PortfolioPosition } from '@ghostfolio/common/interfaces'; + +export interface PortfolioHoldingsResponse { + holdings: PortfolioPosition[]; +}