Feature/improve impersonation mode (#293)

* Improve the impersonation mode * Update changelog
2021-08-14 19:15:26 +02:00 · 2021-08-14 19:15:26 +02:00 · 0ee2258af8
commit 0ee2258af8
parent 308b218487
16 changed files with 86 additions and 125 deletions
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@ -5,6 +5,18 @@ All notable changes to this project will be documented in this file.
 The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).

+## Unreleased
+
+### Fixed
+
+- Fixed an issue with the performance in the portfolio summary tab on the home page (impersonation mode)
+- Fixed various values in the impersonation mode which have not been nullified
+
+### Removed
+
+- Removed the current net performance
+- Removed the read foreign portfolio permission
+
 ## 1.38.0 - 14.08.2021

 ### Added
--- a/apps/api/src/app/account/account.controller.ts
+++ b/apps/api/src/app/account/account.controller.ts
@ -84,25 +84,19 @@ export class AccountController {
  public async getAllAccounts(
    @Headers('impersonation-id') impersonationId
  ): Promise<AccountModel[]> {
-    const impersonationUserId = await this.impersonationService.validateImpersonationId(
+    const impersonationUserId =
+      await this.impersonationService.validateImpersonationId(
        impersonationId,
        this.request.user.id
      );

-    let accounts = await this.accountService.accounts({
-      include: { Order: true, Platform: true },
-      orderBy: { name: 'asc' },
-      where: { userId: impersonationUserId || this.request.user.id }
-    });
+    let accounts = await this.accountService.getAccounts(
+      impersonationUserId || this.request.user.id
+    );

-    if (
-      impersonationUserId &&
-      !hasPermission(
-        getPermissions(this.request.user.role),
-        permissions.readForeignPortfolio
-      )
-    ) {
+    if (impersonationUserId) {
      accounts = nullifyValuesInObjects(accounts, [
+        'balance',
        'fee',
        'quantity',
        'unitPrice'
--- a/apps/api/src/app/account/account.service.ts
+++ b/apps/api/src/app/account/account.service.ts
@ -1,7 +1,7 @@
 import { ExchangeRateDataService } from '@ghostfolio/api/services/exchange-rate-data.service';
 import { PrismaService } from '@ghostfolio/api/services/prisma.service';
 import { Injectable } from '@nestjs/common';
-import { Account, Currency, Order, Prisma } from '@prisma/client';
+import { Account, Currency, Order, Platform, Prisma } from '@prisma/client';

 import { CashDetails } from './interfaces/cash-details.interface';

@ -41,7 +41,12 @@ export class AccountService {
    cursor?: Prisma.AccountWhereUniqueInput;
    where?: Prisma.AccountWhereInput;
    orderBy?: Prisma.AccountOrderByInput;
-  }): Promise<Account[]> {
+  }): Promise<
+    (Account & {
+      Order?: Order[];
+      Platform?: Platform;
+    })[]
+  > {
    const { include, skip, take, cursor, where, orderBy } = params;

    return this.prismaService.account.findMany({
@ -72,6 +77,22 @@ export class AccountService {
    });
  }

+  public async getAccounts(aUserId: string) {
+    const accounts = await this.accounts({
+      include: { Order: true, Platform: true },
+      orderBy: { name: 'asc' },
+      where: { userId: aUserId }
+    });
+
+    return accounts.map((account) => {
+      const result = { ...account, transactionCount: account.Order.length };
+
+      delete result.Order;
+
+      return result;
+    });
+  }
+
  public async getCashDetails(
    aUserId: string,
    aCurrency: Currency
--- a/apps/api/src/app/order/order.controller.ts
+++ b/apps/api/src/app/order/order.controller.ts
@ -88,13 +88,7 @@ export class OrderController {
      where: { userId: impersonationUserId || this.request.user.id }
    });

-    if (
-      impersonationUserId &&
-      !hasPermission(
-        getPermissions(this.request.user.role),
-        permissions.readForeignPortfolio
-      )
-    ) {
+    if (impersonationUserId) {
      orders = nullifyValuesInObjects(orders, ['fee', 'quantity', 'unitPrice']);
    }

--- a/apps/api/src/app/portfolio/portfolio.controller.ts
+++ b/apps/api/src/app/portfolio/portfolio.controller.ts
@ -11,11 +11,6 @@ import {
  PortfolioSummary
 } from '@ghostfolio/common/interfaces';
 import { InvestmentItem } from '@ghostfolio/common/interfaces/investment-item.interface';
-import {
-  getPermissions,
-  hasPermission,
-  permissions
-} from '@ghostfolio/common/permissions';
 import { RequestWithUser } from '@ghostfolio/common/types';
 import {
  Controller,
@ -30,7 +25,6 @@ import {
 } from '@nestjs/common';
 import { REQUEST } from '@nestjs/core';
 import { AuthGuard } from '@nestjs/passport';
-import Big from 'big.js';
 import { Response } from 'express';
 import { StatusCodes, getReasonPhrase } from 'http-status-codes';

@ -50,7 +44,7 @@ export class PortfolioController {
    @Inject(REQUEST) private readonly request: RequestWithUser
  ) {}

-  @Get('/investments')
+  @Get('investments')
  @UseGuards(AuthGuard('jwt'))
  public async findAll(
    @Headers('impersonation-id') impersonationId
@ -59,13 +53,7 @@ export class PortfolioController {
      impersonationId
    );

-    if (
-      impersonationId &&
-      !hasPermission(
-        getPermissions(this.request.user.role),
-        permissions.readForeignPortfolio
-      )
-    ) {
+    if (impersonationId) {
      const maxInvestment = investments.reduce(
        (investment, item) => Math.max(investment, item.investment),
        1
@ -104,13 +92,7 @@ export class PortfolioController {
      res.status(StatusCodes.ACCEPTED);
    }

-    if (
-      impersonationId &&
-      !hasPermission(
-        getPermissions(this.request.user.role),
-        permissions.readForeignPortfolio
-      )
-    ) {
+    if (impersonationId) {
      let maxValue = 0;

      chartData.forEach((portfolioItem) => {
@ -139,17 +121,8 @@ export class PortfolioController {
  ): Promise<{ [symbol: string]: PortfolioPosition }> {
    let details: { [symbol: string]: PortfolioPosition } = {};

-    const impersonationUserId =
-      await this.impersonationService.validateImpersonationId(
-        impersonationId,
-        this.request.user.id
-      );
-
    try {
-      details = await this.portfolioService.getDetails(
-        impersonationUserId,
-        range
-      );
+      details = await this.portfolioService.getDetails(impersonationId, range);
    } catch (error) {
      console.error(error);

@ -160,13 +133,7 @@ export class PortfolioController {
      res.status(StatusCodes.ACCEPTED);
    }

-    if (
-      impersonationId &&
-      !hasPermission(
-        getPermissions(this.request.user.role),
-        permissions.readForeignPortfolio
-      )
-    ) {
+    if (impersonationId) {
      const totalInvestment = Object.values(details)
        .map((portfolioPosition) => {
          return portfolioPosition.investment;
@ -220,16 +187,9 @@ export class PortfolioController {
    }

    let performance = performanceInformation.performance;
-    if (
-      impersonationId &&
-      !hasPermission(
-        getPermissions(this.request.user.role),
-        permissions.readForeignPortfolio
-      )
-    ) {
+    if (impersonationId) {
      performance = nullifyValuesInObject(performance, [
        'currentGrossPerformance',
-        'currentNetPerformance',
        'currentValue'
      ]);
    }
@ -253,6 +213,16 @@ export class PortfolioController {
      res.status(StatusCodes.ACCEPTED);
    }

+    if (impersonationId) {
+      result.positions = result.positions.map((position) => {
+        return nullifyValuesInObject(position, [
+          'grossPerformance',
+          'investment',
+          'quantity'
+        ]);
+      });
+    }
+
    return <any>res.json(result);
  }

@ -263,20 +233,14 @@ export class PortfolioController {
  ): Promise<PortfolioSummary> {
    let summary = await this.portfolioService.getSummary(impersonationId);

-    if (
-      impersonationId &&
-      !hasPermission(
-        getPermissions(this.request.user.role),
-        permissions.readForeignPortfolio
-      )
-    ) {
+    if (impersonationId) {
      summary = nullifyValuesInObject(summary, [
        'cash',
        'committedFunds',
        'currentGrossPerformance',
-        'currentNetPerformance',
        'currentValue',
        'fees',
+        'netWorth',
        'totalBuy',
        'totalSell'
      ]);
@ -297,14 +261,12 @@ export class PortfolioController {
    );

    if (position) {
-      if (
-        impersonationId &&
-        !hasPermission(
-          getPermissions(this.request.user.role),
-          permissions.readForeignPortfolio
-        )
-      ) {
-        position = nullifyValuesInObject(position, ['grossPerformance']);
+      if (impersonationId) {
+        position = nullifyValuesInObject(position, [
+          'grossPerformance',
+          'investment',
+          'quantity'
+        ]);
      }

      return position;
--- a/apps/api/src/app/portfolio/portfolio.service.ts
+++ b/apps/api/src/app/portfolio/portfolio.service.ts
@ -530,8 +530,6 @@ export class PortfolioService {
        performance: {
          currentGrossPerformance: 0,
          currentGrossPerformancePercent: 0,
-          currentNetPerformance: 0,
-          currentNetPerformancePercent: 0,
          currentValue: 0
        }
      };
@ -556,9 +554,6 @@ export class PortfolioService {
      performance: {
        currentGrossPerformance,
        currentGrossPerformancePercent,
-        // TODO: the next two should include fees
-        currentNetPerformance: currentGrossPerformance,
-        currentNetPerformancePercent: currentGrossPerformancePercent,
        currentValue: currentValue
      }
    };
@ -668,7 +663,7 @@ export class PortfolioService {
    const currency = this.request.user.Settings.currency;
    const userId = await this.getUserId(aImpersonationId);

-    const performanceInformation = await this.getPerformance(userId);
+    const performanceInformation = await this.getPerformance(aImpersonationId);

    const { balance } = await this.accountService.getCashDetails(
      userId,
--- a/apps/client/src/app/components/accounts-table/accounts-table.component.html
+++ b/apps/client/src/app/components/accounts-table/accounts-table.component.html
@ -8,7 +8,7 @@
        [tooltip]="element.Platform?.name"
        [url]="element.Platform?.url"
      ></gf-symbol-icon>
-      <span>{{ element.name }}</span>
+      <span>{{ element.name }} </span>
      <span
        *ngIf="element.isDefault"
        class="d-lg-inline-block d-none text-muted"
@ -45,7 +45,7 @@
      <span class="d-none d-sm-block" i18n>Transactions</span>
    </th>
    <td *matCellDef="let element" class="px-1 text-right" mat-cell>
-      {{ element.Order?.length }}
+      {{ element.transactionCount }}
    </td>
  </ng-container>

--- a/apps/client/src/app/components/portfolio-performance/portfolio-performance.component.html
+++ b/apps/client/src/app/components/portfolio-performance/portfolio-performance.component.html
@ -37,7 +37,7 @@
        [colorizeSign]="true"
        [isCurrency]="true"
        [locale]="locale"
-        [value]="isLoading ? undefined : performance?.currentNetPerformance"
+        [value]="isLoading ? undefined : performance?.currentGrossPerformance"
      ></gf-value>
    </div>
    <div class="col">
@ -46,7 +46,7 @@
        [isPercent]="true"
        [locale]="locale"
        [value]="
-          isLoading ? undefined : performance?.currentNetPerformancePercent
+          isLoading ? undefined : performance?.currentGrossPerformancePercent
        "
      ></gf-value>
    </div>
--- a/apps/client/src/app/components/portfolio-performance/portfolio-performance.component.ts
+++ b/apps/client/src/app/components/portfolio-performance/portfolio-performance.component.ts
@ -52,7 +52,7 @@ export class PortfolioPerformanceComponent implements OnChanges, OnInit {

        new CountUp(
          'value',
-          this.performance?.currentNetPerformancePercent * 100,
+          this.performance?.currentGrossPerformancePercent * 100,
          {
            decimalPlaces: 2,
            duration: 0.75,
--- a/apps/client/src/app/components/value/value.component.html
+++ b/apps/client/src/app/components/value/value.component.html
@ -1,16 +1,21 @@
-<ng-container *ngIf="value || value === 0">
+<ng-container *ngIf="value || value === 0 || value === null">
  <div
    class="d-flex"
    [ngClass]="position === 'end' ? 'justify-content-end' : ''"
  >
-    <ng-container *ngIf="isNumber">
+    <ng-container *ngIf="isNumber || value === null">
      <div *ngIf="colorizeSign && value > 0" class="mr-1 text-success">+</div>
      <div *ngIf="colorizeSign && value < 0" class="mr-1 text-danger">-</div>
      <div *ngIf="isPercent" [ngClass]="size === 'medium' ? 'h4 mb-0' : ''">
        {{ formattedValue }}%
      </div>
      <div *ngIf="!isPercent" [ngClass]="size === 'medium' ? 'h4 mb-0' : ''">
+        <ng-container *ngIf="value === null">
+          <span class="text-monospace text-muted">***</span>
+        </ng-container>
+        <ng-container *ngIf="value !== null">
          {{ formattedValue }}
+        </ng-container>
      </div>
      <small *ngIf="currency && size === 'medium'" class="ml-1">
        {{ currency }}
@ -38,8 +43,3 @@
    width: '5rem'
  }"
 ></ngx-skeleton-loader>
-
-<div *ngIf="value === null">
-  <span class="text-monospace text-muted text-right">***</span>
-  <span *ngIf="currency" class="ml-1 text-muted">{{ currency }}</span>
-</div>
--- a/apps/client/src/app/pages/home/home-page.component.ts
+++ b/apps/client/src/app/pages/home/home-page.component.ts
@ -58,7 +58,6 @@ export class HomePageComponent implements OnDestroy, OnInit {
  public fearAndGreedIndex: number;
  public hasImpersonationId: boolean;
  public hasPermissionToAccessFearAndGreedIndex: boolean;
-  public hasPermissionToReadForeignPortfolio: boolean;
  public hasPositions: boolean;
  public historicalDataItems: LineChartItem[];
  public isLoadingPerformance = true;
@ -120,11 +119,6 @@ export class HomePageComponent implements OnDestroy, OnInit {
              });
          }

-          this.hasPermissionToReadForeignPortfolio = hasPermission(
-            this.user.permissions,
-            permissions.readForeignPortfolio
-          );
-
          this.changeDetectorRef.markForCheck();
        }
      });
--- a/apps/client/src/app/pages/home/home-page.html
+++ b/apps/client/src/app/pages/home/home-page.html
@ -61,7 +61,7 @@
            [isLoading]="isLoadingPerformance"
            [locale]="user?.settings?.locale"
            [performance]="performance"
-            [showDetails]="!hasImpersonationId || hasPermissionToReadForeignPortfolio"
+            [showDetails]="!hasImpersonationId"
          ></gf-portfolio-performance>
          <div class="text-center">
            <gf-toggle
--- a/apps/client/src/app/pages/zen/zen-page.component.ts
+++ b/apps/client/src/app/pages/zen/zen-page.component.ts
@ -19,7 +19,6 @@ import {
  Position,
  User
 } from '@ghostfolio/common/interfaces';
-import { hasPermission, permissions } from '@ghostfolio/common/permissions';
 import { DateRange } from '@ghostfolio/common/types';
 import { DeviceDetectorService } from 'ngx-device-detector';
 import { Subject } from 'rxjs';
@ -37,7 +36,6 @@ export class ZenPageComponent implements AfterViewInit, OnDestroy, OnInit {
  public dateRange: DateRange = 'max';
  public deviceType: string;
  public hasImpersonationId: boolean;
-  public hasPermissionToReadForeignPortfolio: boolean;
  public hasPositions: boolean;
  public historicalDataItems: LineChartItem[];
  public isLoadingPerformance = true;
@ -65,11 +63,6 @@ export class ZenPageComponent implements AfterViewInit, OnDestroy, OnInit {
        if (state?.user) {
          this.user = state.user;

-          this.hasPermissionToReadForeignPortfolio = hasPermission(
-            this.user.permissions,
-            permissions.readForeignPortfolio
-          );
-
          this.changeDetectorRef.markForCheck();
        }
      });
--- a/apps/client/src/app/pages/zen/zen-page.html
+++ b/apps/client/src/app/pages/zen/zen-page.html
@ -49,7 +49,7 @@
            [isLoading]="isLoadingPerformance"
            [locale]="user?.settings?.locale"
            [performance]="performance"
-            [showDetails]="!hasImpersonationId || hasPermissionToReadForeignPortfolio"
+            [showDetails]="!hasImpersonationId"
          ></gf-portfolio-performance>
        </div>
      </div>
--- a/libs/common/src/lib/interfaces/portfolio-performance.interface.ts
+++ b/libs/common/src/lib/interfaces/portfolio-performance.interface.ts
@ -1,7 +1,5 @@
 export interface PortfolioPerformance {
  currentGrossPerformance: number;
  currentGrossPerformancePercent: number;
-  currentNetPerformance: number;
-  currentNetPerformancePercent: number;
  currentValue: number;
 }
--- a/libs/common/src/lib/permissions.ts
+++ b/libs/common/src/lib/permissions.ts
@ -19,7 +19,6 @@ export const permissions = {
  enableSocialLogin: 'enableSocialLogin',
  enableStatistics: 'enableStatistics',
  enableSubscription: 'enableSubscription',
-  readForeignPortfolio: 'readForeignPortfolio',
  updateAccount: 'updateAccount',
  updateAuthDevice: 'updateAuthDevice',
  updateOrder: 'updateOrder',
@ -45,7 +44,6 @@ export function getPermissions(aRole: Role): string[] {
        permissions.deleteAuthDevice,
        permissions.deleteOrder,
        permissions.deleteUser,
-        permissions.readForeignPortfolio,
        permissions.updateAccount,
        permissions.updateAuthDevice,
        permissions.updateOrder,