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/portfolio/calculator/roai/portfolio-calculator-valuable.spec.ts b/apps/api/src/app/portfolio/calculator/roai/portfolio-calculator-valuable.spec.ts index 492f58bb1..23d3d2a8c 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 @@ -112,7 +112,7 @@ describe('PortfolioCalculator', () => { usePortfolioSnapshotCache: false }); - const portfolioSnapshot = await portfolioCalculator.computeSnapshot(); + const portfolioSnapshot = await portfolioCalculator.getSnapshot(); expect(portfolioSnapshot).toMatchObject({ currentValueInBaseCurrency: new Big('500000'), 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 f4de83fc5..bfd9c04de 100644 --- a/apps/api/src/app/portfolio/portfolio.service.ts +++ b/apps/api/src/app/portfolio/portfolio.service.ts @@ -888,9 +888,11 @@ 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); @@ -900,32 +902,33 @@ export class PortfolioService { { id: symbol, type: 'SYMBOL' } ]; - const { activities: allActivitiesOfHolding } = + let { activities } = await this.activitiesService.getActivitiesForPortfolioCalculator({ userCurrency, - userId, - filters: holdingFilters, - withExcludedAccountsAndActivities: true + userId }); - if (allActivitiesOfHolding.length === 0) { - return undefined; - } - - const hasExcludedActivities = allActivitiesOfHolding.some((activity) => { - return this.isExcludedFromAnalysis(activity); + let hasActivitiesOfHolding = activities.some(({ assetProfile }) => { + return ( + assetProfile.dataSource === dataSource && assetProfile.symbol === symbol + ); }); - const activities = hasExcludedActivities - ? allActivitiesOfHolding - : ( - await this.activitiesService.getActivitiesForPortfolioCalculator({ - userCurrency, - userId - }) - ).activities; + const isExcludedHolding = withExcludedActivities && !hasActivitiesOfHolding; - if (activities.length === 0) { + if (isExcludedHolding) { + ({ activities } = + await this.activitiesService.getActivitiesForPortfolioCalculator({ + userCurrency, + userId, + filters: holdingFilters, + withExcludedAccountsAndActivities: true + })); + + hasActivitiesOfHolding = activities.length > 0; + } + + if (!hasActivitiesOfHolding) { return undefined; } @@ -952,8 +955,8 @@ export class PortfolioService { userId, calculationType: this.getUserPerformanceCalculationType(user), currency: userCurrency, - filters: hasExcludedActivities ? holdingFilters : undefined, - usePortfolioSnapshotCache: !hasExcludedActivities + filters: isExcludedHolding ? holdingFilters : undefined, + usePortfolioSnapshotCache: !isExcludedHolding }); const transactionPoints = portfolioCalculator.getTransactionPoints();