From 93dcbeb6c7cce0a394b191b449e6af681882200a Mon Sep 17 00:00:00 2001
From: Thomas Kaul <4159106+dtslvr@users.noreply.github.com>
Date: Tue, 12 Oct 2021 22:19:32 +0200
Subject: [PATCH] Feature/add validation for import (#415)

* Valid data types
* Maximum number of orders
* Data provider service returns data for the dataSource / symbol pair
---
 CHANGELOG.md                                  |  7 ++++
 apps/api/src/app/import/import-data.dto.ts    |  8 +++--
 apps/api/src/app/import/import.controller.ts  |  5 ++-
 apps/api/src/app/import/import.service.ts     | 32 ++++++++++++++++-
 apps/api/src/app/order/create-order.dto.ts    |  6 ++--
 .../src/app/portfolio/current-rate.service.ts |  4 +--
 .../src/app/core/http-response.interceptor.ts |  2 +-
 ...ate-or-update-transaction-dialog.module.ts |  2 +-
 .../interfaces/interfaces.ts                  |  2 +-
 .../import-transaction-dialog.component.ts    | 36 +++++++++++++++++++
 .../import-transaction-dialog.html            | 23 ++++++++++++
 .../import-transaction-dialog.module.ts       | 23 ++++++++++++
 .../import-transaction-dialog.scss            |  3 ++
 .../interfaces/interfaces.ts                  |  4 +++
 .../transactions-page.component.ts            | 31 ++++++++++------
 .../transactions/transactions-page.module.ts  |  6 ++--
 apps/client/src/app/services/data.service.ts  |  7 +---
 17 files changed, 171 insertions(+), 30 deletions(-)
 create mode 100644 apps/client/src/app/pages/portfolio/transactions/import-transaction-dialog/import-transaction-dialog.component.ts
 create mode 100644 apps/client/src/app/pages/portfolio/transactions/import-transaction-dialog/import-transaction-dialog.html
 create mode 100644 apps/client/src/app/pages/portfolio/transactions/import-transaction-dialog/import-transaction-dialog.module.ts
 create mode 100644 apps/client/src/app/pages/portfolio/transactions/import-transaction-dialog/import-transaction-dialog.scss
 create mode 100644 apps/client/src/app/pages/portfolio/transactions/import-transaction-dialog/interfaces/interfaces.ts

diff --git a/CHANGELOG.md b/CHANGELOG.md
index c0c3fef08..94ce70534 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -7,6 +7,13 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## Unreleased
 
+### Added
+
+- Extended the validation of the import functionality for transactions
+  - Valid data types
+  - Maximum number of orders
+  - Data provider service returns data for the `dataSource` / `symbol` pair
+
 ### Fixed
 
 - Fixed the broken line charts showing value labels
diff --git a/apps/api/src/app/import/import-data.dto.ts b/apps/api/src/app/import/import-data.dto.ts
index c53cde073..fa1b3aa99 100644
--- a/apps/api/src/app/import/import-data.dto.ts
+++ b/apps/api/src/app/import/import-data.dto.ts
@@ -1,7 +1,11 @@
+import { CreateOrderDto } from '@ghostfolio/api/app/order/create-order.dto';
 import { Order } from '@prisma/client';
-import { IsArray } from 'class-validator';
+import { Type } from 'class-transformer';
+import { IsArray, ValidateNested } from 'class-validator';
 
 export class ImportDataDto {
   @IsArray()
-  orders: Partial<Order>[];
+  @Type(() => CreateOrderDto)
+  @ValidateNested({ each: true })
+  orders: Order[];
 }
diff --git a/apps/api/src/app/import/import.controller.ts b/apps/api/src/app/import/import.controller.ts
index c453f7837..a16682957 100644
--- a/apps/api/src/app/import/import.controller.ts
+++ b/apps/api/src/app/import/import.controller.ts
@@ -42,7 +42,10 @@ export class ImportController {
       console.error(error);
 
       throw new HttpException(
-        getReasonPhrase(StatusCodes.BAD_REQUEST),
+        {
+          error: getReasonPhrase(StatusCodes.BAD_REQUEST),
+          message: [error.message]
+        },
         StatusCodes.BAD_REQUEST
       );
     }
diff --git a/apps/api/src/app/import/import.service.ts b/apps/api/src/app/import/import.service.ts
index 91577b263..8af2559f0 100644
--- a/apps/api/src/app/import/import.service.ts
+++ b/apps/api/src/app/import/import.service.ts
@@ -1,11 +1,17 @@
 import { OrderService } from '@ghostfolio/api/app/order/order.service';
+import { DataProviderService } from '@ghostfolio/api/services/data-provider/data-provider.service';
 import { Injectable } from '@nestjs/common';
 import { Order } from '@prisma/client';
 import { parseISO } from 'date-fns';
 
 @Injectable()
 export class ImportService {
-  public constructor(private readonly orderService: OrderService) {}
+  private static MAX_ORDERS_TO_IMPORT = 20;
+
+  public constructor(
+    private readonly dataProviderService: DataProviderService,
+    private readonly orderService: OrderService
+  ) {}
 
   public async import({
     orders,
@@ -14,7 +20,10 @@ export class ImportService {
     orders: Partial<Order>[];
     userId: string;
   }): Promise<void> {
+    await this.validateOrders(orders);
+
     for (const {
+      accountId,
       currency,
       dataSource,
       date,
@@ -25,6 +34,11 @@ export class ImportService {
       unitPrice
     } of orders) {
       await this.orderService.createOrder({
+        Account: {
+          connect: {
+            id_userId: { userId, id: accountId }
+          }
+        },
         currency,
         dataSource,
         fee,
@@ -37,4 +51,20 @@ export class ImportService {
       });
     }
   }
+
+  private async validateOrders(orders: Partial<Order>[]) {
+    if (orders?.length > ImportService.MAX_ORDERS_TO_IMPORT) {
+      throw new Error('Too many transactions');
+    }
+
+    for (const { dataSource, symbol } of orders) {
+      const result = await this.dataProviderService.get([
+        { dataSource, symbol }
+      ]);
+
+      if (result[symbol] === undefined) {
+        throw new Error(`${symbol} is not a valid symbol for ${dataSource}`);
+      }
+    }
+  }
 }
diff --git a/apps/api/src/app/order/create-order.dto.ts b/apps/api/src/app/order/create-order.dto.ts
index 942915843..bd52578e2 100644
--- a/apps/api/src/app/order/create-order.dto.ts
+++ b/apps/api/src/app/order/create-order.dto.ts
@@ -1,5 +1,5 @@
 import { DataSource, Type } from '@prisma/client';
-import { IsISO8601, IsNumber, IsString } from 'class-validator';
+import { IsEnum, IsISO8601, IsNumber, IsString } from 'class-validator';
 
 export class CreateOrderDto {
   @IsString()
@@ -8,7 +8,7 @@ export class CreateOrderDto {
   @IsString()
   currency: string;
 
-  @IsString()
+  @IsEnum(DataSource, { each: true })
   dataSource: DataSource;
 
   @IsISO8601()
@@ -23,7 +23,7 @@ export class CreateOrderDto {
   @IsString()
   symbol: string;
 
-  @IsString()
+  @IsEnum(Type, { each: true })
   type: Type;
 
   @IsNumber()
diff --git a/apps/api/src/app/portfolio/current-rate.service.ts b/apps/api/src/app/portfolio/current-rate.service.ts
index f52189a6d..7f2813c6e 100644
--- a/apps/api/src/app/portfolio/current-rate.service.ts
+++ b/apps/api/src/app/portfolio/current-rate.service.ts
@@ -30,9 +30,9 @@ export class CurrentRateService {
         { symbol, dataSource: DataSource.YAHOO }
       ]);
       return {
+        symbol,
         date: resetHours(date),
-        marketPrice: dataProviderResult?.[symbol]?.marketPrice ?? 0,
-        symbol: symbol
+        marketPrice: dataProviderResult?.[symbol]?.marketPrice ?? 0
       };
     }
 
diff --git a/apps/client/src/app/core/http-response.interceptor.ts b/apps/client/src/app/core/http-response.interceptor.ts
index ed9e7110c..afed17f4e 100644
--- a/apps/client/src/app/core/http-response.interceptor.ts
+++ b/apps/client/src/app/core/http-response.interceptor.ts
@@ -101,7 +101,7 @@ export class HttpResponseInterceptor implements HttpInterceptor {
           }
         }
 
-        return throwError('');
+        return throwError(error);
       })
     );
   }
