From 0ee2258af8353c02428b0a0216406d2e1d20abac Mon Sep 17 00:00:00 2001 From: Thomas Kaul <4159106+dtslvr@users.noreply.github.com> Date: Sat, 14 Aug 2021 19:15:26 +0200 Subject: [PATCH] Feature/improve impersonation mode (#293) * Improve the impersonation mode * Update changelog --- CHANGELOG.md | 12 +++ .../api/src/app/account/account.controller.ts | 26 +++--- apps/api/src/app/account/account.service.ts | 25 +++++- apps/api/src/app/order/order.controller.ts | 8 +- .../src/app/portfolio/portfolio.controller.ts | 86 ++++++------------- .../src/app/portfolio/portfolio.service.ts | 7 +- .../accounts-table.component.html | 4 +- .../portfolio-performance.component.html | 4 +- .../portfolio-performance.component.ts | 2 +- .../app/components/value/value.component.html | 16 ++-- .../src/app/pages/home/home-page.component.ts | 6 -- apps/client/src/app/pages/home/home-page.html | 2 +- .../src/app/pages/zen/zen-page.component.ts | 7 -- apps/client/src/app/pages/zen/zen-page.html | 2 +- .../portfolio-performance.interface.ts | 2 - libs/common/src/lib/permissions.ts | 2 - 16 files changed, 86 insertions(+), 125 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8290114d7..da1f36a78 100644 --- 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 diff --git a/apps/api/src/app/account/account.controller.ts b/apps/api/src/app/account/account.controller.ts index 3757d216d..3a3604c69 100644 --- 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 { - const impersonationUserId = await this.impersonationService.validateImpersonationId( - impersonationId, - this.request.user.id - ); + 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' diff --git a/apps/api/src/app/account/account.service.ts b/apps/api/src/app/account/account.service.ts index f976ef6a6..011470b4b 100644 --- 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 { + }): 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 diff --git a/apps/api/src/app/order/order.controller.ts b/apps/api/src/app/order/order.controller.ts index d54bbcee2..535c7218d 100644 --- 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']); } diff --git a/apps/api/src/app/portfolio/portfolio.controller.ts b/apps/api/src/app/portfolio/portfolio.controller.ts index 58d6edd47..b97b016c8 100644 --- 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 res.json(result); } @@ -263,20 +233,14 @@ export class PortfolioController { ): Promise { 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; diff --git a/apps/api/src/app/portfolio/portfolio.service.ts b/apps/api/src/app/portfolio/portfolio.service.ts index 8e5e50aec..b21b1112a 100644 --- 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, diff --git a/apps/client/src/app/components/accounts-table/accounts-table.component.html b/apps/client/src/app/components/accounts-table/accounts-table.component.html index 1eafb997f..9521652f9 100644 --- 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" > - {{ element.name }} + {{ element.name }} Transactions - {{ element.Order?.length }} + {{ element.transactionCount }} diff --git a/apps/client/src/app/components/portfolio-performance/portfolio-performance.component.html b/apps/client/src/app/components/portfolio-performance/portfolio-performance.component.html index 42aa5a683..4fb8c85fb 100644 --- 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" >
@@ -46,7 +46,7 @@ [isPercent]="true" [locale]="locale" [value]=" - isLoading ? undefined : performance?.currentNetPerformancePercent + isLoading ? undefined : performance?.currentGrossPerformancePercent " >
diff --git a/apps/client/src/app/components/portfolio-performance/portfolio-performance.component.ts b/apps/client/src/app/components/portfolio-performance/portfolio-performance.component.ts index 90d134d45..0bb3608b5 100644 --- 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, diff --git a/apps/client/src/app/components/value/value.component.html b/apps/client/src/app/components/value/value.component.html index 67482b78e..daa585adb 100644 --- a/apps/client/src/app/components/value/value.component.html +++ b/apps/client/src/app/components/value/value.component.html @@ -1,16 +1,21 @@ - +
- +
+
-
{{ formattedValue }}%
- {{ formattedValue }} + + *** + + + {{ formattedValue }} +
{{ currency }} @@ -38,8 +43,3 @@ width: '5rem' }" > - -
- *** - {{ currency }} -
diff --git a/apps/client/src/app/pages/home/home-page.component.ts b/apps/client/src/app/pages/home/home-page.component.ts index 035794d6b..7da83c066 100644 --- 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(); } }); diff --git a/apps/client/src/app/pages/home/home-page.html b/apps/client/src/app/pages/home/home-page.html index cebf1f072..77dd2c94e 100644 --- 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" >
diff --git a/libs/common/src/lib/interfaces/portfolio-performance.interface.ts b/libs/common/src/lib/interfaces/portfolio-performance.interface.ts index 2051be7fd..7fb176233 100644 --- 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; } diff --git a/libs/common/src/lib/permissions.ts b/libs/common/src/lib/permissions.ts index e8f7824d8..f01103127 100644 --- 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,