Feature/refactor filters (#1299)

* Refactor filters

Co-Authored-By: Zakaria YAHI <9142557+ZakYahi@users.noreply.github.com>

apps/api/src/app/account/account.controller.ts

@@ -95,11 +95,10 @@ export class AccountController {
       );
 
     let accountsWithAggregations =
-      await this.portfolioService.getAccountsWithAggregations(
-        impersonationUserId || this.request.user.id,
-        undefined,
-        true
-      );
+      await this.portfolioService.getAccountsWithAggregations({
+        userId: impersonationUserId || this.request.user.id,
+        withExcludedAccounts: true
+      });
 
     if (
       impersonationUserId ||
@@ -139,11 +138,11 @@ export class AccountController {
       );
 
     let accountsWithAggregations =
-      await this.portfolioService.getAccountsWithAggregations(
-        impersonationUserId || this.request.user.id,
-        [{ id, type: 'ACCOUNT' }],
-        true
-      );
+      await this.portfolioService.getAccountsWithAggregations({
+        filters: [{ id, type: 'ACCOUNT' }],
+        userId: impersonationUserId || this.request.user.id,
+        withExcludedAccounts: true
+      });
 
     if (
       impersonationUserId ||

apps/api/src/app/order/order.controller.ts

@@ -3,8 +3,8 @@ import { nullifyValuesInObjects } from '@ghostfolio/api/helper/object.helper';
 import { RedactValuesInResponseInterceptor } from '@ghostfolio/api/interceptors/redact-values-in-response.interceptor';
 import { TransformDataSourceInRequestInterceptor } from '@ghostfolio/api/interceptors/transform-data-source-in-request.interceptor';
 import { TransformDataSourceInResponseInterceptor } from '@ghostfolio/api/interceptors/transform-data-source-in-response.interceptor';
+import { ApiService } from '@ghostfolio/api/services/api/api.service';
 import { ImpersonationService } from '@ghostfolio/api/services/impersonation.service';
-import { Filter } from '@ghostfolio/common/interfaces';
 import { hasPermission, permissions } from '@ghostfolio/common/permissions';
 import type { RequestWithUser } from '@ghostfolio/common/types';
 import {
@@ -36,6 +36,7 @@ import { UpdateOrderDto } from './update-order.dto';
 @Controller('order')
 export class OrderController {
   public constructor(
+    private readonly apiService: ApiService,
     private readonly impersonationService: ImpersonationService,
     private readonly orderService: OrderService,
     @Inject(REQUEST) private readonly request: RequestWithUser,
@@ -73,30 +74,11 @@ export class OrderController {
     @Query('assetClasses') filterByAssetClasses?: string,
     @Query('tags') filterByTags?: string
   ): Promise<Activities> {
-    const accountIds = filterByAccounts?.split(',') ?? [];
-    const assetClasses = filterByAssetClasses?.split(',') ?? [];
-    const tagIds = filterByTags?.split(',') ?? [];
-
-    const filters: Filter[] = [
-      ...accountIds.map((accountId) => {
-        return <Filter>{
-          id: accountId,
-          type: 'ACCOUNT'
-        };
-      }),
-      ...assetClasses.map((assetClass) => {
-        return <Filter>{
-          id: assetClass,
-          type: 'ASSET_CLASS'
-        };
-      }),
-      ...tagIds.map((tagId) => {
-        return <Filter>{
-          id: tagId,
-          type: 'TAG'
-        };
-      })
-    ];
+    const filters = this.apiService.buildFiltersFromQueryParams({
+      filterByAccounts,
+      filterByAssetClasses,
+      filterByTags
+    });
 
     const impersonationUserId =
       await this.impersonationService.validateImpersonationId(

apps/api/src/app/order/order.module.ts

@@ -2,6 +2,7 @@ import { AccountService } from '@ghostfolio/api/app/account/account.service';
 import { CacheModule } from '@ghostfolio/api/app/cache/cache.module';
 import { RedisCacheModule } from '@ghostfolio/api/app/redis-cache/redis-cache.module';
 import { UserModule } from '@ghostfolio/api/app/user/user.module';
+import { ApiModule } from '@ghostfolio/api/services/api/api.module';
 import { ConfigurationModule } from '@ghostfolio/api/services/configuration.module';
 import { DataGatheringModule } from '@ghostfolio/api/services/data-gathering.module';
 import { DataProviderModule } from '@ghostfolio/api/services/data-provider/data-provider.module';
@@ -18,6 +19,7 @@ import { OrderService } from './order.service';
   controllers: [OrderController],
   exports: [OrderService],
   imports: [
+    ApiModule,
     CacheModule,
     ConfigurationModule,
     DataGatheringModule,

apps/api/src/app/portfolio/portfolio.controller.ts

@@ -7,18 +7,17 @@ import {
 import { RedactValuesInResponseInterceptor } from '@ghostfolio/api/interceptors/redact-values-in-response.interceptor';
 import { TransformDataSourceInRequestInterceptor } from '@ghostfolio/api/interceptors/transform-data-source-in-request.interceptor';
 import { TransformDataSourceInResponseInterceptor } from '@ghostfolio/api/interceptors/transform-data-source-in-response.interceptor';
+import { ApiService } from '@ghostfolio/api/services/api/api.service';
 import { ConfigurationService } from '@ghostfolio/api/services/configuration.service';
 import { ExchangeRateDataService } from '@ghostfolio/api/services/exchange-rate-data.service';
 import { parseDate } from '@ghostfolio/common/helper';
 import {
-  Filter,
   PortfolioChart,
   PortfolioDetails,
   PortfolioInvestments,
   PortfolioPerformanceResponse,
   PortfolioPublicDetails,
-  PortfolioReport,
-  PortfolioSummary
+  PortfolioReport
 } from '@ghostfolio/common/interfaces';
 import { InvestmentItem } from '@ghostfolio/common/interfaces/investment-item.interface';
 import type {
@@ -52,6 +51,7 @@ export class PortfolioController {
 
   public constructor(
     private readonly accessService: AccessService,
+    private readonly apiService: ApiService,
     private readonly configurationService: ConfigurationService,
     private readonly exchangeRateDataService: ExchangeRateDataService,
     private readonly portfolioService: PortfolioService,
@@ -123,32 +123,11 @@ export class PortfolioController {
   ): Promise<PortfolioDetails & { hasError: boolean }> {
     let hasError = false;
 
-    const accountIds = filterByAccounts?.split(',') ?? [];
-    const assetClasses = filterByAssetClasses?.split(',') ?? [];
-    const tagIds = filterByTags?.split(',') ?? [];
-
-    const filters: Filter[] = [
-      ...accountIds.map((accountId) => {
-        return <Filter>{
-          id: accountId,
-          type: 'ACCOUNT'
-        };
-      }),
-      ...assetClasses.map((assetClass) => {
-        return <Filter>{
-          id: assetClass,
-          type: 'ASSET_CLASS'
-        };
-      }),
-      ...tagIds.map((tagId) => {
-        return <Filter>{
-          id: tagId,
-          type: 'TAG'
-        };
-      })
-    ];
-
-    let portfolioSummary: PortfolioSummary;
+    const filters = this.apiService.buildFiltersFromQueryParams({
+      filterByAccounts,
+      filterByAssetClasses,
+      filterByTags
+    });
 
     const {
       accounts,
@@ -158,18 +137,18 @@ export class PortfolioController {
       holdings,
       summary,
       totalValueInBaseCurrency
-    } = await this.portfolioService.getDetails(
+    } = await this.portfolioService.getDetails({
+      filters,
       impersonationId,
-      this.request.user.id,
-      range,
-      filters
-    );
+      dateRange: range,
+      userId: this.request.user.id
+    });
 
     if (hasErrors || hasNotDefinedValuesInObject(holdings)) {
       hasError = true;
     }
 
-    portfolioSummary = summary;
+    let portfolioSummary = summary;
 
     if (
       impersonationId ||
@@ -400,12 +379,12 @@ export class PortfolioController {
       hasDetails = user.subscription.type === 'Premium';
     }
 
-    const { holdings } = await this.portfolioService.getDetails(
-      access.userId,
-      access.userId,
-      'max',
-      [{ id: 'EQUITY', type: 'ASSET_CLASS' }]
-    );
+    const { holdings } = await this.portfolioService.getDetails({
+      dateRange: 'max',
+      filters: [{ id: 'EQUITY', type: 'ASSET_CLASS' }],
+      impersonationId: access.userId,
+      userId: access.userId
+    });
 
     const portfolioPublicDetails: PortfolioPublicDetails = {
       hasDetails,

apps/api/src/app/portfolio/portfolio.module.ts

@@ -2,6 +2,7 @@ import { AccessModule } from '@ghostfolio/api/app/access/access.module';
 import { AccountService } from '@ghostfolio/api/app/account/account.service';
 import { OrderModule } from '@ghostfolio/api/app/order/order.module';
 import { UserModule } from '@ghostfolio/api/app/user/user.module';
+import { ApiModule } from '@ghostfolio/api/services/api/api.module';
 import { ConfigurationModule } from '@ghostfolio/api/services/configuration.module';
 import { DataGatheringModule } from '@ghostfolio/api/services/data-gathering.module';
 import { DataProviderModule } from '@ghostfolio/api/services/data-provider/data-provider.module';
@@ -22,6 +23,7 @@ import { RulesService } from './rules.service';
   exports: [PortfolioService],
   imports: [
     AccessModule,
+    ApiModule,
     ConfigurationModule,
     DataGatheringModule,
     DataProviderModule,

apps/api/src/app/portfolio/portfolio.service.ts

@@ -107,15 +107,19 @@ export class PortfolioService {
     this.baseCurrency = this.configurationService.get('BASE_CURRENCY');
   }
 
-  public async getAccounts(
-    aUserId: string,
-    aFilters?: Filter[],
+  public async getAccounts({
+    filters,
+    userId,
     withExcludedAccounts = false
-  ): Promise<AccountWithValue[]> {
-    const where: Prisma.AccountWhereInput = { userId: aUserId };
+  }: {
+    filters?: Filter[];
+    userId: string;
+    withExcludedAccounts?: boolean;
+  }): Promise<AccountWithValue[]> {
+    const where: Prisma.AccountWhereInput = { userId: userId };
 
-    if (aFilters?.[0].id && aFilters?.[0].type === 'ACCOUNT') {
-      where.id = aFilters[0].id;
+    if (filters?.[0].id && filters?.[0].type === 'ACCOUNT') {
+      where.id = filters[0].id;
     }
 
     const [accounts, details] = await Promise.all([
@@ -124,13 +128,12 @@ export class PortfolioService {
         include: { Order: true, Platform: true },
         orderBy: { name: 'asc' }
       }),
-      this.getDetails(
-        aUserId,
-        aUserId,
-        undefined,
-        aFilters,
-        withExcludedAccounts
-      )
+      this.getDetails({
+        filters,
+        userId,
+        withExcludedAccounts,
+        impersonationId: userId
+      })
     ]);
 
     const userCurrency = this.request.user.Settings.settings.baseCurrency;
@@ -168,16 +171,20 @@ export class PortfolioService {
     });
   }
 
-  public async getAccountsWithAggregations(
-    aUserId: string,
-    aFilters?: Filter[],
+  public async getAccountsWithAggregations({
+    filters,
+    userId,
     withExcludedAccounts = false
-  ): Promise<Accounts> {
-    const accounts = await this.getAccounts(
-      aUserId,
-      aFilters,
+  }: {
+    filters?: Filter[];
+    userId: string;
+    withExcludedAccounts?: boolean;
+  }): Promise<Accounts> {
+    const accounts = await this.getAccounts({
+      filters,
+      userId,
       withExcludedAccounts
-    );
+    });
     let totalBalanceInBaseCurrency = new Big(0);
     let totalValueInBaseCurrency = new Big(0);
     let transactionCount = 0;
@@ -421,14 +428,21 @@ export class PortfolioService {
     };
   }
 
-  public async getDetails(
-    aImpersonationId: string,
-    aUserId: string,
-    aDateRange: DateRange = 'max',
-    aFilters?: Filter[],
+  public async getDetails({
+    impersonationId,
+    userId,
+    dateRange = 'max',
+    filters,
     withExcludedAccounts = false
-  ): Promise<PortfolioDetails & { hasErrors: boolean }> {
-    const userId = await this.getUserId(aImpersonationId, aUserId);
+  }: {
+    impersonationId: string;
+    userId: string;
+    dateRange?: DateRange;
+    filters?: Filter[];
+    withExcludedAccounts?: boolean;
+  }): Promise<PortfolioDetails & { hasErrors: boolean }> {
+    // TODO:
+    userId = await this.getUserId(impersonationId, userId);
     const user = await this.userService.user({ id: userId });
 
     const emergencyFund = new Big(
@@ -441,9 +455,9 @@ export class PortfolioService {
 
     const { orders, portfolioOrders, transactionPoints } =
       await this.getTransactionPoints({
+        filters,
         userId,
-        withExcludedAccounts,
-        filters: aFilters
+        withExcludedAccounts
       });
 
     const portfolioCalculator = new PortfolioCalculator({
@@ -457,15 +471,15 @@ export class PortfolioService {
     const portfolioStart = parseDate(
       transactionPoints[0]?.date ?? format(new Date(), DATE_FORMAT)
     );
-    const startDate = this.getStartDate(aDateRange, portfolioStart);
+    const startDate = this.getStartDate(dateRange, portfolioStart);
     const currentPositions = await portfolioCalculator.getCurrentPositions(
       startDate
     );
 
     const cashDetails = await this.accountService.getCashDetails({
+      filters,
       userId,
-      currency: userCurrency,
-      filters: aFilters
+      currency: userCurrency
     });
 
     const holdings: PortfolioDetails['holdings'] = {};
@@ -475,10 +489,10 @@ export class PortfolioService {
     let filteredValueInBaseCurrency = currentPositions.currentValue;
 
     if (
-      aFilters?.length === 0 ||
-      (aFilters?.length === 1 &&
-        aFilters[0].type === 'ASSET_CLASS' &&
-        aFilters[0].id === 'CASH')
+      filters?.length === 0 ||
+      (filters?.length === 1 &&
+        filters[0].type === 'ASSET_CLASS' &&
+        filters[0].id === 'CASH')
     ) {
       filteredValueInBaseCurrency = filteredValueInBaseCurrency.plus(
         cashDetails.balanceInBaseCurrency
@@ -574,10 +588,10 @@ export class PortfolioService {
     }
 
     if (
-      aFilters?.length === 0 ||
-      (aFilters?.length === 1 &&
-        aFilters[0].type === 'ASSET_CLASS' &&
-        aFilters[0].id === 'CASH')
+      filters?.length === 0 ||
+      (filters?.length === 1 &&
+        filters[0].type === 'ASSET_CLASS' &&
+        filters[0].id === 'CASH')
     ) {
       const cashPositions = await this.getCashPositions({
         cashDetails,
@@ -593,15 +607,15 @@ export class PortfolioService {
     }
 
     const accounts = await this.getValueOfAccounts({
+      filters,
       orders,
       portfolioItemsNow,
       userCurrency,
       userId,
-      withExcludedAccounts,
-      filters: aFilters
+      withExcludedAccounts
     });
 
-    const summary = await this.getSummary(aImpersonationId);
+    const summary = await this.getSummary(impersonationId);
 
     return {
       accounts,

apps/api/src/services/api/api.module.ts

@@ -0,0 +1,9 @@
+import { Module } from '@nestjs/common';
+
+import { ApiService } from './api.service';
+
+@Module({
+  exports: [ApiService],
+  providers: [ApiService]
+})
+export class ApiModule {}

apps/api/src/services/api/api.service.ts

@@ -0,0 +1,42 @@
+import { Filter } from '@ghostfolio/common/interfaces';
+import { Injectable } from '@nestjs/common';
+
+@Injectable()
+export class ApiService {
+  public constructor() {}
+
+  public buildFiltersFromQueryParams({
+    filterByAccounts,
+    filterByAssetClasses,
+    filterByTags
+  }: {
+    filterByAccounts?: string;
+    filterByAssetClasses?: string;
+    filterByTags?: string;
+  }): Filter[] {
+    const accountIds = filterByAccounts?.split(',') ?? [];
+    const assetClasses = filterByAssetClasses?.split(',') ?? [];
+    const tagIds = filterByTags?.split(',') ?? [];
+
+    return [
+      ...accountIds.map((accountId) => {
+        return <Filter>{
+          id: accountId,
+          type: 'ACCOUNT'
+        };
+      }),
+      ...assetClasses.map((assetClass) => {
+        return <Filter>{
+          id: assetClass,
+          type: 'ASSET_CLASS'
+        };
+      }),
+      ...tagIds.map((tagId) => {
+        return <Filter>{
+          id: tagId,
+          type: 'TAG'
+        };
+      })
+    ];
+  }
+}

apps/client/src/app/services/data.service.ts

@@ -74,60 +74,19 @@ export class DataService {
   }: {
     filters?: Filter[];
   }): Observable<Activities> {
-    let params = new HttpParams();
-
-    if (filters?.length > 0) {
-      const {
-        ACCOUNT: filtersByAccount,
-        ASSET_CLASS: filtersByAssetClass,
-        TAG: filtersByTag
-      } = groupBy(filters, (filter) => {
-        return filter.type;
-      });
-
-      if (filtersByAccount) {
-        params = params.append(
-          'accounts',
-          filtersByAccount
-            .map(({ id }) => {
-              return id;
-            })
-            .join(',')
-        );
-      }
-
-      if (filtersByAssetClass) {
-        params = params.append(
-          'assetClasses',
-          filtersByAssetClass
-            .map(({ id }) => {
-              return id;
-            })
-            .join(',')
-        );
-      }
-
-      if (filtersByTag) {
-        params = params.append(
-          'tags',
-          filtersByTag
-            .map(({ id }) => {
-              return id;
-            })
-            .join(',')
-        );
-      }
-    }
-
-    return this.http.get<any>('/api/v1/order', { params }).pipe(
-      map(({ activities }) => {
-        for (const activity of activities) {
-          activity.createdAt = parseISO(activity.createdAt);
-          activity.date = parseISO(activity.date);
-        }
-        return { activities };
+    return this.http
+      .get<any>('/api/v1/order', {
+        params: this.buildFiltersAsQueryParams({ filters })
       })
-    );
+      .pipe(
+        map(({ activities }) => {
+          for (const activity of activities) {
+            activity.createdAt = parseISO(activity.createdAt);
+            activity.date = parseISO(activity.date);
+          }
+          return { activities };
+        })
+      );
   }
 
   public fetchAdminData() {
@@ -135,30 +94,8 @@ export class DataService {
   }
 
   public fetchAdminMarketData({ filters }: { filters?: Filter[] }) {
-    let params = new HttpParams();
-
-    if (filters?.length > 0) {
-      const { ASSET_SUB_CLASS: filtersByAssetSubClass } = groupBy(
-        filters,
-        (filter) => {
-          return filter.type;
-        }
-      );
-
-      if (filtersByAssetSubClass) {
-        params = params.append(
-          'assetSubClasses',
-          filtersByAssetSubClass
-            .map(({ id }) => {
-              return id;
-            })
-            .join(',')
-        );
-      }
-    }
-
     return this.http.get<AdminMarketData>('/api/v1/admin/market-data', {
-      params
+      params: this.buildFiltersAsQueryParams({ filters })
     });
   }
 
@@ -306,54 +243,9 @@ export class DataService {
   }: {
     filters?: Filter[];
   }): Observable<PortfolioDetails> {
-    let params = new HttpParams();
-
-    if (filters?.length > 0) {
-      const {
-        ACCOUNT: filtersByAccount,
-        ASSET_CLASS: filtersByAssetClass,
-        TAG: filtersByTag
-      } = groupBy(filters, (filter) => {
-        return filter.type;
-      });
-
-      if (filtersByAccount) {
-        params = params.append(
-          'accounts',
-          filtersByAccount
-            .map(({ id }) => {
-              return id;
-            })
-            .join(',')
-        );
-      }
-
-      if (filtersByAssetClass) {
-        params = params.append(
-          'assetClasses',
-          filtersByAssetClass
-            .map(({ id }) => {
-              return id;
-            })
-            .join(',')
-        );
-      }
-
-      if (filtersByTag) {
-        params = params.append(
-          'tags',
-          filtersByTag
-            .map(({ id }) => {
-              return id;
-            })
-            .join(',')
-        );
-      }
-    }
-
     return this.http
       .get<any>('/api/v1/portfolio/details', {
-        params
+        params: this.buildFiltersAsQueryParams({ filters })
       })
       .pipe(
         map((response) => {
@@ -458,4 +350,53 @@ export class DataService {
       couponCode
     });
   }
+
+  private buildFiltersAsQueryParams({ filters }: { filters?: Filter[] }) {
+    let params = new HttpParams();
+
+    if (filters?.length > 0) {
+      const {
+        ACCOUNT: filtersByAccount,
+        ASSET_CLASS: filtersByAssetClass,
+        TAG: filtersByTag
+      } = groupBy(filters, (filter) => {
+        return filter.type;
+      });
+
+      if (filtersByAccount) {
+        params = params.append(
+          'accounts',
+          filtersByAccount
+            .map(({ id }) => {
+              return id;
+            })
+            .join(',')
+        );
+      }
+
+      if (filtersByAssetClass) {
+        params = params.append(
+          'assetClasses',
+          filtersByAssetClass
+            .map(({ id }) => {
+              return id;
+            })
+            .join(',')
+        );
+      }
+
+      if (filtersByTag) {
+        params = params.append(
+          'tags',
+          filtersByTag
+            .map(({ id }) => {
+              return id;
+            })
+            .join(',')
+        );
+      }
+    }
+
+    return params;
+  }
 }