diff --git a/apps/client/src/app/pages/portfolio/transactions/create-or-update-transaction-dialog/create-or-update-transaction-dialog.module.ts b/apps/client/src/app/pages/portfolio/transactions/create-or-update-transaction-dialog/create-or-update-transaction-dialog.module.ts
index 3dd0af784..94007bee9 100644
--- a/apps/client/src/app/pages/portfolio/transactions/create-or-update-transaction-dialog/create-or-update-transaction-dialog.module.ts
+++ b/apps/client/src/app/pages/portfolio/transactions/create-or-update-transaction-dialog/create-or-update-transaction-dialog.module.ts
@@ -35,4 +35,4 @@ import { CreateOrUpdateTransactionDialog } from './create-or-update-transaction-
   providers: [],
   schemas: [CUSTOM_ELEMENTS_SCHEMA]
 })
-export class CreateOrUpdateTransactionDialogModule {}
+export class GfCreateOrUpdateTransactionDialogModule {}
diff --git a/apps/client/src/app/pages/portfolio/transactions/create-or-update-transaction-dialog/interfaces/interfaces.ts b/apps/client/src/app/pages/portfolio/transactions/create-or-update-transaction-dialog/interfaces/interfaces.ts
index 291b488cb..ef0eee74c 100644
--- a/apps/client/src/app/pages/portfolio/transactions/create-or-update-transaction-dialog/interfaces/interfaces.ts
+++ b/apps/client/src/app/pages/portfolio/transactions/create-or-update-transaction-dialog/interfaces/interfaces.ts
@@ -1,5 +1,5 @@
 import { User } from '@ghostfolio/common/interfaces';
