diff --git a/CHANGELOG.md b/CHANGELOG.md index e83a6cc75..19baa4cc3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Added a simplified mode to the holdings table component +### Changed + +- Made the details of holdings excluded from analysis accessible via the activities table + ## 3.67.1 - 2026-09-05 ### Added diff --git a/apps/api/src/app/activities/activities.service.spec.ts b/apps/api/src/app/activities/activities.service.spec.ts index 4e935efb3..72d195029 100644 --- a/apps/api/src/app/activities/activities.service.spec.ts +++ b/apps/api/src/app/activities/activities.service.spec.ts @@ -216,6 +216,26 @@ describe('ActivitiesService', () => { }); }); + it('includes excluded accounts and activities when requested', async () => { + jest.spyOn(activitiesService, 'getActivities').mockResolvedValue({ + activities: [], + count: 0 + }); + + await activitiesService.getActivitiesForPortfolioCalculator({ + userCurrency: 'USD', + userId: 'user-id', + withExcludedAccountsAndActivities: true + }); + + expect(activitiesService.getActivities).toHaveBeenCalledWith({ + filters: undefined, + userCurrency: 'USD', + userId: 'user-id', + withExcludedAccountsAndActivities: true + }); + }); + it('does not adjust synthetic cash activities', async () => { const activity = createActivity({ symbol: 'AAPL' }); const cashActivity = createActivity({ diff --git a/apps/api/src/app/activities/activities.service.ts b/apps/api/src/app/activities/activities.service.ts index a0d9ffef5..7f6cbc8b0 100644 --- a/apps/api/src/app/activities/activities.service.ts +++ b/apps/api/src/app/activities/activities.service.ts @@ -759,7 +759,8 @@ export class ActivitiesService { filters, userCurrency, userId, - withCash = false + withCash = false, + withExcludedAccountsAndActivities = false }: { /** Optional filters to apply to the activities. */ filters?: Filter[]; @@ -769,13 +770,15 @@ export class ActivitiesService { userId: string; /** Whether to include cash activities in the result. */ withCash?: boolean; + /** Whether to include activities that are excluded from analysis. */ + withExcludedAccountsAndActivities?: boolean; }) { const [activities, splits] = await Promise.all([ this.getActivities({ filters, userCurrency, userId, - withExcludedAccountsAndActivities: false // TODO + withExcludedAccountsAndActivities }), this.assetProfileSplitService.getSplitsByUserId({ userId }) ]); diff --git a/apps/api/src/app/portfolio/calculator/portfolio-calculator.factory.ts b/apps/api/src/app/portfolio/calculator/portfolio-calculator.factory.ts index 7b5ab1a0d..7d95988c1 100644 --- a/apps/api/src/app/portfolio/calculator/portfolio-calculator.factory.ts +++ b/apps/api/src/app/portfolio/calculator/portfolio-calculator.factory.ts @@ -34,6 +34,7 @@ export class PortfolioCalculatorFactory { calculationType, currency, filters = [], + usePortfolioSnapshotCache = true, userId }: { accountBalanceItems?: HistoricalDataItem[]; @@ -41,6 +42,7 @@ export class PortfolioCalculatorFactory { calculationType: PerformanceCalculationType; currency: string; filters?: Filter[]; + usePortfolioSnapshotCache?: boolean; userId: string; }): PortfolioCalculator { switch (calculationType) { @@ -50,6 +52,7 @@ export class PortfolioCalculatorFactory { activities, currency, filters, + usePortfolioSnapshotCache, userId, configurationService: this.configurationService, currentRateService: this.currentRateService, @@ -64,6 +67,7 @@ export class PortfolioCalculatorFactory { activities, currency, filters, + usePortfolioSnapshotCache, userId, configurationService: this.configurationService, currentRateService: this.currentRateService, @@ -78,6 +82,7 @@ export class PortfolioCalculatorFactory { activities, currency, filters, + usePortfolioSnapshotCache, userId, configurationService: this.configurationService, currentRateService: this.currentRateService, @@ -92,6 +97,7 @@ export class PortfolioCalculatorFactory { activities, currency, filters, + usePortfolioSnapshotCache, userId, configurationService: this.configurationService, currentRateService: this.currentRateService, diff --git a/apps/api/src/app/portfolio/calculator/portfolio-calculator.ts b/apps/api/src/app/portfolio/calculator/portfolio-calculator.ts index 971d8b564..eb3d05a6b 100644 --- a/apps/api/src/app/portfolio/calculator/portfolio-calculator.ts +++ b/apps/api/src/app/portfolio/calculator/portfolio-calculator.ts @@ -90,6 +90,7 @@ export abstract class PortfolioCalculator { private snapshotPromise: Promise; private startDate: Date; private transactionPoints: TransactionPoint[]; + private usePortfolioSnapshotCache: boolean; private userId: string; public constructor({ @@ -102,6 +103,7 @@ export abstract class PortfolioCalculator { filters, portfolioSnapshotService, redisCacheService, + usePortfolioSnapshotCache = true, userId }: { accountBalanceItems: HistoricalDataItem[]; @@ -113,6 +115,7 @@ export abstract class PortfolioCalculator { filters: Filter[]; portfolioSnapshotService: PortfolioSnapshotService; redisCacheService: RedisCacheService; + usePortfolioSnapshotCache?: boolean; userId: string; }) { this.accountBalanceItems = accountBalanceItems; @@ -175,6 +178,7 @@ export abstract class PortfolioCalculator { this.portfolioSnapshotService = portfolioSnapshotService; this.redisCacheService = redisCacheService; + this.usePortfolioSnapshotCache = usePortfolioSnapshotCache; this.userId = userId; const { endDate, startDate } = getIntervalFromDateRange({ @@ -187,7 +191,11 @@ export abstract class PortfolioCalculator { this.computeTransactionPoints(); - this.snapshotPromise = this.initialize(); + this.snapshotPromise = this.usePortfolioSnapshotCache + ? this.initialize() + : this.computeSnapshot().then((snapshot) => { + this.snapshot = snapshot; + }); // Mark the rejection as handled to prevent an unhandled promise rejection // in case the snapshot promise is never awaited. Consumers awaiting it diff --git a/apps/api/src/app/portfolio/calculator/roai/portfolio-calculator-valuable.spec.ts b/apps/api/src/app/portfolio/calculator/roai/portfolio-calculator-valuable.spec.ts index ce5f90f5c..3681bc754 100644 --- a/apps/api/src/app/portfolio/calculator/roai/portfolio-calculator-valuable.spec.ts +++ b/apps/api/src/app/portfolio/calculator/roai/portfolio-calculator-valuable.spec.ts @@ -108,10 +108,11 @@ describe('PortfolioCalculator', () => { activities, calculationType: PerformanceCalculationType.ROAI, currency: 'USD', + usePortfolioSnapshotCache: false, userId: userDummyData.id }); - const portfolioSnapshot = await portfolioCalculator.computeSnapshot(); + const portfolioSnapshot = await portfolioCalculator.getSnapshot(); expect(portfolioSnapshot).toMatchObject({ currentValueInBaseCurrency: new Big('500000'), @@ -169,6 +170,9 @@ describe('PortfolioCalculator', () => { totalInvestmentValueWithCurrencyEffect: 500000 }) ); + + expect(PortfolioSnapshotServiceMock.jobsStore.size).toBe(0); + expect(RedisCacheServiceMock.cache.size).toBe(0); }); }); }); diff --git a/apps/api/src/app/portfolio/portfolio.controller.ts b/apps/api/src/app/portfolio/portfolio.controller.ts index 431a8da20..a65186100 100644 --- a/apps/api/src/app/portfolio/portfolio.controller.ts +++ b/apps/api/src/app/portfolio/portfolio.controller.ts @@ -357,7 +357,8 @@ export class PortfolioController { const holding = await this.portfolioService.getHolding({ dataSource, symbol, - userId + userId, + withExcludedActivities: true }); if (!holding) { @@ -631,7 +632,8 @@ export class PortfolioController { const holding = await this.portfolioService.getHolding({ dataSource, symbol, - userId + userId, + withExcludedActivities: true }); if (!holding) { diff --git a/apps/api/src/app/portfolio/portfolio.service.spec.ts b/apps/api/src/app/portfolio/portfolio.service.spec.ts index 227262c40..aed14e090 100644 --- a/apps/api/src/app/portfolio/portfolio.service.spec.ts +++ b/apps/api/src/app/portfolio/portfolio.service.spec.ts @@ -9,9 +9,14 @@ import { ConfigurationService } from '@ghostfolio/api/services/configuration/con 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 { SymbolProfileService } from '@ghostfolio/api/services/symbol-profile/symbol-profile.service'; -import { TAG_ID_EMERGENCY_FUND, UNKNOWN_KEY } from '@ghostfolio/common/config'; +import { + TAG_ID_EMERGENCY_FUND, + TAG_ID_EXCLUDE_FROM_ANALYSIS, + UNKNOWN_KEY +} from '@ghostfolio/common/config'; import { parseDate } from '@ghostfolio/common/helper'; import { + Activity, AssetProfileIdentifier, Filter, PortfolioSummary @@ -490,6 +495,145 @@ describe('PortfolioService', () => { }); }); + describe('getHolding', () => { + const dataSource = DataSource.YAHOO; + const symbol = 'AAPL'; + const includedActivity = { + assetProfile: { dataSource, symbol }, + tags: [] + } as Activity; + + beforeEach(() => { + jest.spyOn(userService, 'user').mockResolvedValue({ + id: userDummyData.id, + settings: { settings: { baseCurrency: 'USD' } } + } as unknown as Awaited>); + + jest + .spyOn(symbolProfileService, 'getSymbolProfiles') + .mockResolvedValue([]); + + jest + .spyOn(portfolioCalculatorFactory, 'createCalculator') + .mockReturnValue({ + getSnapshot: jest.fn().mockResolvedValue({ positions: [] }), + getTransactionPoints: jest.fn().mockReturnValue([]) + } as unknown as PortfolioCalculator); + }); + + it('keeps the cached path when the holding has included and excluded activities', async () => { + const excludedActivity = { + ...includedActivity, + tags: [{ id: TAG_ID_EXCLUDE_FROM_ANALYSIS }] + } as Activity; + const getActivities = jest + .spyOn(activitiesService, 'getActivitiesForPortfolioCalculator') + .mockResolvedValueOnce({ activities: [includedActivity], count: 1 }) + .mockResolvedValue({ + activities: [includedActivity, excludedActivity], + count: 2 + }); + + await portfolioService.getHolding({ + dataSource, + symbol, + userId: userDummyData.id, + withExcludedActivities: true + }); + + expect(getActivities).toHaveBeenCalledTimes(1); + expect(getActivities).toHaveBeenCalledWith({ + userCurrency: 'USD', + userId: userDummyData.id + }); + expect(portfolioCalculatorFactory.createCalculator).toHaveBeenCalledWith( + expect.objectContaining({ + activities: [includedActivity], + filters: undefined, + usePortfolioSnapshotCache: true + }) + ); + }); + + it.each([ + { + account: { tags: [{ id: TAG_ID_EXCLUDE_FROM_ANALYSIS }] }, + name: 'account', + tags: [] + }, + { + account: { tags: [] }, + name: 'activity tag', + tags: [{ id: TAG_ID_EXCLUDE_FROM_ANALYSIS }] + } + ])( + 'uses the direct path for a holding excluded by its $name', + async ({ account, tags }) => { + const excludedActivity = { + ...includedActivity, + account, + tags + } as Activity; + + const getActivities = jest + .spyOn(activitiesService, 'getActivitiesForPortfolioCalculator') + .mockResolvedValueOnce({ activities: [], count: 0 }) + .mockResolvedValueOnce({ + activities: [excludedActivity], + count: 1 + }); + + await portfolioService.getHolding({ + dataSource, + symbol, + userId: userDummyData.id, + withExcludedActivities: true + }); + + expect(getActivities).toHaveBeenCalledTimes(2); + expect(getActivities).toHaveBeenLastCalledWith({ + filters: [ + { id: dataSource, type: 'DATA_SOURCE' }, + { id: symbol, type: 'SYMBOL' } + ], + userCurrency: 'USD', + userId: userDummyData.id, + withExcludedAccountsAndActivities: true + }); + expect( + portfolioCalculatorFactory.createCalculator + ).toHaveBeenCalledWith( + expect.objectContaining({ + activities: [excludedActivity], + filters: [ + { id: dataSource, type: 'DATA_SOURCE' }, + { id: symbol, type: 'SYMBOL' } + ], + usePortfolioSnapshotCache: false + }) + ); + } + ); + + it('does not load excluded activities by default', async () => { + const getActivities = jest + .spyOn(activitiesService, 'getActivitiesForPortfolioCalculator') + .mockResolvedValue({ activities: [], count: 0 }); + + const holding = await portfolioService.getHolding({ + dataSource, + symbol, + userId: userDummyData.id + }); + + expect(holding).toBeUndefined(); + expect(getActivities).toHaveBeenCalledTimes(1); + expect( + portfolioCalculatorFactory.createCalculator + ).not.toHaveBeenCalled(); + }); + }); + describe('getSummary', () => { const getSummary = (args: object) => { return ( diff --git a/apps/api/src/app/portfolio/portfolio.service.ts b/apps/api/src/app/portfolio/portfolio.service.ts index 9f81754f4..19c2dd794 100644 --- a/apps/api/src/app/portfolio/portfolio.service.ts +++ b/apps/api/src/app/portfolio/portfolio.service.ts @@ -888,20 +888,47 @@ export class PortfolioService { public async getHolding({ dataSource, symbol, - userId + userId, + withExcludedActivities = false }: { userId: string; + withExcludedActivities?: boolean; } & AssetProfileIdentifier): Promise { const user = await this.userService.user({ id: userId }); const userCurrency = this.getUserCurrency(user); - const { activities } = + const holdingFilters: Filter[] = [ + { id: dataSource, type: 'DATA_SOURCE' }, + { id: symbol, type: 'SYMBOL' } + ]; + + let { activities } = await this.activitiesService.getActivitiesForPortfolioCalculator({ userCurrency, userId }); - if (activities.length === 0) { + let hasActivitiesOfHolding = activities.some(({ assetProfile }) => { + return ( + assetProfile.dataSource === dataSource && assetProfile.symbol === symbol + ); + }); + + const isExcludedHolding = withExcludedActivities && !hasActivitiesOfHolding; + + if (isExcludedHolding) { + ({ activities } = + await this.activitiesService.getActivitiesForPortfolioCalculator({ + userCurrency, + userId, + filters: holdingFilters, + withExcludedAccountsAndActivities: true + })); + + hasActivitiesOfHolding = activities.length > 0; + } + + if (!hasActivitiesOfHolding) { return undefined; } @@ -927,7 +954,9 @@ export class PortfolioService { activities, userId, calculationType: this.getUserPerformanceCalculationType(user), - currency: userCurrency + currency: userCurrency, + filters: isExcludedHolding ? holdingFilters : undefined, + usePortfolioSnapshotCache: !isExcludedHolding }); const transactionPoints = portfolioCalculator.getTransactionPoints(); @@ -2033,12 +2062,7 @@ export class PortfolioService { const nonExcludedActivities: Activity[] = []; for (const activity of activities) { - if ( - (activity.account && isAccountExcluded(activity.account)) || - activity.tags?.some(({ id }) => { - return id === TAG_ID_EXCLUDE_FROM_ANALYSIS; - }) - ) { + if (this.isExcludedFromAnalysis(activity)) { excludedActivities.push(activity); } else { nonExcludedActivities.push(activity); @@ -2418,4 +2442,13 @@ export class PortfolioService { return { accounts, platforms }; } + + private isExcludedFromAnalysis(activity: Activity) { + return ( + isAccountExcluded(activity.account) || + activity.tags?.some(({ id }) => { + return id === TAG_ID_EXCLUDE_FROM_ANALYSIS; + }) === true + ); + } } diff --git a/apps/api/src/interceptors/transform-data-source-in-response/transform-data-source-in-response.interceptor.ts b/apps/api/src/interceptors/transform-data-source-in-response/transform-data-source-in-response.interceptor.ts index 3b9addf76..31bfb620e 100644 --- a/apps/api/src/interceptors/transform-data-source-in-response/transform-data-source-in-response.interceptor.ts +++ b/apps/api/src/interceptors/transform-data-source-in-response/transform-data-source-in-response.interceptor.ts @@ -70,6 +70,8 @@ export class TransformDataSourceInResponseInterceptor< } } + data.dataProviderInfo = undefined; + if (Object.keys(valueMap).length === 0) { return data; } @@ -94,8 +96,6 @@ export class TransformDataSourceInResponseInterceptor< 'watchlist[*].dataSource' ] }); - - data.dataProviderInfo = undefined; } return data; diff --git a/apps/client/src/app/components/holding-detail-dialog/holding-detail-dialog.html b/apps/client/src/app/components/holding-detail-dialog/holding-detail-dialog.html index 51a0783fc..dff4c2a1d 100644 --- a/apps/client/src/app/components/holding-detail-dialog/holding-detail-dialog.html +++ b/apps/client/src/app/components/holding-detail-dialog/holding-detail-dialog.html @@ -358,7 +358,7 @@ - @if (dataProviderInfo) { + @if (dataProviderInfo?.name) {
{ - return id === TAG_ID_EXCLUDE_FROM_ANALYSIS; - }) === true - ); - } - public onChangePage(page: PageEvent) { this.pageChanged.emit(page); }