From 20cefaba19bd56021754494f891b3eb7d851fad5 Mon Sep 17 00:00:00 2001
From: Ajay <97356092+iajaymk@users.noreply.github.com>
Date: Sat, 28 Oct 2023 18:39:01 +0530
Subject: [PATCH] Improve usability and validation in cash balance transfer
 (#2552)

* Improve usability and validation in cash balance transfer

* Update changelog

---------

Co-authored-by: Thomas <4159106+dtslvr@users.noreply.github.com>
---
 CHANGELOG.md                                  |  6 +++
 .../api/src/app/account/account.controller.ts | 38 ++++++++++++-------
 .../accounts-table.component.html             |  1 +
 .../pages/accounts/accounts-page.component.ts | 14 +++++--
 .../transfer-balance-dialog.component.ts      | 32 +++++++++++++---
 5 files changed, 67 insertions(+), 24 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index ec59301f..c3ff6246 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -5,6 +5,12 @@ 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
+
+### Changed
+
+- Improved the usability and validation in the cash balance transfer from one to another account
+
 ## 2.15.0 - 2023-10-26
 
 ### Added
diff --git a/apps/api/src/app/account/account.controller.ts b/apps/api/src/app/account/account.controller.ts
index 4666e508..e141dc11 100644
--- a/apps/api/src/app/account/account.controller.ts
+++ b/apps/api/src/app/account/account.controller.ts
@@ -190,36 +190,46 @@ export class AccountController {
       this.request.user.id
     );
 
-    const currentAccountIds = accountsOfUser.map(({ id }) => {
-      return id;
+    const accountFrom = accountsOfUser.find(({ id }) => {
+      return id === accountIdFrom;
     });
 
-    if (
-      ![accountIdFrom, accountIdTo].every((accountId) => {
-        return currentAccountIds.includes(accountId);
-      })
-    ) {
+    const accountTo = accountsOfUser.find(({ id }) => {
+      return id === accountIdTo;
+    });
+
+    if (!accountFrom || !accountTo) {
       throw new HttpException(
         getReasonPhrase(StatusCodes.NOT_FOUND),
         StatusCodes.NOT_FOUND
       );
     }
 
-    const { currency } = accountsOfUser.find(({ id }) => {
-      return id === accountIdFrom;
-    });
+    if (accountFrom.id === accountTo.id) {
+      throw new HttpException(
+        getReasonPhrase(StatusCodes.BAD_REQUEST),
+        StatusCodes.BAD_REQUEST
+      );
+    }
+
+    if (accountFrom.balance < balance) {
+      throw new HttpException(
+        getReasonPhrase(StatusCodes.BAD_REQUEST),
+        StatusCodes.BAD_REQUEST
+      );
+    }
 
     await this.accountService.updateAccountBalance({
-      currency,
-      accountId: accountIdFrom,
+      accountId: accountFrom.id,
       amount: -balance,
+      currency: accountFrom.currency,
       userId: this.request.user.id
     });
 
     await this.accountService.updateAccountBalance({
-      currency,
-      accountId: accountIdTo,
+      accountId: accountTo.id,
       amount: balance,
+      currency: accountFrom.currency,
       userId: this.request.user.id
     });
   }
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 bfe5a667..991ab745 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
@@ -2,6 +2,7 @@
   <button
     class="align-items-center d-flex"
     mat-stroked-button
+    [disabled]="dataSource?.data.length < 2"
     (click)="onTransferBalance()"
   >
     <ion-icon class="mr-2" name="arrow-redo-outline"></ion-icon>
diff --git a/apps/client/src/app/pages/accounts/accounts-page.component.ts b/apps/client/src/app/pages/accounts/accounts-page.component.ts
index 61db4757..b0a3d512 100644
--- a/apps/client/src/app/pages/accounts/accounts-page.component.ts
+++ b/apps/client/src/app/pages/accounts/accounts-page.component.ts
@@ -13,8 +13,8 @@ import { User } from '@ghostfolio/common/interfaces';
 import { hasPermission, permissions } from '@ghostfolio/common/permissions';
 import { Account as AccountModel } from '@prisma/client';
 import { DeviceDetectorService } from 'ngx-device-detector';
-import { Subject, Subscription } from 'rxjs';
-import { takeUntil } from 'rxjs/operators';
+import { EMPTY, Subject, Subscription } from 'rxjs';
+import { catchError, takeUntil } from 'rxjs/operators';
 
 import { CreateOrUpdateAccountDialog } from './create-or-update-account-dialog/create-or-update-account-dialog.component';
 import { TransferBalanceDialog } from './transfer-balance/transfer-balance-dialog.component';
@@ -283,7 +283,6 @@ export class AccountsPageComponent implements OnDestroy, OnInit {
       data: {
         accounts: this.accounts
       },
-      height: this.deviceType === 'mobile' ? '97.5vh' : '80vh',
       width: this.deviceType === 'mobile' ? '100vw' : '50rem'
     });
 
@@ -301,7 +300,14 @@ export class AccountsPageComponent implements OnDestroy, OnInit {
               accountIdTo,
               balance
             })
-            .pipe(takeUntil(this.unsubscribeSubject))
+            .pipe(
+              catchError(() => {
+                alert($localize`Oops, transfer cash balance has failed.`);
+
+                return EMPTY;
+              }),
+              takeUntil(this.unsubscribeSubject)
+            )
             .subscribe(() => {
               this.fetchAccounts();
             });
diff --git a/apps/client/src/app/pages/accounts/transfer-balance/transfer-balance-dialog.component.ts b/apps/client/src/app/pages/accounts/transfer-balance/transfer-balance-dialog.component.ts
index 4850e96e..8fc4e137 100644
--- a/apps/client/src/app/pages/accounts/transfer-balance/transfer-balance-dialog.component.ts
+++ b/apps/client/src/app/pages/accounts/transfer-balance/transfer-balance-dialog.component.ts
@@ -4,7 +4,13 @@ import {
   Inject,
   OnDestroy
 } from '@angular/core';
-import { FormBuilder, FormGroup, Validators } from '@angular/forms';
+import {
+  AbstractControl,
+  FormBuilder,
+  FormGroup,
+  ValidationErrors,
+  Validators
+} from '@angular/forms';
 import { MAT_DIALOG_DATA, MatDialogRef } from '@angular/material/dialog';
 import { TransferBalanceDto } from '@ghostfolio/api/app/account/transfer-balance.dto';
 import { Account } from '@prisma/client';
@@ -35,11 +41,16 @@ export class TransferBalanceDialog implements OnDestroy {
   public ngOnInit() {
     this.accounts = this.data.accounts;
 
-    this.transferBalanceForm = this.formBuilder.group({
-      balance: [0, Validators.required],
-      fromAccount: ['', Validators.required],
-      toAccount: ['', Validators.required]
-    });
+    this.transferBalanceForm = this.formBuilder.group(
+      {
+        balance: ['', Validators.required],
+        fromAccount: ['', Validators.required],
+        toAccount: ['', Validators.required]
+      },
+      {
+        validators: this.compareAccounts
+      }
+    );
 
     this.transferBalanceForm.get('fromAccount').valueChanges.subscribe((id) => {
       this.currency = this.accounts.find((account) => {
@@ -66,4 +77,13 @@ export class TransferBalanceDialog implements OnDestroy {
     this.unsubscribeSubject.next();
     this.unsubscribeSubject.complete();
   }
+
+  private compareAccounts(control: AbstractControl): ValidationErrors {
+    const accountFrom = control.get('fromAccount');
+    const accountTo = control.get('toAccount');
+
+    if (accountFrom.value === accountTo.value) {
+      return { invalid: true };
+    }
+  }
 }