-import { Account, Order } from '@prisma/client';
+import { Order } from '@prisma/client';
 
 export interface CreateOrUpdateTransactionDialogParams {
   accountId: string;
diff --git a/apps/client/src/app/pages/portfolio/transactions/import-transaction-dialog/import-transaction-dialog.component.ts b/apps/client/src/app/pages/portfolio/transactions/import-transaction-dialog/import-transaction-dialog.component.ts
new file mode 100644
index 000000000..f5ccf8f26
--- /dev/null
+++ b/apps/client/src/app/pages/portfolio/transactions/import-transaction-dialog/import-transaction-dialog.component.ts
@@ -0,0 +1,36 @@
+import {
+  ChangeDetectionStrategy,
+  Component,
+  Inject,
+  OnDestroy
+} from '@angular/core';
+import { MAT_DIALOG_DATA, MatDialogRef } from '@angular/material/dialog';
+import { Subject } from 'rxjs';
+
+import { ImportTransactionDialogParams } from './interfaces/interfaces';
+
+@Component({
+  changeDetection: ChangeDetectionStrategy.OnPush,
+  selector: 'gf-import-transaction-dialog',
+  styleUrls: ['./import-transaction-dialog.scss'],
+  templateUrl: 'import-transaction-dialog.html'
+})
+export class ImportTransactionDialog implements OnDestroy {
+  private unsubscribeSubject = new Subject<void>();
+
+  public constructor(
+    @Inject(MAT_DIALOG_DATA) public data: ImportTransactionDialogParams,
+    public dialogRef: MatDialogRef<ImportTransactionDialog>
+  ) {}
+
+  public ngOnInit() {}
+
+  public onCancel(): void {
+    this.dialogRef.close();
+  }
+
+  public ngOnDestroy() {
+    this.unsubscribeSubject.next();
+    this.unsubscribeSubject.complete();
+  }
+}
diff --git a/apps/client/src/app/pages/portfolio/transactions/import-transaction-dialog/import-transaction-dialog.html b/apps/client/src/app/pages/portfolio/transactions/import-transaction-dialog/import-transaction-dialog.html
new file mode 100644
index 000000000..28e2a7f50
--- /dev/null
+++ b/apps/client/src/app/pages/portfolio/transactions/import-transaction-dialog/import-transaction-dialog.html
@@ -0,0 +1,23 @@
+<gf-dialog-header
+  mat-dialog-title
+  title="Import Transactions Error"
+  [deviceType]="data.deviceType"
+  (closeButtonClicked)="onCancel()"
+></gf-dialog-header>
+
+<div class="flex-grow-1" mat-dialog-content>
+  <ul class="list-unstyled">
+    <li *ngFor="let message of data.messages" class="d-flex">
+      <div class="align-items-center d-flex px-2">
+        <ion-icon name="warning-outline"></ion-icon>
+      </div>
+      <div>{{ message }}</div>
+    </li>
+  </ul>
+</div>
+
+<gf-dialog-footer
+  mat-dialog-actions
+  [deviceType]="data.deviceType"
+  (closeButtonClicked)="onCancel()"
+></gf-dialog-footer>
diff --git a/apps/client/src/app/pages/portfolio/transactions/import-transaction-dialog/import-transaction-dialog.module.ts b/apps/client/src/app/pages/portfolio/transactions/import-transaction-dialog/import-transaction-dialog.module.ts
new file mode 100644
index 000000000..b36158e25
--- /dev/null
+++ b/apps/client/src/app/pages/portfolio/transactions/import-transaction-dialog/import-transaction-dialog.module.ts
@@ -0,0 +1,23 @@
+import { CommonModule } from '@angular/common';
+import { CUSTOM_ELEMENTS_SCHEMA, NgModule } from '@angular/core';
+import { MatButtonModule } from '@angular/material/button';
+import { MatDialogModule } from '@angular/material/dialog';
+import { GfDialogFooterModule } from '@ghostfolio/client/components/dialog-footer/dialog-footer.module';
+import { GfDialogHeaderModule } from '@ghostfolio/client/components/dialog-header/dialog-header.module';
+
+import { ImportTransactionDialog } from './import-transaction-dialog.component';
+
+@NgModule({
+  declarations: [ImportTransactionDialog],
+  exports: [],
+  imports: [
+    CommonModule,
+    GfDialogFooterModule,
+    GfDialogHeaderModule,
+    MatButtonModule,
+    MatDialogModule
+  ],
+  providers: [],
+  schemas: [CUSTOM_ELEMENTS_SCHEMA]
+})
+export class GfImportTransactionDialogModule {}
diff --git a/apps/client/src/app/pages/portfolio/transactions/import-transaction-dialog/import-transaction-dialog.scss b/apps/client/src/app/pages/portfolio/transactions/import-transaction-dialog/import-transaction-dialog.scss
new file mode 100644
index 000000000..5d4e87f30
--- /dev/null
+++ b/apps/client/src/app/pages/portfolio/transactions/import-transaction-dialog/import-transaction-dialog.scss
@@ -0,0 +1,3 @@
+:host {
+  display: block;
+}
diff --git a/apps/client/src/app/pages/portfolio/transactions/import-transaction-dialog/interfaces/interfaces.ts b/apps/client/src/app/pages/portfolio/transactions/import-transaction-dialog/interfaces/interfaces.ts
new file mode 100644
index 000000000..4041054eb
--- /dev/null
+++ b/apps/client/src/app/pages/portfolio/transactions/import-transaction-dialog/interfaces/interfaces.ts
@@ -0,0 +1,4 @@
+export interface ImportTransactionDialogParams {
+  deviceType: string;
+  messages: string[];
+}
diff --git a/apps/client/src/app/pages/portfolio/transactions/transactions-page.component.ts b/apps/client/src/app/pages/portfolio/transactions/transactions-page.component.ts
index 39eefdd32..f6cda59c0 100644
--- a/apps/client/src/app/pages/portfolio/transactions/transactions-page.component.ts
+++ b/apps/client/src/app/pages/portfolio/transactions/transactions-page.component.ts
@@ -16,6 +16,7 @@ import { EMPTY, Subject, Subscription } from 'rxjs';
 import { catchError, takeUntil } from 'rxjs/operators';
 
 import { CreateOrUpdateTransactionDialog } from './create-or-update-transaction-dialog/create-or-update-transaction-dialog.component';
+import { ImportTransactionDialog } from './import-transaction-dialog/import-transaction-dialog.component';
 
 @Component({
   selector: 'gf-transactions-page',
@@ -23,6 +24,7 @@ import { CreateOrUpdateTransactionDialog } from './create-or-update-transaction-
   styleUrls: ['./transactions-page.scss']
 })
 export class TransactionsPageComponent implements OnDestroy, OnInit {
+  public defaultAccountId: string;
   public deviceType: string;
   public hasImpersonationId: boolean;
   public hasPermissionToCreateOrder: boolean;
@@ -93,6 +95,10 @@ export class TransactionsPageComponent implements OnDestroy, OnInit {
         if (state?.user) {
           this.user = state.user;
 
+          this.defaultAccountId = this.user?.accounts.find((account) => {
+            return account.isDefault;
+          })?.id;
+
           this.hasPermissionToCreateOrder = hasPermission(
             this.user.permissions,
             permissions.createOrder
@@ -175,7 +181,9 @@ export class TransactionsPageComponent implements OnDestroy, OnInit {
 
           this.dataService
             .postImport({
-              orders: content.orders
+              orders: content.orders.map((order) => {
+                return { ...order, accountId: this.defaultAccountId };
+              })
             })
             .pipe(
               catchError((error) => {
@@ -195,7 +203,7 @@ export class TransactionsPageComponent implements OnDestroy, OnInit {
               }
             });
         } catch (error) {
-          this.handleImportError(error);
+          this.handleImportError({ error: { message: ['Unexpected format'] } });
         }
       };
     };
@@ -281,20 +289,23 @@ export class TransactionsPageComponent implements OnDestroy, OnInit {
     a.click();
   }
 
-  private handleImportError(aError: unknown) {
-    console.error(aError);
-    this.snackBar.open('❌ Oops, something went wrong...');
+  private handleImportError(aError: any) {
+    this.snackBar.dismiss();
+
+    this.dialog.open(ImportTransactionDialog, {
+      data: {
+        deviceType: this.deviceType,
+        messages: aError?.error?.message
+      },
+      width: this.deviceType === 'mobile' ? '100vw' : '50rem'
+    });
   }
 
   private openCreateTransactionDialog(aTransaction?: OrderModel): void {
     const dialogRef = this.dialog.open(CreateOrUpdateTransactionDialog, {
       data: {
         transaction: {
-          accountId:
-            aTransaction?.accountId ??
-            this.user?.accounts.find((account) => {
-              return account.isDefault;
-            })?.id,
+          accountId: aTransaction?.accountId ?? this.defaultAccountId,
           currency: aTransaction?.currency ?? null,
           dataSource: aTransaction?.dataSource ?? null,
           date: new Date(),
diff --git a/apps/client/src/app/pages/portfolio/transactions/transactions-page.module.ts b/apps/client/src/app/pages/portfolio/transactions/transactions-page.module.ts
index 261eadfdb..efdac22e2 100644
--- a/apps/client/src/app/pages/portfolio/transactions/transactions-page.module.ts
+++ b/apps/client/src/app/pages/portfolio/transactions/transactions-page.module.ts
@@ -5,7 +5,8 @@ import { MatSnackBarModule } from '@angular/material/snack-bar';
 import { RouterModule } from '@angular/router';
 import { GfTransactionsTableModule } from '@ghostfolio/client/components/transactions-table/transactions-table.module';
 
-import { CreateOrUpdateTransactionDialogModule } from './create-or-update-transaction-dialog/create-or-update-transaction-dialog.module';
+import { GfCreateOrUpdateTransactionDialogModule } from './create-or-update-transaction-dialog/create-or-update-transaction-dialog.module';
+import { GfImportTransactionDialogModule } from './import-transaction-dialog/import-transaction-dialog.module';
 import { TransactionsPageRoutingModule } from './transactions-page-routing.module';
 import { TransactionsPageComponent } from './transactions-page.component';
 
@@ -14,7 +15,8 @@ import { TransactionsPageComponent } from './transactions-page.component';
   exports: [],
   imports: [
     CommonModule,
-    CreateOrUpdateTransactionDialogModule,
+    GfCreateOrUpdateTransactionDialogModule,
+    GfImportTransactionDialogModule,
     GfTransactionsTableModule,
     MatButtonModule,
     MatSnackBarModule,
diff --git a/apps/client/src/app/services/data.service.ts b/apps/client/src/app/services/data.service.ts
index 936efc888..3bac0486d 100644
--- a/apps/client/src/app/services/data.service.ts
+++ b/apps/client/src/app/services/data.service.ts
@@ -39,18 +39,13 @@ import { cloneDeep } from 'lodash';
 import { Observable } from 'rxjs';
 import { map } from 'rxjs/operators';
 
-import { SettingsStorageService } from './settings-storage.service';
-
 @Injectable({
   providedIn: 'root'
 })
 export class DataService {
   private info: InfoItem;
 
-  public constructor(
-    private http: HttpClient,
-    private settingsStorageService: SettingsStorageService
-  ) {}
+  public constructor(private http: HttpClient) {}
 
   public createCheckoutSession({
     couponId,