Refactoring of code for #1159

app/Support/Import/JobConfiguration/FinTS/ChooseAccountHandler.php

@@ -1,7 +1,7 @@
 <?php
 /**
  * ChooseAccountHandler.php
- * Copyright (c) 2017 thegrumpydictator@gmail.com
+ * Copyright (c) 2018 https://github.com/bnw
  *
  * This file is part of Firefly III.
  *
@@ -28,19 +28,23 @@ use FireflyIII\Exceptions\FireflyException;
 use FireflyIII\Import\JobConfiguration\FinTSConfigurationSteps;
 use FireflyIII\Models\AccountType;
 use FireflyIII\Models\ImportJob;
+use FireflyIII\Repositories\Account\AccountRepositoryInterface;
 use FireflyIII\Repositories\ImportJob\ImportJobRepositoryInterface;
 use FireflyIII\Support\FinTS\FinTS;
 use Illuminate\Support\MessageBag;
-use FireflyIII\Repositories\Account\AccountRepositoryInterface;
 
+/**
+ *
+ * Class ChooseAccountHandler
+ */
 class ChooseAccountHandler implements FinTSConfigurationInterface
 {
+    /** @var AccountRepositoryInterface */
+    private $accountRepository;
     /** @var ImportJob */
     private $importJob;
     /** @var ImportJobRepositoryInterface */
     private $repository;
-    /** @var AccountRepositoryInterface */
-    private $accountRepository;
 
     /**
      * Store data associated with current stage.
@@ -51,7 +55,7 @@ class ChooseAccountHandler implements FinTSConfigurationInterface
      */
     public function configureJob(array $data): MessageBag
     {
-        $config                  = $this->importJob->configuration;
+        $config                  = $this->repository->getConfiguration($this->importJob);
         $config['fints_account'] = (string)($data['fints_account'] ?? '');
         $config['local_account'] = (string)($data['local_account'] ?? '');
         $config['from_date']     = (string)($data['from_date'] ?? '');
@@ -59,7 +63,7 @@ class ChooseAccountHandler implements FinTSConfigurationInterface
         $this->repository->setConfiguration($this->importJob, $config);
 
         try {
-            $finTS = app(FinTS::class, ['config' => $this->importJob->configuration]);
+            $finTS = app(FinTS::class, ['config' => $config]);
             $finTS->getAccount($config['fints_account']);
         } catch (FireflyException $e) {
             return new MessageBag([$e->getMessage()]);
@@ -89,19 +93,20 @@ class ChooseAccountHandler implements FinTSConfigurationInterface
         foreach ($this->accountRepository->getAccountsByType([AccountType::ASSET]) as $localAccount) {
             $display_name = $localAccount->name;
             if ($localAccount->iban) {
-                $display_name .= " - $localAccount->iban";
+                $display_name .= sprintf(' - %s', $localAccount->iban);
             }
             $localAccounts[$localAccount->id] = $display_name;
         }
 
         $data = [
             'fints_accounts' => $finTSAccountsData,
-            'fints_account' => $this->importJob->configuration['fints_account'] ?? null,
+            'fints_account'  => $this->importJob->configuration['fints_account'] ?? null,
             'local_accounts' => $localAccounts,
-            'local_account' => $this->importJob->configuration['local_account'] ?? null,
-            'from_date' => $this->importJob->configuration['from_date'] ?? (new Carbon('now - 1 month'))->format('Y-m-d'),
-            'to_date' => $this->importJob->configuration['to_date'] ?? (new Carbon('now'))->format('Y-m-d')
+            'local_account'  => $this->importJob->configuration['local_account'] ?? null,
+            'from_date'      => $this->importJob->configuration['from_date'] ?? (new Carbon('now - 1 month'))->format('Y-m-d'),
+            'to_date'        => $this->importJob->configuration['to_date'] ?? (new Carbon('now'))->format('Y-m-d'),
         ];
+
         return $data;
     }
 
@@ -115,6 +120,4 @@ class ChooseAccountHandler implements FinTSConfigurationInterface
         $this->accountRepository = app(AccountRepositoryInterface::class);
         $this->repository->setUser($importJob->user);
     }
-
-
 }

app/Support/Import/JobConfiguration/FinTS/FinTSConfigurationInterface.php

