From c0033ae56bf10baac02a19c5af01b943cb45a936 Mon Sep 17 00:00:00 2001
From: James Cole <thegrumpydictator@gmail.com>
Date: Sat, 10 Aug 2019 17:11:57 +0200
Subject: [PATCH] Code cleanup in AccountForm.

---
 app/Support/Amount.php           |  10 +-
 app/Support/Form/AccountForm.php | 242 ++++++++++++-------------------
 app/Support/Form/FormSupport.php |  28 ++++
 app/Support/Steam.php            |  16 +-
 4 files changed, 126 insertions(+), 170 deletions(-)

diff --git a/app/Support/Amount.php b/app/Support/Amount.php
index fa8d176d2c..65cffb7b10 100644
--- a/app/Support/Amount.php
+++ b/app/Support/Amount.php
@@ -23,7 +23,6 @@ declare(strict_types=1);
 namespace FireflyIII\Support;
 
 use Crypt;
-use FireflyIII\Exceptions\FireflyException;
 use FireflyIII\Models\TransactionCurrency;
 use FireflyIII\User;
 use Illuminate\Contracts\Encryption\DecryptException;
@@ -269,8 +268,6 @@ class Amount
 
     /**
      * @return \FireflyIII\Models\TransactionCurrency
-     *
-     * @throws \FireflyIII\Exceptions\FireflyException
      */
     public function getDefaultCurrency(): TransactionCurrency
     {
@@ -287,8 +284,6 @@ class Amount
      * @param User $user
      *
      * @return \FireflyIII\Models\TransactionCurrency
-     *
-     * @throws \FireflyIII\Exceptions\FireflyException
      */
     public function getDefaultCurrencyByUser(User $user): TransactionCurrency
     {
@@ -309,12 +304,13 @@ class Amount
 
         // could still be json encoded:
         if (strlen($currencyCode) > 3) {
-            $currencyCode = json_decode($currencyCode) ?? 'EUR';
+            $currencyCode = json_decode($currencyCode, true) ?? 'EUR';
         }
 
         $currency = TransactionCurrency::where('code', $currencyCode)->first();
         if (null === $currency) {
-            throw new FireflyException(sprintf('No currency found with code "%s"', $currencyCode));
+            // get EUR
+            $currency = TransactionCurrency::where('code', 'EUR')->first();
         }
         $cache->store($currency);
 
diff --git a/app/Support/Form/AccountForm.php b/app/Support/Form/AccountForm.php
index c94d150dbc..b32a19105a 100644
--- a/app/Support/Form/AccountForm.php
+++ b/app/Support/Form/AccountForm.php
@@ -22,11 +22,8 @@
 namespace FireflyIII\Support\Form;
 
 
-use Carbon\Carbon;
 use FireflyIII\Models\Account;
 use FireflyIII\Models\AccountType;
-use FireflyIII\Repositories\Account\AccountRepositoryInterface;
-use FireflyIII\Repositories\Currency\CurrencyRepositoryInterface;
 use Log;
 use Throwable;
 
@@ -43,88 +40,75 @@ class AccountForm
     use FormSupport;
 
     /**
-     * TODO describe.
+     * Shows a <select> with all active asset accounts.
      *
      * @param string $name
-     * @param mixed $value
-     * @param array $options
+     * @param mixed  $value
+     * @param array  $options
      *
      * @return string
      */
     public function activeAssetAccountList(string $name, $value = null, array $options = null): string
     {
-        // make repositories
-        /** @var AccountRepositoryInterface $repository */
-        $repository = app(AccountRepositoryInterface::class);
-        /** @var CurrencyRepositoryInterface $currencyRepos */
-        $currencyRepos = app(CurrencyRepositoryInterface::class);
-
+        $repository      = $this->getAccountRepository();
         $accountList     = $repository->getActiveAccountsByType([AccountType::ASSET, AccountType::DEFAULT]);
         $defaultCurrency = app('amount')->getDefaultCurrency();
         $grouped         = [];
-        // group accounts:
+        $date            = $this->getDate();
+
         /** @var Account $account */
         foreach ($accountList as $account) {
-            $balance    = app('steam')->balance($account, new Carbon);
-            $currencyId = (int)$repository->getMetaValue($account, 'currency_id');
-            $currency   = $currencyRepos->findNull($currencyId);
-            $role       = $repository->getMetaValue($account, 'account_role');
-            if ('' === $role) {
-                $role = 'no_account_type'; // @codeCoverageIgnore
-            }
-
-            if (null === $currency) {
-                $currency = $defaultCurrency;
-            }
-
-            $key                         = (string)trans('firefly.opt_group_' . $role);
-            $grouped[$key][$account->id] = $account->name . ' (' . app('amount')->formatAnything($currency, $balance, false) . ')';
+            $balance                     = app('steam')->balance($account, $date);
+            $role                        = $repository->getMetaValue($account, 'account_role');
+            $currency                    = $repository->getAccountCurrency($account) ?? $defaultCurrency;
+            $role                        = '' === $role ? 'no_account_type' : $role;
+            $key                         = (string)trans(sprintf('firefly.opt_group_%s', $role));
+            $formatted                   = app('amount')->formatAnything($currency, $balance, false);
+            $name                        = sprintf('%s (%s)', $account->name, $formatted);
+            $grouped[$key][$account->id] = $name;
         }
 
         return $this->select($name, $grouped, $value, $options);
     }
 
 
-
     /**
-     * TODO describe.
+     * Return a list that includes liabilities.
      *
      * @param string $name
-     * @param mixed $value
-     * @param array $options
+     * @param mixed  $value
+     * @param array  $options
      *
      * @return string
      */
     public function activeLongAccountList(string $name, $value = null, array $options = null): string
     {
-        // make repositories
-        /** @var AccountRepositoryInterface $repository */
-        $repository      = app(AccountRepositoryInterface::class);
-        $accountList     = $repository->getActiveAccountsByType(
-            [AccountType::ASSET, AccountType::DEFAULT, AccountType::MORTGAGE, AccountType::DEBT, AccountType::CREDITCARD, AccountType::LOAN,]
-        );
+        $types           = [AccountType::ASSET, AccountType::DEFAULT, AccountType::MORTGAGE, AccountType::DEBT, AccountType::CREDITCARD, AccountType::LOAN,];
         $liabilityTypes  = [AccountType::MORTGAGE, AccountType::DEBT, AccountType::CREDITCARD, AccountType::LOAN];
+        $repository      = $this->getAccountRepository();
+        $accountList     = $repository->getActiveAccountsByType($types);
         $defaultCurrency = app('amount')->getDefaultCurrency();
         $grouped         = [];
-        // group accounts:
+        $date            = $this->getDate();
 
         /** @var Account $account */
         foreach ($accountList as $account) {
-            $balance = app('steam')->balance($account, new Carbon);
-
+            $balance  = app('steam')->balance($account, $date);
             $currency = $repository->getAccountCurrency($account) ?? $defaultCurrency;
-
-            $role = $repository->getMetaValue($account, 'account_role');
+            $role     = $repository->getMetaValue($account, 'account_role');
 
             if ('' === $role && !in_array($account->accountType->type, $liabilityTypes, true)) {
-                $role = 'no_account_type'; // @codeCoverageIgnore
+                $role = 'no_account_type';
             }
 
             if (in_array($account->accountType->type, $liabilityTypes, true)) {
-                $role = 'l_' . $account->accountType->type; // @codeCoverageIgnore
+                $role = sprintf('l_%s', $account->accountType->type);
             }
-            $key                         = (string)trans('firefly.opt_group_' . $role);
-            $grouped[$key][$account->id] = $account->name . ' (' . app('amount')->formatAnything($currency, $balance, false) . ')';
+
+            $key                         = (string)trans(sprintf('firefly.opt_group_%s', $role));
+            $formatted                   = app('amount')->formatAnything($currency, $balance, false);
+            $name                        = sprintf('%s (%s)', $account->name, $formatted);
+            $grouped[$key][$account->id] = $name;
         }
 
 
@@ -132,8 +116,6 @@ class AccountForm
     }
 
     /**
-     * TODO clean up.
-     *
      * Grouped dropdown list of all accounts that are valid as the destination of a withdrawal.
      *
      * @param string $name
@@ -144,24 +126,14 @@ class AccountForm
      */
     public function activeWithdrawalDestinations(string $name, $value = null, array $options = null): string
     {
-        // make repositories
-        /** @var AccountRepositoryInterface $repository */
-        $repository = app(AccountRepositoryInterface::class);
-
-        $accountList     = $repository->getActiveAccountsByType(
-            [
-                AccountType::MORTGAGE,
-                AccountType::DEBT,
-                AccountType::CREDITCARD,
-                AccountType::LOAN,
-                AccountType::EXPENSE,
-            ]
-        );
+        $types           = [AccountType::MORTGAGE, AccountType::DEBT, AccountType::CREDITCARD, AccountType::LOAN, AccountType::EXPENSE,];
         $liabilityTypes  = [AccountType::MORTGAGE, AccountType::DEBT, AccountType::CREDITCARD, AccountType::LOAN];
+        $repository      = $this->getAccountRepository();
+        $accountList     = $repository->getActiveAccountsByType($types);
         $defaultCurrency = app('amount')->getDefaultCurrency();
         $grouped         = [];
+        $date            = $this->getDate();
 
-        // add cash account first:
         $cash                     = $repository->getCashAccount();
         $key                      = (string)trans('firefly.cash_account_type');
         $grouped[$key][$cash->id] = sprintf('(%s)', (string)trans('firefly.cash'));
@@ -169,34 +141,30 @@ class AccountForm
         // group accounts:
         /** @var Account $account */
         foreach ($accountList as $account) {
-            $balance  = app('steam')->balance($account, new Carbon);
+            $balance  = app('steam')->balance($account, $date);
             $currency = $repository->getAccountCurrency($account) ?? $defaultCurrency;
             $role     = (string)$repository->getMetaValue($account, 'account_role');
             if ('' === $role && !in_array($account->accountType->type, $liabilityTypes, true)) {
-                $role = 'no_account_type'; // @codeCoverageIgnore
+                $role = 'no_account_type';
             }
-            if ('no_account_type' === $role && AccountType::EXPENSE === $account->accountType->type) {
-                $role = 'expense_account'; // @codeCoverageIgnore
 
+            if ('no_account_type' === $role && AccountType::EXPENSE === $account->accountType->type) {
+                $role = 'expense_account';
             }
 
             if (in_array($account->accountType->type, $liabilityTypes, true)) {
-                $role = 'l_' . $account->accountType->type; // @codeCoverageIgnore
+                $role = sprintf('l_%s', $account->accountType->type);
             }
-
-            if (null === $currency) {
-                $currency = $defaultCurrency;
-            }
-
             $key                         = (string)trans('firefly.opt_group_' . $role);
-            $grouped[$key][$account->id] = $account->name . ' (' . app('amount')->formatAnything($currency, $balance, false) . ')';
+            $formatted                   = app('amount')->formatAnything($currency, $balance, false);
+            $name                        = sprintf('%s (%s)', $account->name, $formatted);
+            $grouped[$key][$account->id] = $name;
         }
 
         return $this->select($name, $grouped, $value, $options);
     }
 
     /**
-     * TODO cleanup.
      * Grouped dropdown list of all accounts that are valid as the destination of a withdrawal.
      *
      * @param string $name
@@ -207,24 +175,13 @@ class AccountForm
      */
     public function activeDepositDestinations(string $name, $value = null, array $options = null): string
     {
-        // make repositories
-        /** @var AccountRepositoryInterface $repository */
-        $repository = app(AccountRepositoryInterface::class);
-
-        $accountList     = $repository->getActiveAccountsByType(
-            [
-                AccountType::MORTGAGE,
-                AccountType::DEBT,
-                AccountType::CREDITCARD,
-                AccountType::LOAN,
-                AccountType::REVENUE,
-            ]
-        );
-        $liabilityTypes  = [AccountType::MORTGAGE, AccountType::DEBT, AccountType::CREDITCARD, AccountType::LOAN];
-        $defaultCurrency = app('amount')->getDefaultCurrency();
-        $grouped         = [];
-
-        // add cash account first:
+        $types                    = [AccountType::MORTGAGE, AccountType::DEBT, AccountType::CREDITCARD, AccountType::LOAN, AccountType::REVENUE,];
+        $liabilityTypes           = [AccountType::MORTGAGE, AccountType::DEBT, AccountType::CREDITCARD, AccountType::LOAN];
+        $repository               = $this->getAccountRepository();
+        $accountList              = $repository->getActiveAccountsByType($types);
+        $defaultCurrency          = app('amount')->getDefaultCurrency();
+        $grouped                  = [];
+        $date                     = $this->getDate();
         $cash                     = $repository->getCashAccount();
         $key                      = (string)trans('firefly.cash_account_type');
         $grouped[$key][$cash->id] = sprintf('(%s)', (string)trans('firefly.cash'));
@@ -232,23 +189,22 @@ class AccountForm
         // group accounts:
         /** @var Account $account */
         foreach ($accountList as $account) {
-            $balance  = app('steam')->balance($account, new Carbon);
+            $balance  = app('steam')->balance($account, $date);
             $currency = $repository->getAccountCurrency($account) ?? $defaultCurrency;
             $role     = (string)$repository->getMetaValue($account, 'account_role');
             if ('' === $role && !in_array($account->accountType->type, $liabilityTypes, true)) {
-                $role = 'no_account_type'; // @codeCoverageIgnore
+                $role = 'no_account_type';
             }
             if ('no_account_type' === $role && AccountType::REVENUE === $account->accountType->type) {
-                $role = 'revenue_account'; // @codeCoverageIgnore
-
+                $role = 'revenue_account';
             }
-
             if (in_array($account->accountType->type, $liabilityTypes, true)) {
-                $role = 'l_' . $account->accountType->type; // @codeCoverageIgnore
+                $role = sprintf('l_%s', $account->accountType->type); // @codeCoverageIgnore
             }
-
-            $key                         = (string)trans('firefly.opt_group_' . $role);
-            $grouped[$key][$account->id] = $account->name . ' (' . app('amount')->formatAnything($currency, $balance, false) . ')';
+            $key                         = (string)trans(sprintf('firefly.opt_group_%s', $role));
+            $formatted                   = app('amount')->formatAnything($currency, $balance, false);
+            $name                        = sprintf('%s (%s)', $account->name, $formatted);
+            $grouped[$key][$account->id] = $name;
         }
 
         return $this->select($name, $grouped, $value, $options);
@@ -256,7 +212,7 @@ class AccountForm
 
 
     /**
-     * TODO describe and cleanup.
+     * Check list of asset accounts.
      *
      * @param string $name
      * @param array $options
@@ -273,18 +229,18 @@ class AccountForm
         $selected = request()->old($name) ?? [];
 
         // get all asset accounts:
-        /** @var AccountRepositoryInterface $repository */
-        $repository    = app(AccountRepositoryInterface::class);
-        $assetAccounts = $repository->getAccountsByType([AccountType::ASSET, AccountType::DEFAULT]);
+        $repository    = $this->getAccountRepository();
+        $types         = [AccountType::ASSET, AccountType::DEFAULT];
+        $assetAccounts = $repository->getAccountsByType($types);
         $grouped       = [];
         // group accounts:
         /** @var Account $account */
         foreach ($assetAccounts as $account) {
             $role = $repository->getMetaValue($account, 'account_role');
             if (null === $role) {
-                $role = 'no_account_type'; // @codeCoverageIgnore
+                $role = 'no_account_type';
             }
-            $key                         = (string)trans('firefly.opt_group_' . $role);
+            $key                         = (string)trans(sprintf('firefly.opt_group_%s', $role));
             $grouped[$key][$account->id] = $account->name;
         }
 
@@ -300,7 +256,7 @@ class AccountForm
     }
 
     /**
-     * TODO describe and cleanup.
+     * Basic list of asset accounts.
      *
      * @param string $name
      * @param mixed $value
@@ -310,32 +266,25 @@ class AccountForm
      */
     public function assetAccountList(string $name, $value = null, array $options = null): string
     {
-        // make repositories
-        /** @var AccountRepositoryInterface $repository */
-        $repository = app(AccountRepositoryInterface::class);
-        /** @var CurrencyRepositoryInterface $currencyRepos */
-        $currencyRepos = app(CurrencyRepositoryInterface::class);
-
-        $accountList     = $repository->getAccountsByType([AccountType::ASSET, AccountType::DEFAULT]);
+        $repository      = $this->getAccountRepository();
+        $types           = [AccountType::ASSET, AccountType::DEFAULT];
+        $accountList     = $repository->getAccountsByType($types);
         $defaultCurrency = app('amount')->getDefaultCurrency();
         $grouped         = [];
-        // group accounts:
+        $date            = $this->getDate();
+
         /** @var Account $account */
         foreach ($accountList as $account) {
-            $balance    = app('steam')->balance($account, new Carbon);
-            $currencyId = (int)$repository->getMetaValue($account, 'currency_id');
-            $currency   = $currencyRepos->findNull($currencyId);
-            $role       = (string)$repository->getMetaValue($account, 'account_role');
+            $balance  = app('steam')->balance($account, $date);
+            $currency = $repository->getAccountCurrency($account) ?? $defaultCurrency;
+            $role     = (string)$repository->getMetaValue($account, 'account_role');
             if ('' === $role) {
-                $role = 'no_account_type'; // @codeCoverageIgnore
+                $role = 'no_account_type';
             }
 
-            if (null === $currency) {
-                $currency = $defaultCurrency;
-            }
-
-            $key                         = (string)trans('firefly.opt_group_' . $role);
-            $grouped[$key][$account->id] = $account->name . ' (' . app('amount')->formatAnything($currency, $balance, false) . ')';
+            $key                         = (string)trans(sprintf('firefly.opt_group_%s', $role));
+            $formatted                   = app('amount')->formatAnything($currency, $balance, false);
+            $grouped[$key][$account->id] = sprintf('%s (%s)', $account->name, $formatted);
         }
 
         return $this->select($name, $grouped, $value, $options);
@@ -343,7 +292,8 @@ class AccountForm
 
 
     /**
-     * TODO decribe and cleanup
+     * Same list but all liabilities as well.
+     *
      * @param string $name
      * @param mixed $value
      * @param array $options
@@ -352,39 +302,27 @@ class AccountForm
      */
     public function longAccountList(string $name, $value = null, array $options = null): string
     {
-        // make repositories
-        /** @var AccountRepositoryInterface $repository */
-        $repository = app(AccountRepositoryInterface::class);
-        /** @var CurrencyRepositoryInterface $currencyRepos */
-        $currencyRepos = app(CurrencyRepositoryInterface::class);
-
-        $accountList     = $repository->getAccountsByType(
-            [AccountType::ASSET, AccountType::DEFAULT, AccountType::MORTGAGE, AccountType::DEBT, AccountType::CREDITCARD, AccountType::LOAN,]
-        );
+        $types           = [AccountType::ASSET, AccountType::DEFAULT, AccountType::MORTGAGE, AccountType::DEBT, AccountType::CREDITCARD, AccountType::LOAN,];
         $liabilityTypes  = [AccountType::MORTGAGE, AccountType::DEBT, AccountType::CREDITCARD, AccountType::LOAN];
+        $repository      = $this->getAccountRepository();
+        $accountList     = $repository->getAccountsByType($types);
         $defaultCurrency = app('amount')->getDefaultCurrency();
         $grouped         = [];
-        // group accounts:
+        $date            = $this->getDate();
         /** @var Account $account */
         foreach ($accountList as $account) {
-            $balance    = app('steam')->balance($account, new Carbon);
-            $currencyId = (int)$repository->getMetaValue($account, 'currency_id');
-            $currency   = $currencyRepos->findNull($currencyId);
-            $role       = (string)$repository->getMetaValue($account, 'account_role'); // TODO bad form for currency
+            $balance  = app('steam')->balance($account, $date);
+            $currency = $repository->getAccountCurrency($account) ?? $defaultCurrency;
+            $role     = (string)$repository->getMetaValue($account, 'account_role');
             if ('' === $role) {
-                $role = 'no_account_type'; // @codeCoverageIgnore
+                $role = 'no_account_type';
             }
-
             if (in_array($account->accountType->type, $liabilityTypes, true)) {
-                $role = 'l_' . $account->accountType->type; // @codeCoverageIgnore
+                $role = sprintf('l_%s', $account->accountType->type);
             }
-
-            if (null === $currency) {
-                $currency = $defaultCurrency;
-            }
-
-            $key                         = (string)trans('firefly.opt_group_' . $role);
-            $grouped[$key][$account->id] = $account->name . ' (' . app('amount')->formatAnything($currency, $balance, false) . ')';
+            $key                         = (string)trans(sprintf('firefly.opt_group_%s', $role));
+            $formatted                   = app('amount')->formatAnything($currency, $balance, false);
+            $grouped[$key][$account->id] = sprintf('%s (%s)', $account->name, $formatted);
         }
 
         return $this->select($name, $grouped, $value, $options);
diff --git a/app/Support/Form/FormSupport.php b/app/Support/Form/FormSupport.php
index 4f3ef06336..947c7374e1 100644
--- a/app/Support/Form/FormSupport.php
+++ b/app/Support/Form/FormSupport.php
@@ -22,6 +22,8 @@
 namespace FireflyIII\Support\Form;
 
 use Carbon\Carbon;
+use Exception;
+use FireflyIII\Repositories\Account\AccountRepositoryInterface;
 use Illuminate\Support\MessageBag;
 use Log;
 use RuntimeException;
@@ -32,6 +34,32 @@ use Throwable;
  */
 trait FormSupport
 {
+
+
+    /**
+     * @return AccountRepositoryInterface
+     */
+    protected function getAccountRepository(): AccountRepositoryInterface
+    {
+        return app(AccountRepositoryInterface::class);
+    }
+
+    /**
+     * @return Carbon
+     */
+    protected function getDate(): Carbon
+    {
+        /** @var Carbon $date */
+        $date = null;
+        try {
+            $date = new Carbon;
+        } catch (Exception $e) {
+            $e->getMessage();
+        }
+
+        return $date;
+    }
+
     /**
      * @param string $name
      * @param array  $list
diff --git a/app/Support/Steam.php b/app/Support/Steam.php
index 1333c22c71..425c3efe2a 100644
--- a/app/Support/Steam.php
+++ b/app/Support/Steam.php
@@ -55,31 +55,25 @@ class Steam
         $cache->addProperty('balance');
         $cache->addProperty($date);
         if ($cache->has()) {
-            //return $cache->get(); // @codeCoverageIgnore
+            return $cache->get(); // @codeCoverageIgnore
         }
-        //
         /** @var AccountRepositoryInterface $repository */
         $repository = app(AccountRepositoryInterface::class);
-        $currencyId = (int)$repository->getMetaValue($account, 'currency_id');
+        $currency = $repository->getAccountCurrency($account) ?? app('amount')->getDefaultCurrencyByUser($account->user);
 
-        // use system default currency:
-        if (0 === $currencyId) {
-            $currency   = app('amount')->getDefaultCurrencyByUser($account->user);
-            $currencyId = $currency->id;
-        }
         // first part: get all balances in own currency:
         $nativeBalance = (string)$account->transactions()
                                          ->leftJoin('transaction_journals', 'transaction_journals.id', '=', 'transactions.transaction_journal_id')
                                          ->where('transaction_journals.date', '<=', $date->format('Y-m-d 23:59:59'))
-                                         ->where('transactions.transaction_currency_id', $currencyId)
+                                         ->where('transactions.transaction_currency_id', $currency->id)
                                          ->sum('transactions.amount');
 
         // get all balances in foreign currency:
         $foreignBalance = (string)$account->transactions()
                                           ->leftJoin('transaction_journals', 'transaction_journals.id', '=', 'transactions.transaction_journal_id')
                                           ->where('transaction_journals.date', '<=', $date->format('Y-m-d'))
-                                          ->where('transactions.foreign_currency_id', $currencyId)
-                                          ->where('transactions.transaction_currency_id', '!=', $currencyId)
+                                          ->where('transactions.foreign_currency_id', $currency->id)
+                                          ->where('transactions.transaction_currency_id', '!=', $currency->id)
                                           ->sum('transactions.foreign_amount');
 
         $balance = bcadd($nativeBalance, $foreignBalance);