From f7c0a1b9fd7c67218dc0f7409870acb77789d731 Mon Sep 17 00:00:00 2001 From: Thomas Kaul <4159106+dtslvr@users.noreply.github.com> Date: Mon, 7 Sep 2026 20:43:49 +0200 Subject: [PATCH] Task/include closed holdings by default in holdings endpoint (#7843) * Return active and closed holdings in holdings endpoint * Update changelog --- CHANGELOG.md | 1 + .../app/portfolio/portfolio.service.spec.ts | 136 +++++++++++++++++- .../src/app/portfolio/portfolio.service.ts | 37 +++-- .../account-detail-dialog.component.ts | 8 +- .../home-holdings/home-holdings.component.ts | 4 +- ...reate-or-update-access-dialog.component.ts | 4 +- ...ate-or-update-activity-dialog.component.ts | 4 +- .../import-activities-dialog.component.ts | 4 + .../analysis/analysis-page.component.ts | 5 +- .../src/lib/assistant/assistant.component.ts | 4 +- 10 files changed, 180 insertions(+), 27 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 46214cfba..dd97e5e31 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed - Improved the loading state of the activity count in the portfolio summary +- Changed the holdings endpoint to return active and closed holdings by default and reuse a single snapshot for both types - Upgraded `zod` from version `4.4.3` to `4.5.4` ## 3.68.0 - 2026-09-06 diff --git a/apps/api/src/app/portfolio/portfolio.service.spec.ts b/apps/api/src/app/portfolio/portfolio.service.spec.ts index aed14e090..0882ff7e6 100644 --- a/apps/api/src/app/portfolio/portfolio.service.spec.ts +++ b/apps/api/src/app/portfolio/portfolio.service.spec.ts @@ -355,8 +355,13 @@ describe('PortfolioService', () => { describe('getDetails', () => { const setUpCashOnlyPortfolio = ({ baseCurrency = 'CHF', - emergencyFund - }: { baseCurrency?: string; emergencyFund?: number } = {}) => { + emergencyFund, + quantity = 2000 + }: { + baseCurrency?: string; + emergencyFund?: number; + quantity?: number; + } = {}) => { const cashAccount: AccountWithBalance = { balance: 2000, comment: null, @@ -421,7 +426,7 @@ describe('PortfolioService', () => { netPerformancePercentage: new Big(0), netPerformancePercentageWithCurrencyEffectMap: {}, netPerformanceWithCurrencyEffectMap: {}, - quantity: new Big(2000), + quantity: new Big(quantity), symbol: 'USD', tags: [], timeWeightedInvestment: new Big(0), @@ -493,6 +498,131 @@ describe('PortfolioService', () => { expect(holdings[0].assetProfile.symbol).toBe('USD'); expect(holdings[0].valueInBaseCurrency).toBe(1000); }); + + it('should include closed holdings when all holdings are requested', async () => { + setUpCashOnlyPortfolio({ quantity: 0 }); + + const { holdings } = await portfolioService.getDetails({ + filters: [], + includeAllHoldings: true, + userId: userDummyData.id + }); + + expect(holdings).toHaveLength(1); + expect(holdings[0].quantity).toBe(0); + }); + + it.each([ + { holdingType: 'ACTIVE', quantity: 2000 }, + { holdingType: 'CLOSED', quantity: 0 } + ])( + 'should return $holdingType holdings when the holding type is specified', + async ({ holdingType, quantity }) => { + setUpCashOnlyPortfolio({ quantity }); + + const { holdings } = await portfolioService.getDetails({ + filters: [{ id: holdingType, type: 'HOLDING_TYPE' }], + userId: userDummyData.id + }); + + expect(holdings).toHaveLength(1); + expect(holdings[0].quantity).toBe(quantity); + } + ); + + it('should remove the holding type only from the snapshot filters', async () => { + setUpCashOnlyPortfolio({ quantity: 0 }); + + await portfolioService.getDetails({ + filters: [ + { id: AssetClass.EQUITY, type: 'ASSET_CLASS' }, + { id: 'CLOSED', type: 'HOLDING_TYPE' } + ], + userId: userDummyData.id + }); + + expect(portfolioCalculatorFactory.createCalculator).toHaveBeenCalledWith( + expect.objectContaining({ + filters: [{ id: AssetClass.EQUITY, type: 'ASSET_CLASS' }] + }) + ); + expect( + activitiesService.getActivitiesForPortfolioCalculator + ).toHaveBeenCalledWith({ + filters: [{ id: AssetClass.EQUITY, type: 'ASSET_CLASS' }], + userCurrency: 'CHF', + userId: userDummyData.id + }); + }); + }); + + describe('getHoldings', () => { + const activeHolding = { + assetProfile: { + isin: 'US0378331005', + name: 'Apple', + symbol: 'AAPL' + }, + quantity: 1 + }; + + const closedHolding = { + assetProfile: { + isin: 'US5949181045', + name: 'Microsoft', + symbol: 'MSFT' + }, + quantity: 0 + }; + + beforeEach(() => { + jest.spyOn(portfolioService, 'getDetails').mockResolvedValue({ + holdings: [activeHolding, closedHolding] + } as unknown as Awaited>); + }); + + it('should request all holdings when the holding type is not specified', async () => { + const holdings = await portfolioService.getHoldings({ + dateRange: 'max', + userId: userDummyData.id + }); + + expect(holdings).toEqual([activeHolding, closedHolding]); + expect(portfolioService.getDetails).toHaveBeenCalledWith({ + dateRange: 'max', + filters: undefined, + includeAllHoldings: true, + userId: userDummyData.id + }); + }); + + it('should find a closed holding when the holding type is not specified', async () => { + const holdings = await portfolioService.getHoldings({ + dateRange: 'max', + filters: [{ id: 'Microsoft', type: 'SEARCH_QUERY' }], + userId: userDummyData.id + }); + + expect(holdings).toEqual([closedHolding]); + }); + + it.each(['ACTIVE', 'CLOSED'])( + 'should not request all holdings when the holding type is %s', + async (holdingType) => { + await portfolioService.getHoldings({ + dateRange: 'max', + filters: [{ id: holdingType, type: 'HOLDING_TYPE' }], + userId: userDummyData.id + }); + + expect(portfolioService.getDetails).toHaveBeenCalledWith({ + dateRange: 'max', + filters: [{ id: holdingType, type: 'HOLDING_TYPE' }], + includeAllHoldings: false, + userId: userDummyData.id + }); + } + ); }); describe('getHolding', () => { diff --git a/apps/api/src/app/portfolio/portfolio.service.ts b/apps/api/src/app/portfolio/portfolio.service.ts index 19c2dd794..60dab5032 100644 --- a/apps/api/src/app/portfolio/portfolio.service.ts +++ b/apps/api/src/app/portfolio/portfolio.service.ts @@ -477,12 +477,12 @@ export class PortfolioService { filters?: Filter[]; userId: string; }) { - const { SEARCH_QUERY: [filterBySearchQuery] = [] } = groupBy( - filters, - ({ type }) => { - return type; - } - ); + const { + HOLDING_TYPE: [filterByHoldingType] = [], + SEARCH_QUERY: [filterBySearchQuery] = [] + } = groupBy(filters, ({ type }) => { + return type; + }); const filtersWithoutSearchQueryFilter = filters?.filter(({ type }) => { return type !== 'SEARCH_QUERY'; @@ -491,7 +491,8 @@ export class PortfolioService { let { holdings } = await this.getDetails({ dateRange, userId, - filters: filtersWithoutSearchQueryFilter + filters: filtersWithoutSearchQueryFilter, + includeAllHoldings: !filterByHoldingType }); if (filterBySearchQuery) { @@ -592,6 +593,7 @@ export class PortfolioService { public async getDetails({ dateRange = DEFAULT_DATE_RANGE, filters, + includeAllHoldings = false, user: userFromCaller, userId, withExcludedAccounts = false, @@ -600,6 +602,7 @@ export class PortfolioService { }: { dateRange?: DateRange; filters?: Filter[]; + includeAllHoldings?: boolean; user?: UserWithSettings; userId: string; withExcludedAccounts?: boolean; @@ -614,19 +617,23 @@ export class PortfolioService { (user.settings?.settings as UserSettings)?.emergencyFund ?? 0 ); + const portfolioSnapshotFilters = filters?.filter(({ type }) => { + return type !== 'HOLDING_TYPE'; + }); + const { activities } = await this.activitiesService.getActivitiesForPortfolioCalculator({ - filters, userCurrency, - userId + userId, + filters: portfolioSnapshotFilters }); const portfolioCalculator = this.calculatorFactory.createCalculator({ activities, - filters, userId, calculationType: this.getUserPerformanceCalculationType(user), - currency: userCurrency + currency: userCurrency, + filters: portfolioSnapshotFilters }); const { createdAt, currentValueInBaseCurrency, hasErrors, positions } = @@ -706,13 +713,13 @@ export class PortfolioService { tags, valueInBaseCurrency } of positions) { - if (isFilteredByClosedHoldings === true) { - if (!quantity.eq(0)) { + if (!includeAllHoldings) { + if (isFilteredByClosedHoldings && !quantity.eq(0)) { // Ignore positions with a quantity continue; } - } else { - if (quantity.eq(0)) { + + if (!isFilteredByClosedHoldings && quantity.eq(0)) { // Ignore positions without any quantity continue; } 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 518458e8a..75186b174 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 @@ -393,8 +393,12 @@ export class GfAccountDetailDialogComponent implements OnInit { .fetchPortfolioHoldings({ filters: [ { - type: 'ACCOUNT', - id: this.data.accountId + id: this.data.accountId, + type: 'ACCOUNT' + }, + { + id: 'ACTIVE', + type: 'HOLDING_TYPE' } ] }) diff --git a/apps/client/src/app/components/home-holdings/home-holdings.component.ts b/apps/client/src/app/components/home-holdings/home-holdings.component.ts index 7f02cfb64..f079e1c9c 100644 --- a/apps/client/src/app/components/home-holdings/home-holdings.component.ts +++ b/apps/client/src/app/components/home-holdings/home-holdings.component.ts @@ -153,9 +153,7 @@ export class GfHomeHoldingsComponent implements OnInit { private fetchHoldings() { const filters = this.userService.getFilters(); - if (this.holdingType === 'CLOSED') { - filters.push({ id: 'CLOSED', type: 'HOLDING_TYPE' }); - } + filters.push({ id: this.holdingType, type: 'HOLDING_TYPE' }); return this.dataService.fetchPortfolioHoldings({ filters, diff --git a/apps/client/src/app/components/user-account-access/create-or-update-access-dialog/create-or-update-access-dialog.component.ts b/apps/client/src/app/components/user-account-access/create-or-update-access-dialog/create-or-update-access-dialog.component.ts index 573d4e5bb..63e7872c1 100644 --- a/apps/client/src/app/components/user-account-access/create-or-update-access-dialog/create-or-update-access-dialog.component.ts +++ b/apps/client/src/app/components/user-account-access/create-or-update-access-dialog/create-or-update-access-dialog.component.ts @@ -367,7 +367,9 @@ export class GfCreateOrUpdateAccessDialogComponent implements OnInit { private loadHoldings() { this.dataService - .fetchPortfolioHoldings() + .fetchPortfolioHoldings({ + filters: [{ id: 'ACTIVE', type: 'HOLDING_TYPE' }] + }) .pipe(takeUntilDestroyed(this.destroyRef)) .subscribe(({ holdings }) => { this.holdings = getHoldingsForFilter(holdings); diff --git a/apps/client/src/app/pages/portfolio/activities/create-or-update-activity-dialog/create-or-update-activity-dialog.component.ts b/apps/client/src/app/pages/portfolio/activities/create-or-update-activity-dialog/create-or-update-activity-dialog.component.ts index 853787f80..3b7938c8c 100644 --- a/apps/client/src/app/pages/portfolio/activities/create-or-update-activity-dialog/create-or-update-activity-dialog.component.ts +++ b/apps/client/src/app/pages/portfolio/activities/create-or-update-activity-dialog/create-or-update-activity-dialog.component.ts @@ -153,7 +153,9 @@ export class GfCreateOrUpdateActivityDialogComponent { this.defaultDateFormat = getDateFormatString(this.locale); this.dataService - .fetchPortfolioHoldings() + .fetchPortfolioHoldings({ + filters: [{ id: 'ACTIVE', type: 'HOLDING_TYPE' }] + }) .pipe(takeUntilDestroyed(this.destroyRef)) .subscribe(({ holdings }) => { this.defaultLookupItems = holdings diff --git a/apps/client/src/app/pages/portfolio/activities/import-activities-dialog/import-activities-dialog.component.ts b/apps/client/src/app/pages/portfolio/activities/import-activities-dialog/import-activities-dialog.component.ts index 48f1d5abd..98e0c0ad8 100644 --- a/apps/client/src/app/pages/portfolio/activities/import-activities-dialog/import-activities-dialog.component.ts +++ b/apps/client/src/app/pages/portfolio/activities/import-activities-dialog/import-activities-dialog.component.ts @@ -144,6 +144,10 @@ export class GfImportActivitiesDialogComponent { { id: AssetClass.FIXED_INCOME, type: 'ASSET_CLASS' + }, + { + id: 'ACTIVE', + type: 'HOLDING_TYPE' } ], range: DEFAULT_DATE_RANGE diff --git a/apps/client/src/app/pages/portfolio/analysis/analysis-page.component.ts b/apps/client/src/app/pages/portfolio/analysis/analysis-page.component.ts index 3a3c44269..26fb711bb 100644 --- a/apps/client/src/app/pages/portfolio/analysis/analysis-page.component.ts +++ b/apps/client/src/app/pages/portfolio/analysis/analysis-page.component.ts @@ -377,7 +377,10 @@ export class GfAnalysisPageComponent implements OnInit { this.dataService .fetchPortfolioHoldings({ - filters: this.userService.getFilters(), + filters: [ + ...this.userService.getFilters(), + { id: 'ACTIVE', type: 'HOLDING_TYPE' } + ], range: this.user?.settings?.dateRange }) .pipe(takeUntilDestroyed(this.destroyRef)) diff --git a/libs/ui/src/lib/assistant/assistant.component.ts b/libs/ui/src/lib/assistant/assistant.component.ts index bdbaa6cdb..6babc1039 100644 --- a/libs/ui/src/lib/assistant/assistant.component.ts +++ b/libs/ui/src/lib/assistant/assistant.component.ts @@ -479,7 +479,9 @@ export class GfAssistantComponent implements OnChanges, OnDestroy, OnInit { this.setIsOpen(true); this.dataService - .fetchPortfolioHoldings() + .fetchPortfolioHoldings({ + filters: [{ id: 'ACTIVE', type: 'HOLDING_TYPE' }] + }) .pipe(takeUntilDestroyed(this.destroyRef)) .subscribe(({ holdings }) => { this.holdings = getHoldingsForFilter(holdings);