Feature/improve allocations by account (#308)

* Improve allocations by account

* Eliminate accounts from PortfolioPosition

* Ignore cash assets in the allocation chart by sector, continent and country

* Add missing accounts to portfolio details

* Update changelog

CHANGELOG.md

@@ -15,6 +15,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 - Improved the wording for the _Restricted View_: _Presenter View_
 - Improved the styling of the tables
+- Ignored cash assets in the allocation chart by sector, continent and country
+
+### Fixed
+
+- Fixed an issue in the allocation chart by account (wrong calculation)
+- Fixed an issue in the allocation chart by account (missing cash accounts)
 
 ## 1.40.0 - 19.08.2021
 

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

@@ -5,8 +5,8 @@ import {
 } from '@ghostfolio/api/helper/object.helper';
 import { ExchangeRateDataService } from '@ghostfolio/api/services/exchange-rate-data.service';
 import {
+  PortfolioDetails,
   PortfolioPerformance,
-  PortfolioPosition,
   PortfolioReport,
   PortfolioSummary
 } from '@ghostfolio/common/interfaces';
@@ -124,13 +124,11 @@ export class PortfolioController {
     @Headers('impersonation-id') impersonationId,
     @Query('range') range,
     @Res() res: Response
-  ): Promise<{ [symbol: string]: PortfolioPosition }> {
+  ): Promise<PortfolioDetails> {
-    const { details, hasErrors } = await this.portfolioService.getDetails(
+    const { accounts, holdings, hasErrors } =
-      impersonationId,
+      await this.portfolioService.getDetails(impersonationId, range);
-      range
-    );
 
-    if (hasErrors || hasNotDefinedValuesInObject(details)) {
+    if (hasErrors || hasNotDefinedValuesInObject(holdings)) {
       res.status(StatusCodes.ACCEPTED);
     }
 
@@ -138,13 +136,13 @@ export class PortfolioController {
       impersonationId ||
       this.userService.isRestrictedView(this.request.user)
     ) {
-      const totalInvestment = Object.values(details)
+      const totalInvestment = Object.values(holdings)
         .map((portfolioPosition) => {
           return portfolioPosition.investment;
         })
         .reduce((a, b) => a + b, 0);
 
-      const totalValue = Object.values(details)
+      const totalValue = Object.values(holdings)
         .map((portfolioPosition) => {
           return this.exchangeRateDataService.toCurrency(
             portfolioPosition.quantity * portfolioPosition.marketPrice,
@@ -154,24 +152,21 @@ export class PortfolioController {
         })
         .reduce((a, b) => a + b, 0);
 
-      for (const [symbol, portfolioPosition] of Object.entries(details)) {
+      for (const [symbol, portfolioPosition] of Object.entries(holdings)) {
         portfolioPosition.grossPerformance = null;
         portfolioPosition.investment =
           portfolioPosition.investment / totalInvestment;
 
-        for (const [account, { current, original }] of Object.entries(
-          portfolioPosition.accounts
-        )) {
-          portfolioPosition.accounts[account].current = current / totalValue;
-          portfolioPosition.accounts[account].original =
-            original / totalInvestment;
-        }
-
         portfolioPosition.quantity = null;
       }
+
+      for (const [name, { current, original }] of Object.entries(accounts)) {
+        accounts[name].current = current / totalValue;
+        accounts[name].original = original / totalInvestment;
+      }
     }
 
-    return <any>res.json(details);
+    return <any>res.json({ accounts, holdings });
   }
 
   @Get('performance')

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

@@ -24,6 +24,7 @@ import { SymbolProfileService } from '@ghostfolio/api/services/symbol-profile.se
 import { UNKNOWN_KEY, ghostfolioCashSymbol } from '@ghostfolio/common/config';
 import { DATE_FORMAT, parseDate } from '@ghostfolio/common/helper';
 import {
+  PortfolioDetails,
   PortfolioPerformance,
   PortfolioPosition,
   PortfolioReport,
@@ -154,10 +155,7 @@ export class PortfolioService {
   public async getDetails(
     aImpersonationId: string,
     aDateRange: DateRange = 'max'
-  ): Promise<{
+  ): Promise<PortfolioDetails & { hasErrors: boolean }> {
-    details: { [symbol: string]: PortfolioPosition };
-    hasErrors: boolean;
-  }> {
     const userId = await this.getUserId(aImpersonationId);
 
     const userCurrency = this.request.user.Settings.currency;
@@ -171,7 +169,7 @@ export class PortfolioService {
     });
 
     if (transactionPoints?.length <= 0) {
-      return { details: {}, hasErrors: false };
+      return { accounts: {}, holdings: {}, hasErrors: false };
     }
 
     portfolioCalculator.setTransactionPoints(transactionPoints);
@@ -187,7 +185,7 @@ export class PortfolioService {
       userCurrency
     );
 
-    const details: { [symbol: string]: PortfolioPosition } = {};
+    const holdings: PortfolioDetails['holdings'] = {};
     const totalInvestment = currentPositions.totalInvestment.plus(
       cashDetails.balance
     );
@@ -211,14 +209,12 @@ export class PortfolioService {
     for (const position of currentPositions.positions) {
       portfolioItemsNow[position.symbol] = position;
     }
-    const accounts = this.getAccounts(orders, portfolioItemsNow, userCurrency);
 
     for (const item of currentPositions.positions) {
       const value = item.quantity.mul(item.marketPrice);
       const symbolProfile = symbolProfileMap[item.symbol];
       const dataProviderResponse = dataProviderResponses[item.symbol];
-      details[item.symbol] = {
+      holdings[item.symbol] = {
-        accounts,
         allocationCurrent: value.div(totalValue).toNumber(),
         allocationInvestment: item.investment.div(totalInvestment).toNumber(),
         assetClass: symbolProfile.assetClass,
@@ -241,13 +237,20 @@ export class PortfolioService {
     }
 
     // TODO: Add a cash position for each currency
-    details[ghostfolioCashSymbol] = await this.getCashPosition({
+    holdings[ghostfolioCashSymbol] = await this.getCashPosition({
       cashDetails,
       investment: totalInvestment,
       value: totalValue
     });
 
-    return { details, hasErrors: currentPositions.hasErrors };
+    const accounts = await this.getAccounts(
+      orders,
+      portfolioItemsNow,
+      userCurrency,
+      userId
+    );
+
+    return { accounts, holdings, hasErrors: currentPositions.hasErrors };
   }
 
   public async getPosition(
@@ -604,7 +607,12 @@ export class PortfolioService {
     for (const position of currentPositions.positions) {
       portfolioItemsNow[position.symbol] = position;
     }
-    const accounts = this.getAccounts(orders, portfolioItemsNow, baseCurrency);
+    const accounts = await this.getAccounts(
+      orders,
+      portfolioItemsNow,
+      baseCurrency,
+      userId
+    );
     return {
       rules: {
         accountClusterRisk: await this.rulesService.evaluate(
@@ -704,18 +712,9 @@ export class PortfolioService {
     investment: Big;
     value: Big;
   }) {
-    const accounts = {};
     const cashValue = new Big(cashDetails.balance);
 
-    cashDetails.accounts.forEach((account) => {
-      accounts[account.name] = {
-        current: account.balance,
-        original: account.balance
-      };
-    });
-
     return {
-      accounts,
       allocationCurrent: cashValue.div(value).toNumber(),
       allocationInvestment: cashValue.div(investment).toNumber(),
       assetClass: AssetClass.CASH,
@@ -797,41 +796,67 @@ export class PortfolioService {
     };
   }
 
-  private getAccounts(
+  private async getAccounts(
     orders: OrderWithAccount[],
     portfolioItemsNow: { [p: string]: TimelinePosition },
-    userCurrency
+    userCurrency: Currency,
+    userId: string
   ) {
-    const accounts: PortfolioPosition['accounts'] = {};
+    const accounts: PortfolioDetails['accounts'] = {};
-    for (const order of orders) {
-      let currentValueOfSymbol = this.exchangeRateDataService.toCurrency(
-        order.quantity * portfolioItemsNow[order.symbol].marketPrice,
-        order.currency,
-        userCurrency
-      );
-      let originalValueOfSymbol = this.exchangeRateDataService.toCurrency(
-        order.quantity * order.unitPrice,
-        order.currency,
-        userCurrency
-      );
 
-      if (order.type === 'SELL') {
+    const currentAccounts = await this.accountService.getAccounts(userId);
-        currentValueOfSymbol *= -1;
+
-        originalValueOfSymbol *= -1;
+    for (const account of currentAccounts) {
+      const ordersByAccount = orders.filter(({ accountId }) => {
+        return accountId === account.id;
+      });
+
+      if (ordersByAccount.length <= 0) {
+        // Add account without orders
+        const balance = this.exchangeRateDataService.toCurrency(
+          account.balance,
+          account.currency,
+          userCurrency
+        );
+        accounts[account.name] = {
+          current: balance,
+          original: balance
+        };
+
+        continue;
       }
 
-      if (accounts[order.Account?.name || UNKNOWN_KEY]?.current) {
+      for (const order of ordersByAccount) {
-        accounts[order.Account?.name || UNKNOWN_KEY].current +=
+        let currentValueOfSymbol = this.exchangeRateDataService.toCurrency(
-          currentValueOfSymbol;
+          order.quantity * portfolioItemsNow[order.symbol].marketPrice,
-        accounts[order.Account?.name || UNKNOWN_KEY].original +=
+          order.currency,
-          originalValueOfSymbol;
+          userCurrency
-      } else {
+        );
-        accounts[order.Account?.name || UNKNOWN_KEY] = {
+        let originalValueOfSymbol = this.exchangeRateDataService.toCurrency(
-          current: currentValueOfSymbol,
+          order.quantity * order.unitPrice,
-          original: originalValueOfSymbol
+          order.currency,
-        };
+          userCurrency
+        );
+
+        if (order.type === 'SELL') {
+          currentValueOfSymbol *= -1;
+          originalValueOfSymbol *= -1;
+        }
+
+        if (accounts[order.Account?.name || UNKNOWN_KEY]?.current) {
+          accounts[order.Account?.name || UNKNOWN_KEY].current +=
+            currentValueOfSymbol;
+          accounts[order.Account?.name || UNKNOWN_KEY].original +=
+            originalValueOfSymbol;
+        } else {
+          accounts[order.Account?.name || UNKNOWN_KEY] = {
+            current: currentValueOfSymbol,
+            original: originalValueOfSymbol
+          };
+        }
       }
     }
+
     return accounts;
   }
 

apps/api/src/models/rules/account-cluster-risk/current-investment.ts

@@ -1,16 +1,17 @@
 import { RuleSettings } from '@ghostfolio/api/models/interfaces/rule-settings.interface';
 import { UserSettings } from '@ghostfolio/api/models/interfaces/user-settings.interface';
 import { ExchangeRateDataService } from '@ghostfolio/api/services/exchange-rate-data.service';
-import { PortfolioPosition } from '@ghostfolio/common/interfaces';
+import {
+  PortfolioDetails,
+  PortfolioPosition
+} from '@ghostfolio/common/interfaces';
 
 import { Rule } from '../../rule';
 
 export class AccountClusterRiskCurrentInvestment extends Rule<Settings> {
   public constructor(
     protected exchangeRateDataService: ExchangeRateDataService,
-    private accounts: {
+    private accounts: PortfolioDetails['accounts']
-      [account: string]: { current: number; original: number };
-    }
   ) {
     super(exchangeRateDataService, {
       name: 'Current Investment'

apps/api/src/models/rules/account-cluster-risk/initial-investment.ts

@@ -1,6 +1,9 @@
 import { RuleSettings } from '@ghostfolio/api/models/interfaces/rule-settings.interface';
 import { UserSettings } from '@ghostfolio/api/models/interfaces/user-settings.interface';
-import { PortfolioPosition } from '@ghostfolio/common/interfaces';
+import {
+  PortfolioDetails,
+  PortfolioPosition
+} from '@ghostfolio/common/interfaces';
 import { ExchangeRateDataService } from 'apps/api/src/services/exchange-rate-data.service';
 
 import { Rule } from '../../rule';
@@ -8,9 +11,7 @@ import { Rule } from '../../rule';
 export class AccountClusterRiskInitialInvestment extends Rule<Settings> {
   public constructor(
     protected exchangeRateDataService: ExchangeRateDataService,
-    private accounts: {
+    private accounts: PortfolioDetails['accounts']
-      [account: string]: { current: number; original: number };
-    }
   ) {
     super(exchangeRateDataService, {
       name: 'Initial Investment'

apps/api/src/models/rules/account-cluster-risk/single-account.ts

@@ -1,5 +1,6 @@
 import { RuleSettings } from '@ghostfolio/api/models/interfaces/rule-settings.interface';
 import { UserSettings } from '@ghostfolio/api/models/interfaces/user-settings.interface';
+import { PortfolioDetails } from '@ghostfolio/common/interfaces';
 import { ExchangeRateDataService } from 'apps/api/src/services/exchange-rate-data.service';
 
 import { Rule } from '../../rule';
@@ -7,9 +8,7 @@ import { Rule } from '../../rule';
 export class AccountClusterRiskSingleAccount extends Rule<RuleSettings> {
   public constructor(
     protected exchangeRateDataService: ExchangeRateDataService,
-    private accounts: {
+    private accounts: PortfolioDetails['accounts']
-      [account: string]: { current: number; original: number };
-    }
   ) {
     super(exchangeRateDataService, {
       name: 'Single Account'

apps/client/src/app/pages/portfolio/allocations/allocations-page.component.ts

@@ -3,8 +3,13 @@ import { ToggleOption } from '@ghostfolio/client/components/toggle/interfaces/to
 import { DataService } from '@ghostfolio/client/services/data.service';
 import { ImpersonationStorageService } from '@ghostfolio/client/services/impersonation-storage.service';
 import { UserService } from '@ghostfolio/client/services/user/user.service';
-import { UNKNOWN_KEY } from '@ghostfolio/common/config';
+import { ghostfolioCashSymbol, UNKNOWN_KEY } from '@ghostfolio/common/config';
-import { PortfolioPosition, User } from '@ghostfolio/common/interfaces';
+import {
+  PortfolioDetails,
+  PortfolioPosition,
+  User
+} from '@ghostfolio/common/interfaces';
+import { AssetClass } from '@prisma/client';
 import { DeviceDetectorService } from 'ngx-device-detector';
 import { Subject } from 'rxjs';
 import { takeUntil } from 'rxjs/operators';
@@ -31,7 +36,7 @@ export class AllocationsPageComponent implements OnDestroy, OnInit {
     { label: 'Initial', value: 'original' },
     { label: 'Current', value: 'current' }
   ];
-  public portfolioPositions: { [symbol: string]: PortfolioPosition };
+  public portfolioDetails: PortfolioDetails;
   public positions: { [symbol: string]: any };
   public positionsArray: PortfolioPosition[];
   public sectors: {
@@ -66,11 +71,12 @@ export class AllocationsPageComponent implements OnDestroy, OnInit {
       });
 
     this.dataService
-      .fetchPortfolioPositions({})
+      .fetchPortfolioDetails({})
       .pipe(takeUntil(this.unsubscribeSubject))
-      .subscribe((response = {}) => {
+      .subscribe((portfolioDetails) => {
-        this.portfolioPositions = response;
+        this.portfolioDetails = portfolioDetails;
-        this.initializeAnalysisData(this.portfolioPositions, this.period);
+
+        this.initializeAnalysisData(this.period);
 
         this.changeDetectorRef.markForCheck();
       });
@@ -86,12 +92,7 @@ export class AllocationsPageComponent implements OnDestroy, OnInit {
       });
   }
 
-  public initializeAnalysisData(
+  public initializeAnalysisData(aPeriod: string) {
-    aPortfolioPositions: {
-      [symbol: string]: PortfolioPosition;
-    },
-    aPeriod: string
-  ) {
     this.accounts = {};
     this.continents = {
       [UNKNOWN_KEY]: {
@@ -114,7 +115,18 @@ export class AllocationsPageComponent implements OnDestroy, OnInit {
       }
     };
 
-    for (const [symbol, position] of Object.entries(aPortfolioPositions)) {
+    for (const [name, { current, original }] of Object.entries(
+      this.portfolioDetails.accounts
+    )) {
+      this.accounts[name] = {
+        name,
+        value: aPeriod === 'original' ? original : current
+      };
+    }
+
+    for (const [symbol, position] of Object.entries(
+      this.portfolioDetails.holdings
+    )) {
       this.positions[symbol] = {
         assetClass: position.assetClass,
         currency: position.currency,
@@ -126,84 +138,74 @@ export class AllocationsPageComponent implements OnDestroy, OnInit {
       };
       this.positionsArray.push(position);
 
-      for (const [account, { current, original }] of Object.entries(
+      if (position.assetClass !== AssetClass.CASH) {
-        position.accounts
+        // Prepare analysis data by continents, countries and sectors except for cash
-      )) {
+
-        if (this.accounts[account]?.value) {
+        if (position.countries.length > 0) {
-          this.accounts[account].value +=
+          for (const country of position.countries) {
-            aPeriod === 'original' ? original : current;
+            const { code, continent, name, weight } = country;
+
+            if (this.continents[continent]?.value) {
+              this.continents[continent].value += weight * position.value;
+            } else {
+              this.continents[continent] = {
+                name: continent,
+                value:
+                  weight *
+                  (aPeriod === 'original'
+                    ? this.portfolioDetails.holdings[symbol].investment
+                    : this.portfolioDetails.holdings[symbol].value)
+              };
+            }
+
+            if (this.countries[code]?.value) {
+              this.countries[code].value += weight * position.value;
+            } else {
+              this.countries[code] = {
+                name,
+                value:
+                  weight *
+                  (aPeriod === 'original'
+                    ? this.portfolioDetails.holdings[symbol].investment
+                    : this.portfolioDetails.holdings[symbol].value)
+              };
+            }
+          }
         } else {
-          this.accounts[account] = {
+          this.continents[UNKNOWN_KEY].value +=
-            name: account,
+            aPeriod === 'original'
-            value: aPeriod === 'original' ? original : current
+              ? this.portfolioDetails.holdings[symbol].investment
-          };
+              : this.portfolioDetails.holdings[symbol].value;
+
+          this.countries[UNKNOWN_KEY].value +=
+            aPeriod === 'original'
+              ? this.portfolioDetails.holdings[symbol].investment
+              : this.portfolioDetails.holdings[symbol].value;
         }
-      }
 
-      if (position.countries.length > 0) {
+        if (position.sectors.length > 0) {
-        for (const country of position.countries) {
+          for (const sector of position.sectors) {
-          const { code, continent, name, weight } = country;
+            const { name, weight } = sector;
 
-          if (this.continents[continent]?.value) {
+            if (this.sectors[name]?.value) {
-            this.continents[continent].value += weight * position.value;
+              this.sectors[name].value += weight * position.value;
-          } else {
+            } else {
-            this.continents[continent] = {
+              this.sectors[name] = {
-              name: continent,
+                name,
-              value:
+                value:
-                weight *
+                  weight *
-                (aPeriod === 'original'
+                  (aPeriod === 'original'
-                  ? this.portfolioPositions[symbol].investment
+                    ? this.portfolioDetails.holdings[symbol].investment
-                  : this.portfolioPositions[symbol].value)
+                    : this.portfolioDetails.holdings[symbol].value)
-            };
+              };
-          }
+            }
-
-          if (this.countries[code]?.value) {
-            this.countries[code].value += weight * position.value;
-          } else {
-            this.countries[code] = {
-              name,
-              value:
-                weight *
-                (aPeriod === 'original'
-                  ? this.portfolioPositions[symbol].investment
-                  : this.portfolioPositions[symbol].value)
-            };
           }
+        } else {
+          this.sectors[UNKNOWN_KEY].value +=
+            aPeriod === 'original'
+              ? this.portfolioDetails.holdings[symbol].investment
+              : this.portfolioDetails.holdings[symbol].value;
         }
-      } else {
-        this.continents[UNKNOWN_KEY].value +=
-          aPeriod === 'original'
-            ? this.portfolioPositions[symbol].investment
-            : this.portfolioPositions[symbol].value;
-
-        this.countries[UNKNOWN_KEY].value +=
-          aPeriod === 'original'
-            ? this.portfolioPositions[symbol].investment
-            : this.portfolioPositions[symbol].value;
-      }
-
-      if (position.sectors.length > 0) {
-        for (const sector of position.sectors) {
-          const { name, weight } = sector;
-
-          if (this.sectors[name]?.value) {
-            this.sectors[name].value += weight * position.value;
-          } else {
-            this.sectors[name] = {
-              name,
-              value:
-                weight *
-                (aPeriod === 'original'
-                  ? this.portfolioPositions[symbol].investment
-                  : this.portfolioPositions[symbol].value)
-            };
-          }
-        }
-      } else {
-        this.sectors[UNKNOWN_KEY].value +=
-          aPeriod === 'original'
-            ? this.portfolioPositions[symbol].investment
-            : this.portfolioPositions[symbol].value;
       }
     }
   }
@@ -211,7 +213,7 @@ export class AllocationsPageComponent implements OnDestroy, OnInit {
   public onChangePeriod(aValue: string) {
     this.period = aValue;
 
-    this.initializeAnalysisData(this.portfolioPositions, this.period);
+    this.initializeAnalysisData(this.period);
   }
 
   public ngOnDestroy() {

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

@@ -20,8 +20,8 @@ import {
   AdminData,
   Export,
   InfoItem,
+  PortfolioDetails,
   PortfolioPerformance,
-  PortfolioPosition,
   PortfolioReport,
   PortfolioSummary,
   User
@@ -148,17 +148,16 @@ export class DataService {
     return this.http.get<InvestmentItem[]>('/api/portfolio/investments');
   }
 
-  public fetchPortfolioPerformance(aParams: { [param: string]: any }) {
+  public fetchPortfolioDetails(aParams: { [param: string]: any }) {
-    return this.http.get<PortfolioPerformance>('/api/portfolio/performance', {
+    return this.http.get<PortfolioDetails>('/api/portfolio/details', {
       params: aParams
     });
   }
 
-  public fetchPortfolioPositions(aParams: { [param: string]: any }) {
+  public fetchPortfolioPerformance(aParams: { [param: string]: any }) {
-    return this.http.get<{ [symbol: string]: PortfolioPosition }>(
+    return this.http.get<PortfolioPerformance>('/api/portfolio/performance', {
-      '/api/portfolio/details',
+      params: aParams
-      { params: aParams }
+    });
-    );
   }
 
   public fetchPortfolioReport() {

libs/common/src/lib/interfaces/index.ts

@@ -2,6 +2,7 @@ import { Access } from './access.interface';
 import { AdminData } from './admin-data.interface';
 import { Export } from './export.interface';
 import { InfoItem } from './info-item.interface';
+import { PortfolioDetails } from './portfolio-details.interface';
 import { PortfolioItem } from './portfolio-item.interface';
 import { PortfolioOverview } from './portfolio-overview.interface';
 import { PortfolioPerformance } from './portfolio-performance.interface';
@@ -20,6 +21,7 @@ export {
   AdminData,
   Export,
   InfoItem,
+  PortfolioDetails,
   PortfolioItem,
   PortfolioOverview,
   PortfolioPerformance,

libs/common/src/lib/interfaces/portfolio-details.interface.ts

@@ -0,0 +1,8 @@
+import { PortfolioPosition } from '@ghostfolio/common/interfaces';
+
+export interface PortfolioDetails {
+  accounts: {
+    [name: string]: { current: number; original: number };
+  };
+  holdings: { [symbol: string]: PortfolioPosition };
+}

libs/common/src/lib/interfaces/portfolio-position.interface.ts

@@ -5,9 +5,6 @@ import { Country } from './country.interface';
 import { Sector } from './sector.interface';
 
 export interface PortfolioPosition {
-  accounts: {
-    [name: string]: { current: number; original: number };
-  };
   allocationCurrent: number;
   allocationInvestment: number;
   assetClass?: AssetClass;