@@ -1,7 +1,7 @@
 <?php
 /**
  * FinTSConfigurationInterface.php
- * Copyright (c) 2017 thegrumpydictator@gmail.com
+ * Copyright (c) 2018 https://github.com/bnw
  *
  * This file is part of Firefly III.
  *
@@ -25,6 +25,9 @@ namespace FireflyIII\Support\Import\JobConfiguration\FinTS;
 use FireflyIII\Models\ImportJob;
 use Illuminate\Support\MessageBag;
 
+/**
+ *
+ */
 interface FinTSConfigurationInterface
 {
     /**

app/Support/Import/JobConfiguration/FinTS/NewFinTSJobHandler.php

@@ -1,7 +1,7 @@
 <?php
 /**
  * NewFinTSJobHandler.php
- * Copyright (c) 2017 thegrumpydictator@gmail.com
+ * Copyright (c) 2018 https://github.com/bnw
  *
  * This file is part of Firefly III.
  *
@@ -31,6 +31,10 @@ use FireflyIII\Support\FinTS\FinTS;
 use Illuminate\Support\Facades\Crypt;
 use Illuminate\Support\MessageBag;
 
+/**
+ *
+ * Class NewFinTSJobHandler
+ */
 class NewFinTSJobHandler implements FinTSConfigurationInterface
 {
     /** @var ImportJob */
@@ -60,14 +64,14 @@ class NewFinTSJobHandler implements FinTSConfigurationInterface
 
         $incomplete = false;
         foreach ($config as $value) {
-            $incomplete = $value === '' or $incomplete;
+            $incomplete = '' === $value or $incomplete;
         }
         if ($incomplete) {
             return new MessageBag([trans('import.incomplete_fints_form')]);
         }
 
         $finTS = app(FinTS::class, ['config' => $this->importJob->configuration]);
-        if (($checkConnection = $finTS->checkConnection()) !== true) {
+        if (true !== ($checkConnection = $finTS->checkConnection())) {
             return new MessageBag([trans('import.fints_connection_failed', ['originalError' => $checkConnection])]);
         }
 
@@ -84,11 +88,12 @@ class NewFinTSJobHandler implements FinTSConfigurationInterface
     public function getNextData(): array
     {
         $config = $this->importJob->configuration;
+
         return [
-            'fints_url' => $config['fints_url'] ?? '',
-            'fints_port' => $config['fints_port'] ?? '443',
+            'fints_url'       => $config['fints_url'] ?? '',
+            'fints_port'      => $config['fints_port'] ?? '443',
             'fints_bank_code' => $config['fints_bank_code'] ?? '',
-            'fints_username' => $config['fints_username'] ?? ''
+            'fints_username'  => $config['fints_username'] ?? '',
         ];
     }
 

app/Support/Import/Routine/FinTS/StageImportDataHandler.php

@@ -1,34 +1,92 @@
 <?php
+/**
+ * StageImportDataHandler.php
+ * Copyright (c) 2018 https://github.com/bnw
+ *
+ * This file is part of Firefly III.
+ *
+ * Firefly III is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * Firefly III is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with Firefly III. If not, see <http://www.gnu.org/licenses/>.
+ */
 
+declare(strict_types=1);
 
 namespace FireflyIII\Support\Import\Routine\FinTS;
 
 
-use Fhp\Model\StatementOfAccount\Transaction as FinTSTransaction;
 use Fhp\Model\StatementOfAccount\Transaction;
+use Fhp\Model\StatementOfAccount\Transaction as FinTSTransaction;
 use FireflyIII\Exceptions\FireflyException;
+use FireflyIII\Models\Account as LocalAccount;
 use FireflyIII\Models\AccountType;
 use FireflyIII\Models\ImportJob;
 use FireflyIII\Models\TransactionType;
 use FireflyIII\Repositories\Account\AccountRepositoryInterface;
 use FireflyIII\Repositories\ImportJob\ImportJobRepositoryInterface;
 use FireflyIII\Support\FinTS\FinTS;
-use FireflyIII\Models\Account as LocalAccount;
 use FireflyIII\Support\Import\Routine\File\OpposingAccountMapper;
 use Illuminate\Support\Facades\Log;
 
+/**
+ *
+ * Class StageImportDataHandler
+ */
 class StageImportDataHandler
 {
     /** @var AccountRepositoryInterface */
     private $accountRepository;
     /** @var ImportJob */
     private $importJob;
+    /** @var OpposingAccountMapper */
+    private $mapper;
     /** @var ImportJobRepositoryInterface */
     private $repository;
     /** @var array */
     private $transactions;
-    /** @var OpposingAccountMapper */
-    private $mapper;
+
+    /**
+     * @return array
+     */
+    public function getTransactions(): array
+    {
+        return $this->transactions;
+    }
+
+    /**
+     * @throws FireflyException
+     */
+    public function run()
+    {
+        Log::debug('Now in StageImportDataHandler::run()');
+
+        $localAccount = $this->accountRepository->findNull($this->importJob->configuration['local_account']);
+        if (null === $localAccount) {
+            throw new FireflyException(sprintf('Cannot find Firefly account with id #%d ' , $this->importJob->configuration['local_account']));
+        }
+        $finTS              = app(FinTS::class, ['config' => $this->importJob->configuration]);
+        $fintTSAccount      = $finTS->getAccount($this->importJob->configuration['fints_account']);
+        $statementOfAccount = $finTS->getStatementOfAccount(
+            $fintTSAccount, new \DateTime($this->importJob->configuration['from_date']), new \DateTime($this->importJob->configuration['to_date'])
+        );
+        $collection         = [];
+        foreach ($statementOfAccount->getStatements() as $statement) {
+            foreach ($statement->getTransactions() as $transaction) {
+                $collection[] = $this->convertTransaction($transaction, $localAccount);
+            }
+        }
+
+        $this->transactions = $collection;
+    }
 
     /**
      * @param ImportJob $importJob
@@ -48,40 +106,23 @@ class StageImportDataHandler
     }
 
     /**
-     * @throws FireflyException
+     * @param FinTSTransaction $transaction
+     * @param LocalAccount     $source
+     *
+     * @return array
      */
-    public function run()
-    {
-        Log::debug('Now in StageImportDataHandler::run()');
-
-        $localAccount = $this->accountRepository->findNull($this->importJob->configuration['local_account']);
-        if ($localAccount === null) {
-            throw new FireflyException('Cannot find Firefly account with id ' . $this->importJob->configuration['local_account']);
-        }
-        $finTS              = app(FinTS::class, ['config' => $this->importJob->configuration]);
-        $fintTSAccount      = $finTS->getAccount($this->importJob->configuration['fints_account']);
-        $statementOfAccount = $finTS->getStatementOfAccount($fintTSAccount, new \DateTime($this->importJob->configuration['from_date']), new \DateTime($this->importJob->configuration['to_date']));
-        $collection         = [];
-        foreach ($statementOfAccount->getStatements() as $statement) {
-            foreach ($statement->getTransactions() as $transaction) {
-                $collection[] = $this->convertTransaction($transaction, $localAccount);
-            }
-        }
-
-        $this->transactions = $collection;
-    }
-
     private function convertTransaction(FinTSTransaction $transaction, LocalAccount $source): array
     {
         Log::debug(sprintf('Start converting transaction %s', $transaction->getDescription1()));
 
-        $amount        = (string) $transaction->getAmount();
+        $amount        = (string)$transaction->getAmount();
         $debitOrCredit = $transaction->getCreditDebit();
-
+        // assume deposit.
+        $type = TransactionType::DEPOSIT;
         Log::debug(sprintf('Amount is %s', $amount));
-        if ($debitOrCredit == Transaction::CD_CREDIT) {
-            $type = TransactionType::DEPOSIT;
-        } else {
+
+        // inverse if not.
+        if ($debitOrCredit !== Transaction::CD_CREDIT) {
             $type   = TransactionType::WITHDRAWAL;
             $amount = bcmul($amount, '-1');
         }
@@ -91,7 +132,7 @@ class StageImportDataHandler
             $amount,
             ['iban' => $transaction->getAccountNumber(), 'name' => $transaction->getName()]
         );
-        if ($debitOrCredit == Transaction::CD_CREDIT) {
+        if ($debitOrCredit === Transaction::CD_CREDIT) {
             [$source, $destination] = [$destination, $source];
         }
 
@@ -101,52 +142,44 @@ class StageImportDataHandler
         }
 
         $storeData = [
-            'user' => $this->importJob->user_id,
-            'type' => $type,
-            'date' => $transaction->getValutaDate()->format('Y-m-d'),
-            'description' => $transaction->getDescription1(),
-            'piggy_bank_id' => null,
-            'piggy_bank_name' => null,
-            'bill_id' => null,
-            'bill_name' => null,
-            'tags' => [],
+            'user'               => $this->importJob->user_id,
+            'type'               => $type,
+            'date'               => $transaction->getValutaDate()->format('Y-m-d'),
+            'description'        => $transaction->getDescription1(),
+            'piggy_bank_id'      => null,
+            'piggy_bank_name'    => null,
+            'bill_id'            => null,
+            'bill_name'          => null,
+            'tags'               => [],
             'internal_reference' => null,
-            'external_id' => null,
-            'notes' => null,
-            'bunq_payment_id' => null,
-            'original-source' => sprintf('fints-v%s', config('firefly.version')),
-            'transactions' => [
+            'external_id'        => null,
+            'notes'              => null,
+            'bunq_payment_id'    => null,
+            'original-source'    => sprintf('fints-v%s', config('firefly.version')),
+            'transactions'       => [
                 // single transaction:
                 [
-                    'description' => null,
-                    'amount' => $amount,
-                    'currency_id' => null,
-                    'currency_code' => 'EUR',
-                    'foreign_amount' => null,
-                    'foreign_currency_id' => null,
+                    'description'           => null,
+                    'amount'                => $amount,
+                    'currency_id'           => null,
+                    'currency_code'         => 'EUR',
+                    'foreign_amount'        => null,
+                    'foreign_currency_id'   => null,
                     'foreign_currency_code' => null,
-                    'budget_id' => null,
-                    'budget_name' => null,
-                    'category_id' => null,
-                    'category_name' => null,
-                    'source_id' => $source->id,
-                    'source_name' => null,
-                    'destination_id' => $destination->id,
-                    'destination_name' => null,
-                    'reconciled' => false,
-                    'identifier' => 0,
+                    'budget_id'             => null,
+                    'budget_name'           => null,
+                    'category_id'           => null,
+                    'category_name'         => null,
+                    'source_id'             => $source->id,
+                    'source_name'           => null,
+                    'destination_id'        => $destination->id,
+                    'destination_name'      => null,
+                    'reconciled'            => false,
+                    'identifier'            => 0,
                 ],
             ],
         ];
 
         return $storeData;
     }
-
-    /**
-     * @return array
-     */
-    public function getTransactions(): array
-    {
-        return $this->transactions;
-    }
 }