Improve handling of excluded holdings

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({

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'),

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) {

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<ReturnType<typeof userService.user>>);
+
+      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 (

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<PortfolioHoldingResponse> {
     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,
+        userId
-        filters: holdingFilters,
-        withExcludedAccountsAndActivities: true
       });
 
-    if (allActivitiesOfHolding.length === 0) {
+    let hasActivitiesOfHolding = activities.some(({ assetProfile }) => {
-      return undefined;
+      return (
-    }
+        assetProfile.dataSource === dataSource && assetProfile.symbol === symbol
-
+      );
-    const hasExcludedActivities = allActivitiesOfHolding.some((activity) => {
-      return this.isExcludedFromAnalysis(activity);
     });
 
-    const activities = hasExcludedActivities
+    const isExcludedHolding = withExcludedActivities && !hasActivitiesOfHolding;
-      ? allActivitiesOfHolding
-      : (
-          await this.activitiesService.getActivitiesForPortfolioCalculator({
-            userCurrency,
-            userId
-          })
-        ).activities;
 
-    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,
+      filters: isExcludedHolding ? holdingFilters : undefined,
-      usePortfolioSnapshotCache: !hasExcludedActivities
+      usePortfolioSnapshotCache: !isExcludedHolding
     });
 
     const transactionPoints = portfolioCalculator.getTransactionPoints();