Clean up code for import routine.

app/Import/Configuration/BunqConfigurator.php

@@ -25,7 +25,6 @@ namespace FireflyIII\Import\Configuration;
 use FireflyIII\Exceptions\FireflyException;
 use FireflyIII\Models\ImportJob;
 use FireflyIII\Repositories\ImportJob\ImportJobRepositoryInterface;
-use FireflyIII\Support\Import\Configuration\Bunq\HasAccounts;
 use FireflyIII\Support\Import\Configuration\Bunq\HaveAccounts;
 use Log;
 
@@ -56,6 +55,7 @@ class BunqConfigurator implements ConfiguratorInterface
      * @param array $data
      *
      * @return bool
+     *
      * @throws FireflyException
      */
     public function configureJob(array $data): bool
@@ -89,6 +89,7 @@ class BunqConfigurator implements ConfiguratorInterface
      * Return the data required for the next step in the job configuration.
      *
      * @return array
+     *
      * @throws FireflyException
      */
     public function getNextData(): array
@@ -116,6 +117,7 @@ class BunqConfigurator implements ConfiguratorInterface
 
     /**
      * @return string
+     *
      * @throws FireflyException
      */
     public function getNextView(): string
@@ -131,7 +133,6 @@ class BunqConfigurator implements ConfiguratorInterface
                 return 'import.bunq.accounts';
             default:
                 return '';
-
         }
     }
 
@@ -147,6 +148,7 @@ class BunqConfigurator implements ConfiguratorInterface
 
     /**
      * @return bool
+     *
      * @throws FireflyException
      */
     public function isJobConfigured(): bool

app/Import/Configuration/FileConfigurator.php

@@ -61,7 +61,6 @@ class FileConfigurator implements ConfiguratorInterface
     /** @var ImportJobRepositoryInterface */
     private $repository;
 
-
     // give job default config:
     /** @var string */
     private $warning = '';
@@ -145,6 +144,7 @@ class FileConfigurator implements ConfiguratorInterface
      * Return possible warning to user.
      *
      * @return string
+     *
      * @throws FireflyException
      */
     public function getWarningMessage(): string
@@ -158,6 +158,7 @@ class FileConfigurator implements ConfiguratorInterface
 
     /**
      * @return bool
+     *
      * @throws FireflyException
      */
     public function isJobConfigured(): bool
@@ -167,7 +168,7 @@ class FileConfigurator implements ConfiguratorInterface
         }
         $config = $this->getConfig();
         $stage  = $config['stage'] ?? 'initial';
-        if ($stage === 'ready') {
+        if ('ready' === $stage) {
             Log::debug('isJobConfigured returns true');
 
             return true;
@@ -252,6 +253,7 @@ class FileConfigurator implements ConfiguratorInterface
      * Shorthand method to return the extended status.
      *
      * @codeCoverageIgnore
+     *
      * @return array
      */
     private function getExtendedStatus(): array

app/Import/Configuration/SpectreConfigurator.php

@@ -55,6 +55,7 @@ class SpectreConfigurator implements ConfiguratorInterface
      * @param array $data
      *
      * @return bool
+     *
      * @throws FireflyException
      */
     public function configureJob(array $data): bool
@@ -87,6 +88,7 @@ class SpectreConfigurator implements ConfiguratorInterface
      * Return the data required for the next step in the job configuration.
      *
      * @return array
+     *
      * @throws FireflyException
      */
     public function getNextData(): array
@@ -124,6 +126,7 @@ class SpectreConfigurator implements ConfiguratorInterface
 
     /**
      * @return string
+     *
      * @throws FireflyException
      */
     public function getNextView(): string
@@ -143,7 +146,6 @@ class SpectreConfigurator implements ConfiguratorInterface
                 return 'import.spectre.accounts';
             default:
                 return '';
-
         }
     }
 
@@ -159,6 +161,7 @@ class SpectreConfigurator implements ConfiguratorInterface
 
     /**
      * @return bool
+     *
      * @throws FireflyException
      */
     public function isJobConfigured(): bool

app/Import/Converter/Amount.php

@@ -113,13 +113,12 @@ class Amount implements ConverterInterface
     {
         $str = preg_replace('/[^\-\(\)\.\,0-9 ]/', '', $value);
         $len = strlen($str);
-        if ($str{0} === '(' && $str{$len - 1} === ')') {
+        if ('(' === $str[0] && ')' === $str[$len - 1]) {
             $str = '-' . substr($str, 1, ($len - 2));
         }
 
         Log::debug(sprintf('Stripped "%s" away to "%s"', $value, $str));
 
         return $str;
-
     }
 }

app/Import/FileProcessor/CsvProcessor.php

@@ -62,6 +62,7 @@ class CsvProcessor implements FileProcessorInterface
 
     /**
      * @return Collection
+     *
      * @throws FireflyException
      */
     public function getObjects(): Collection
@@ -192,6 +193,7 @@ class CsvProcessor implements FileProcessorInterface
      * Shorthand method to return configuration.
      *
      * @codeCoverageIgnore
+     *
      * @return array
      */
     private function getConfig(): array
@@ -237,6 +239,7 @@ class CsvProcessor implements FileProcessorInterface
      * @param int $jsonError
      *
      * @codeCoverageIgnore
+     *
      * @return string
      */
     private function getJsonError(int $jsonError): string

app/Import/Object/ImportAccount.php

@@ -27,7 +27,6 @@ use FireflyIII\Models\Account;
 use FireflyIII\Models\AccountType;
 use FireflyIII\Repositories\Account\AccountRepositoryInterface;
 use FireflyIII\User;
-use Illuminate\Support\Collection;
 use Log;
 
 /**
@@ -70,18 +69,18 @@ class ImportAccount
     public function __construct()
     {
         $this->expectedType = AccountType::ASSET;
-        $this->account      = new Account;
         $this->repository   = app(AccountRepositoryInterface::class);
         Log::debug('Created ImportAccount.');
     }
 
     /**
      * @return Account
+     *
      * @throws FireflyException
      */
     public function getAccount(): Account
     {
-        if (null === $this->account->id) {
+        if (null === $this->account) {
             $this->store();
         }
 
@@ -90,6 +89,7 @@ class ImportAccount
 
     /**
      * @codeCoverageIgnore
+     *
      * @return string
      */
     public function getExpectedType(): string
@@ -187,43 +187,26 @@ class ImportAccount
     }
 
     /**
+     * Find account by IBAN and type.
+     *
+     * @param AccountType $type
+     *
      * @return Account|null
-     * @SuppressWarnings(PHPMD.CyclomaticComplexity)
      */
-    private function findExistingObject(): ?Account
+    private function findByIBAN(AccountType $type): ?Account
     {
-        Log::debug('In findExistingObject() for Account');
-        // 0: determin account type:
-        /** @var AccountType $accountType */
-        $accountType = $this->repository->getAccountType($this->expectedType);
-
-        // 1: find by ID, iban or name (and type)
-        if (3 === count($this->accountId)) {
-            Log::debug(sprintf('Finding account of type %d and ID %d', $accountType->id, $this->accountId['value']));
-            /** @var Account $account */
-
-            $account = $this->user->accounts()->where('id', '!=', $this->forbiddenAccountId)->where('account_type_id', $accountType->id)->where(
-                'id',
-                $this->accountId['value']
-            )->first();
-            if (null !== $account) {
-                Log::debug(sprintf('Found unmapped %s account by ID (#%d): %s', $this->expectedType, $account->id, $account->name));
-
-                return $account;
-            }
-            Log::debug('Found nothing.');
-        }
-        /** @var Collection $accounts */
-        $accounts = $this->repository->getAccountsByType([$accountType->type]);
-        // Two: find by IBAN (and type):
         if (3 === count($this->accountIban)) {
+            $accounts = $this->repository->getAccountsByType([$type->type]);
             $iban     = $this->accountIban['value'];
-            Log::debug(sprintf('Finding account of type %d and IBAN %s', $accountType->id, $iban));
+            Log::debug(sprintf('Finding account of type %d and IBAN %s', $type->id, $iban));
             $filtered = $accounts->filter(
                 function (Account $account) use ($iban) {
                     if ($account->iban === $iban && $account->id !== $this->forbiddenAccountId) {
                         Log::debug(
-                            sprintf('Found unmapped %s account by IBAN (#%d): %s (%s)', $this->expectedType, $account->id, $account->name, $account->iban)
+                            sprintf(
+                                'Found unmapped %s account by IBAN (#%d): %s (%s)',
+                                $this->expectedType, $account->id, $account->name, $account->iban
+                            )
                         );
 
                         return $account;
@@ -238,10 +221,52 @@ class ImportAccount
             Log::debug('Found nothing.');
         }
 
+        return null;
+    }
+
+    /**
+     * Find account of type X by its ID.
+     *
+     * @param AccountType $type
+     *
+     * @return Account|null
+     */
+    private function findById(AccountType $type): ?Account
+    {
+        if (3 === count($this->accountId)) {
+            Log::debug(sprintf('Finding account of type %d and ID %d', $type->id, $this->accountId['value']));
+            /** @var Account $account */
+            $account = $this->user->accounts()
+                                  ->where('id', '!=', $this->forbiddenAccountId)
+                                  ->where('account_type_id', $type->id)
+                                  ->where('id', $this->accountId['value'])
+                                  ->first();
+
+            if (null !== $account) {
+                Log::debug(sprintf('Found unmapped %s account by ID (#%d): %s', $this->expectedType, $account->id, $account->name));
+
+                return $account;
+            }
+            Log::debug('Found nothing.');
+        }
+
+        return null;
+    }
+
+    /**
+     * Find account by account type and name.
+     *
+     * @param AccountType $type
+     *
+     * @return Account|null
+     */
+    private function findByName(AccountType $type): ?Account
+    {
         // Three: find by name (and type):
         if (3 === count($this->accountName)) {
+            $accounts = $this->repository->getAccountsByType([$type->type]);
             $name     = $this->accountName['value'];
-            Log::debug(sprintf('Finding account of type %d and name %s', $accountType->id, $name));
+            Log::debug(sprintf('Finding account of type %d and name %s', $type->id, $name));
             $filtered = $accounts->filter(
                 function (Account $account) use ($name) {
                     if ($account->name === $name && $account->id !== $this->forbiddenAccountId) {
@@ -260,7 +285,36 @@ class ImportAccount
             Log::debug('Found nothing.');
         }
 
-        // 4: do not search by account number.
+        return null;
+    }
+
+    /**
+     * Determin account type to find, then use fields in object to try and find it.
+     *
+     * @return Account|null
+     */
+    private function findExistingObject(): ?Account
+    {
+        Log::debug('In findExistingObject() for Account');
+        /** @var AccountType $accountType */
+        $accountType = $this->repository->getAccountType($this->expectedType);
+        $result      = $this->findById($accountType);
+        if (!is_null($result)) {
+            return $result;
+        }
+
+        $result = $this->findByIBAN($accountType);
+
+        if (!is_null($result)) {
+            return $result;
+        }
+
+        $result = $this->findByName($accountType);
+
+        if (!is_null($result)) {
+            return $result;
+        }
+
         Log::debug('Found NO existing accounts.');
 
         return null;
@@ -342,6 +396,7 @@ class ImportAccount
 
     /**
      * @return bool
+     *
      * @throws FireflyException
      */
     private function store(): bool
@@ -349,7 +404,7 @@ class ImportAccount
         if (is_null($this->user)) {
             throw new FireflyException('ImportAccount cannot continue without user.');
         }
-        if ((is_null($this->defaultAccountId) || intval($this->defaultAccountId) === 0) && AccountType::ASSET === $this->expectedType) {
+        if ((is_null($this->defaultAccountId) || 0 === intval($this->defaultAccountId)) && AccountType::ASSET === $this->expectedType) {
             throw new FireflyException('ImportAccount cannot continue without a default account to fall back on.');
         }
         // 1: find mapped object:

app/Import/Object/ImportBill.php

@@ -25,7 +25,6 @@ namespace FireflyIII\Import\Object;
 use FireflyIII\Models\Bill;
 use FireflyIII\Repositories\Bill\BillRepositoryInterface;
 use FireflyIII\User;
-use Illuminate\Support\Collection;
 use Log;
 use Steam;
 
@@ -52,7 +51,6 @@ class ImportBill
      */
     public function __construct()
     {
-        $this->bill       = new Bill;
         $this->repository = app(BillRepositoryInterface::class);
         Log::debug('Created ImportBill.');
     }
@@ -62,7 +60,7 @@ class ImportBill
      */
     public function getBill(): Bill
     {
-        if (null === $this->bill->id) {
+        if (null === $this->bill) {
             $this->store();
         }
 
@@ -103,13 +101,10 @@ class ImportBill
     }
 
     /**
-     * @return Bill
+     * @return Bill|null
      */
-    private function findExistingObject(): Bill
+    private function findById(): ?Bill
     {
-        Log::debug('In findExistingObject() for Bill');
-        // 1: find by ID, or name
-
         if (3 === count($this->id)) {
             Log::debug(sprintf('Finding bill with ID #%d', $this->id['value']));
             /** @var Bill $bill */
@@ -121,9 +116,16 @@ class ImportBill
             }
             Log::debug('Found nothing.');
         }
-        // 2: find by name
+
+        return null;
+    }
+
+    /**
+     * @return Bill|null
+     */
+    private function findByName(): ?Bill
+    {
         if (3 === count($this->name)) {
-            /** @var Collection $bills */
             $bills = $this->repository->getBills();
             $name  = $this->name['value'];
             Log::debug(sprintf('Finding bill with name %s', $name));
@@ -145,16 +147,33 @@ class ImportBill
             Log::debug('Found nothing.');
         }
 
-        // 4: do not search by account number.
+        return null;
-        Log::debug('Found NO existing bills.');
-
-        return new Bill;
     }
 
     /**
-     * @return Bill
+     * @return Bill|null
      */
-    private function findMappedObject(): Bill
+    private function findExistingObject(): ?Bill
+    {
+        Log::debug('In findExistingObject() for Bill');
+        $result = $this->findById();
+        if (!is_null($result)) {
+            return $result;
+        }
+        $result = $this->findByName();
+        if (!is_null($result)) {
+            return $result;
+        }
+
+        Log::debug('Found NO existing bills.');
+
+        return null;
+    }
+
+    /**
+     * @return Bill|null
+     */
+    private function findMappedObject(): ?Bill
     {
         Log::debug('In findMappedObject() for Bill');
         $fields = ['id', 'name'];
@@ -163,7 +182,7 @@ class ImportBill
             Log::debug(sprintf('Find mapped bill based on field "%s" with value', $field), $array);
             // check if a pre-mapped object exists.
             $mapped = $this->getMappedObject($array);
-            if (null !== $mapped->id) {
+            if (null !== $mapped) {
                 Log::debug(sprintf('Found bill #%d!', $mapped->id));
 
                 return $mapped;
@@ -171,7 +190,7 @@ class ImportBill
         }
         Log::debug('Found no bill on mapped data or no map present.');
 
-        return new Bill;
+        return null;
     }
 
     /**
@@ -179,19 +198,19 @@ class ImportBill
      *
      * @return Bill
      */
-    private function getMappedObject(array $array): Bill
+    private function getMappedObject(array $array): ?Bill
     {
         Log::debug('In getMappedObject() for Bill');
         if (0 === count($array)) {
             Log::debug('Array is empty, nothing will come of this.');
 
-            return new Bill;
+            return null;
         }
 
         if (array_key_exists('mapped', $array) && null === $array['mapped']) {
             Log::debug(sprintf('No map present for value "%s". Return NULL.', $array['value']));
 
-            return new Bill;
+            return null;
         }
 
         Log::debug('Finding a mapped bill based on', $array);
@@ -202,7 +221,7 @@ class ImportBill
         if (null === $bill) {
             Log::error(sprintf('There is no bill with id #%d. Invalid mapping will be ignored!', $search));
 
-            return new Bill;
+            return null;
         }
 
         Log::debug(sprintf('Found bill! #%d ("%s"). Return it', $bill->id, $bill->name));
@@ -217,14 +236,14 @@ class ImportBill
     {
         // 1: find mapped object:
         $mapped = $this->findMappedObject();
-        if (null !== $mapped->id) {
+        if (null !== $mapped) {
             $this->bill = $mapped;
 
             return true;
         }
         // 2: find existing by given values:
         $found = $this->findExistingObject();
-        if (null !== $found->id) {
+        if (null !== $found) {
             $this->bill = $found;
 
             return true;

app/Import/Object/ImportBudget.php

@@ -25,7 +25,6 @@ namespace FireflyIII\Import\Object;
 use FireflyIII\Models\Budget;
 use FireflyIII\Repositories\Budget\BudgetRepositoryInterface;
 use FireflyIII\User;
-use Illuminate\Support\Collection;
 use Log;
 
 /**
@@ -49,7 +48,6 @@ class ImportBudget
      */
     public function __construct()
     {
-        $this->budget     = new Budget;
         $this->repository = app(BudgetRepositoryInterface::class);
         Log::debug('Created ImportBudget.');
     }
@@ -59,7 +57,7 @@ class ImportBudget
      */
     public function getBudget(): Budget
     {
-        if (null === $this->budget->id) {
+        if (null === $this->budget) {
             $this->store();
         }
 
@@ -92,27 +90,31 @@ class ImportBudget
     }
 
     /**
-     * @return Budget
+     * @return Budget|null
      */
-    private function findExistingObject(): Budget
+    private function findById(): ?Budget
     {
-        Log::debug('In findExistingObject() for Budget');
-        // 1: find by ID, or name
-
         if (3 === count($this->id)) {
             Log::debug(sprintf('Finding budget with ID #%d', $this->id['value']));
             /** @var Budget $budget */
-            $budget = $this->repository->find(intval($this->id['value']));
+            $budget = $this->repository->findNull(intval($this->id['value']));
-            if (null !== $budget->id) {
+            if (null !== $budget) {
                 Log::debug(sprintf('Found unmapped budget by ID (#%d): %s', $budget->id, $budget->name));
 
                 return $budget;
             }
             Log::debug('Found nothing.');
         }
-        // 2: find by name
+
+        return null;
+    }
+
+    /**
+     * @return Budget|null
+     */
+    private function findByName(): ?Budget
+    {
         if (3 === count($this->name)) {
-            /** @var Collection $budgets */
             $budgets = $this->repository->getBudgets();
             $name    = $this->name['value'];
             Log::debug(sprintf('Finding budget with name %s', $name));
@@ -134,16 +136,33 @@ class ImportBudget
             Log::debug('Found nothing.');
         }
 
-        // 4: do not search by account number.
+        return null;
-        Log::debug('Found NO existing budgets.');
-
-        return new Budget;
     }
 
     /**
      * @return Budget
      */
-    private function findMappedObject(): Budget
+    private function findExistingObject(): ?Budget
+    {
+        Log::debug('In findExistingObject() for Budget');
+        $result = $this->findById();
+        if (!is_null($result)) {
+            return $result;
+        }
+        $result = $this->findByName();
+        if (!is_null($result)) {
+            return $result;
+        }
+
+        Log::debug('Found NO existing budgets.');
+
+        return null;
+    }
+
+    /**
+     * @return Budget
+     */
+    private function findMappedObject(): ?Budget
     {
         Log::debug('In findMappedObject() for Budget');
         $fields = ['id', 'name'];
@@ -160,7 +179,7 @@ class ImportBudget
         }
         Log::debug('Found no budget on mapped data or no map present.');
 
-        return new Budget;
+        return null;
     }
 
     /**
@@ -168,19 +187,19 @@ class ImportBudget
      *
      * @return Budget
      */
-    private function getMappedObject(array $array): Budget
+    private function getMappedObject(array $array): ?Budget
     {
         Log::debug('In getMappedObject() for Budget');
         if (0 === count($array)) {
             Log::debug('Array is empty, nothing will come of this.');
 
-            return new Budget;
+            return null;
         }
 
         if (array_key_exists('mapped', $array) && null === $array['mapped']) {
             Log::debug(sprintf('No map present for value "%s". Return NULL.', $array['value']));
 
-            return new Budget;
+            return null;
         }
 
         Log::debug('Finding a mapped budget based on', $array);
@@ -191,7 +210,7 @@ class ImportBudget
         if (null === $budget->id) {
             Log::error(sprintf('There is no budget with id #%d. Invalid mapping will be ignored!', $search));
 
-            return new Budget;
+            return null;
         }
 
         Log::debug(sprintf('Found budget! #%d ("%s"). Return it', $budget->id, $budget->name));
@@ -206,14 +225,14 @@ class ImportBudget
     {
         // 1: find mapped object:
         $mapped = $this->findMappedObject();
-        if (null !== $mapped->id) {
+        if (null !== $mapped) {
             $this->budget = $mapped;
 
             return true;
         }
         // 2: find existing by given values:
         $found = $this->findExistingObject();
-        if (null !== $found->id) {
+        if (null !== $found) {
             $this->budget = $found;
 
             return true;

app/Import/Object/ImportCategory.php

@@ -25,7 +25,6 @@ namespace FireflyIII\Import\Object;
 use FireflyIII\Models\Category;
 use FireflyIII\Repositories\Category\CategoryRepositoryInterface;
 use FireflyIII\User;
-use Illuminate\Support\Collection;
 use Log;
 
 /**
@@ -49,7 +48,6 @@ class ImportCategory
      */
     public function __construct()
     {
-        $this->category   = new Category();
         $this->repository = app(CategoryRepositoryInterface::class);
         Log::debug('Created ImportCategory.');
     }
@@ -59,7 +57,7 @@ class ImportCategory
      */
     public function getCategory(): Category
     {
-        if (null === $this->category->id) {
+        if (null === $this->category) {
             $this->store();
         }
 
@@ -92,27 +90,35 @@ class ImportCategory
     }
 
     /**
-     * @return Category
+     * Find category by ID.
+     *
+     * @return Category|null
      */
-    private function findExistingObject(): Category
+    private function findById(): ?Category
     {
-        Log::debug('In findExistingObject() for Category');
-        // 1: find by ID, or name
-
         if (3 === count($this->id)) {
             Log::debug(sprintf('Finding category with ID #%d', $this->id['value']));
             /** @var Category $category */
-            $category = $this->repository->find(intval($this->id['value']));
+            $category = $this->repository->findNull(intval($this->id['value']));
-            if (null !== $category->id) {
+            if (null !== $category) {
                 Log::debug(sprintf('Found unmapped category by ID (#%d): %s', $category->id, $category->name));
 
                 return $category;
             }
             Log::debug('Found nothing.');
         }
-        // 2: find by name
+
+        return null;
+    }
+
+    /**
+     * Find category by name.
+     *
+     * @return Category|null
+     */
+    private function findByName(): ?Category
+    {
         if (3 === count($this->name)) {
-            /** @var Collection $categories */
             $categories = $this->repository->getCategories();
             $name       = $this->name['value'];
             Log::debug(sprintf('Finding category with name %s', $name));
@@ -134,16 +140,34 @@ class ImportCategory
             Log::debug('Found nothing.');
         }
 
-        // 4: do not search by account number.
+        return null;
-        Log::debug('Found NO existing categories.');
-
-        return new Category;
     }
 
     /**
      * @return Category
      */
-    private function findMappedObject(): Category
+    private function findExistingObject(): ?Category
+    {
+        Log::debug('In findExistingObject() for Category');
+        $result = $this->findById();
+        if (!is_null($result)) {
+            return $result;
+        }
+
+        $result = $this->findByName();
+        if (!is_null($result)) {
+            return $result;
+        }
+
+        Log::debug('Found NO existing categories.');
+
+        return null;
+    }
+
+    /**
+     * @return Category
+     */
+    private function findMappedObject(): ?Category
     {
         Log::debug('In findMappedObject() for Category');
         $fields = ['id', 'name'];
@@ -152,7 +176,7 @@ class ImportCategory
             Log::debug(sprintf('Find mapped category based on field "%s" with value', $field), $array);
             // check if a pre-mapped object exists.
             $mapped = $this->getMappedObject($array);
-            if (null !== $mapped->id) {
+            if (null !== $mapped) {
                 Log::debug(sprintf('Found category #%d!', $mapped->id));
 
                 return $mapped;
@@ -160,7 +184,7 @@ class ImportCategory
         }
         Log::debug('Found no category on mapped data or no map present.');
 
-        return new Category;
+        return null;
     }
 
     /**
@@ -174,24 +198,24 @@ class ImportCategory
         if (0 === count($array)) {
             Log::debug('Array is empty, nothing will come of this.');
 
-            return new Category;
+            return null;
         }
 
         if (array_key_exists('mapped', $array) && null === $array['mapped']) {
             Log::debug(sprintf('No map present for value "%s". Return NULL.', $array['value']));
 
-            return new Category;
+            return null;
         }
 
         Log::debug('Finding a mapped category based on', $array);
 
         $search   = intval($array['mapped']);
-        $category = $this->repository->find($search);
+        $category = $this->repository->findNull($search);
 
-        if (null === $category->id) {
+        if (null === $category) {
             Log::error(sprintf('There is no category with id #%d. Invalid mapping will be ignored!', $search));
 
-            return new Category;
+            return null;
         }
 
         Log::debug(sprintf('Found category! #%d ("%s"). Return it', $category->id, $category->name));
@@ -206,14 +230,14 @@ class ImportCategory
     {
         // 1: find mapped object:
         $mapped = $this->findMappedObject();
-        if (null !== $mapped->id) {
+        if (null !== $mapped) {
             $this->category = $mapped;
 
             return true;
         }
         // 2: find existing by given values:
         $found = $this->findExistingObject();
-        if (null !== $found->id) {
+        if (null !== $found) {
             $this->category = $found;
 
             return true;
@@ -226,11 +250,7 @@ class ImportCategory
 
         Log::debug('Found no category so must create one ourselves.');
 
-        $data = [
+        $this->category = $this->repository->store(['name' => $name]);
-            'name' => $name,
-        ];
-
-        $this->category = $this->repository->store($data);
         Log::debug(sprintf('Successfully stored new category #%d: %s', $this->category->id, $this->category->name));
 
         return true;

app/Import/Object/ImportCurrency.php

@@ -52,23 +52,22 @@ class ImportCurrency
      */
     public function __construct()
     {
-        $this->currency   = new TransactionCurrency;
         $this->repository = app(CurrencyRepositoryInterface::class);
     }
 
     /**
      * @return TransactionCurrency
      */
-    public function getTransactionCurrency(): TransactionCurrency
+    public function getTransactionCurrency(): ?TransactionCurrency
     {
-        if (null !== $this->currency->id) {
+        if (null !== $this->currency) {
             return $this->currency;
         }
         Log::debug('In createCurrency()');
         // check if any of them is mapped:
         $mapped = $this->findMappedObject();
 
-        if (null !== $mapped->id) {
+        if (null !== $mapped) {
             Log::debug('Mapped existing currency.', ['new' => $mapped->toArray()]);
             $this->currency = $mapped;
 
@@ -76,7 +75,7 @@ class ImportCurrency
         }
 
         $searched = $this->findExistingObject();
-        if (null !== $searched->id) {
+        if (null !== $searched) {
             Log::debug('Found existing currency.', ['found' => $searched->toArray()]);
             $this->currency = $searched;
 
@@ -91,7 +90,7 @@ class ImportCurrency
         if (null === $data['code']) {
             Log::debug('Need at least a code to create currency, return nothing.');
 
-            return new TransactionCurrency();
+            return null;
         }
 
         Log::debug('Search for maps resulted in nothing, create new one based on', $data);
@@ -147,33 +146,34 @@ class ImportCurrency
     /**
      * @return TransactionCurrency
      */
-    private function findExistingObject(): TransactionCurrency
+    private function findExistingObject(): ?TransactionCurrency
     {
         $search = [
-            'id'     => 'find',
+            'id'     => 'findNull',
-            'code'   => 'findByCode',
+            'code'   => 'findByCodeNull',
-            'symbol' => 'findBySymbol',
+            'symbol' => 'findBySymbolNull',
-            'name'   => 'findByName',
+            'name'   => 'findByNameNull',
         ];
         foreach ($search as $field => $function) {
             $value = $this->$field['value'] ?? null;
             if (null !== $value) {
                 Log::debug(sprintf('Searching for %s using function %s and value %s', $field, $function, $value));
+                /** @var TransactionCurrency|null $currency */
                 $currency = $this->repository->$function($value);
 
-                if (null !== $currency->id) {
+                if (null !== $currency) {
                     return $currency;
                 }
             }
         }
 
-        return new TransactionCurrency();
+        return null;
     }
 
     /**
      * @return TransactionCurrency
      */
-    private function findMappedObject(): TransactionCurrency
+    private function findMappedObject(): ?TransactionCurrency
     {
         Log::debug('In findMappedObject()');
         $fields = ['id', 'code', 'name', 'symbol'];
@@ -182,7 +182,7 @@ class ImportCurrency
             Log::debug(sprintf('Find mapped currency based on field "%s" with value', $field), $array);
             // check if a pre-mapped object exists.
             $mapped = $this->getMappedObject($array);
-            if (null !== $mapped->id) {
+            if (null !== $mapped) {
                 Log::debug(sprintf('Found currency #%d!', $mapped->id));
 
                 return $mapped;
@@ -190,7 +190,7 @@ class ImportCurrency
         }
         Log::debug('Found no currency on mapped data or no map present.');
 
-        return new TransactionCurrency;
+        return null;
     }
 
     /**
@@ -198,30 +198,30 @@ class ImportCurrency
      *
      * @return TransactionCurrency
      */
-    private function getMappedObject(array $array): TransactionCurrency
+    private function getMappedObject(array $array): ?TransactionCurrency
     {
         Log::debug('In getMappedObject()');
         if (0 === count($array)) {
             Log::debug('Array is empty, nothing will come of this.');
 
-            return new TransactionCurrency;
+            return null;
         }
 
         if (array_key_exists('mapped', $array) && null === $array['mapped']) {
             Log::debug(sprintf('No map present for value "%s". Return NULL.', $array['value']));
 
-            return new TransactionCurrency;
+            return null;
         }
 
         Log::debug('Finding a mapped object based on', $array);
 
         $search   = intval($array['mapped']);
-        $currency = $this->repository->find($search);
+        $currency = $this->repository->findNull($search);
 
-        if (null === $currency->id) {
+        if (null === $currency) {
             Log::error(sprintf('There is no currency with id #%d. Invalid mapping will be ignored!', $search));
 
-            return new TransactionCurrency;
+            return null;
         }
 
         Log::debug(sprintf('Found currency! #%d ("%s"). Return it', $currency->id, $currency->name));

app/Import/Object/ImportJournal.php

@@ -339,7 +339,7 @@ class ImportJournal
             throw new FireflyException('No amount information for this row.');
         }
         $class = $info['class'] ?? '';
-        if (strlen($class) === 0) {
+        if (0 === strlen($class)) {
             throw new FireflyException('No amount information (conversion class) for this row.');
         }
 

app/Import/Prerequisites/BunqPrerequisites.php

@@ -114,5 +114,4 @@ class BunqPrerequisites implements PrerequisitesInterface
 
         return new MessageBag;
     }
-
 }

app/Import/Prerequisites/FilePrerequisites.php

@@ -63,6 +63,7 @@ class FilePrerequisites implements PrerequisitesInterface
      * True if prerequisites. False if not.
      *
      * @return bool
+     *
      * @throws FireflyException
      */
     public function hasPrerequisites(): bool

app/Import/Routine/BunqRoutine.php

@@ -71,8 +71,6 @@ use Requests;
  * Map accounts to existing accounts
  *
  * Stage 'do-import'?
- *
- *
  */
 class BunqRoutine implements RoutineInterface
 {
@@ -122,8 +120,8 @@ class BunqRoutine implements RoutineInterface
     }
 
     /**
-     *
      * @return bool
+     *
      * @throws FireflyException
      */
     public function run(): bool
@@ -153,7 +151,7 @@ class BunqRoutine implements RoutineInterface
     protected function continueJob()
     {
         // if in "configuring"
-        if ($this->getStatus() === 'configuring') {
+        if ('configuring' === $this->getStatus()) {
             Log::debug('Job is in configuring stage, will do nothing.');
 
             return;
@@ -203,7 +201,6 @@ class BunqRoutine implements RoutineInterface
     }
 
     /**
-     *
      * @throws FireflyException
      */
     protected function runStageInitial()
@@ -372,6 +369,7 @@ class BunqRoutine implements RoutineInterface
      * Get the installation token, either from the users preferences or from Bunq.
      *
      * @return InstallationToken
+     *
      * @throws FireflyException
      */
     private function getInstallationToken(): InstallationToken
@@ -477,6 +475,7 @@ class BunqRoutine implements RoutineInterface
      * Get the public key of the server, necessary to verify server signature.
      *
      * @return ServerPublicKey
+     *
      * @throws FireflyException
      */
     private function getServerPublicKey(): ServerPublicKey
@@ -575,14 +574,14 @@ class BunqRoutine implements RoutineInterface
         $mapping = $config['accounts-mapped'];
         $token   = new SessionToken($config['session_token']);
         $count   = 0;
-        if ($user->getId() === 0) {
+        if (0 === $user->getId()) {
             $user = new UserCompany($config['user_company']);
         }
 
         foreach ($config['accounts'] as $accountData) {
             $account  = new MonetaryAccountBank($accountData);
             $importId = $account->getId();
-            if ($mapping[$importId] === 1) {
+            if (1 === $mapping[$importId]) {
                 // grab all transactions
                 $request = new ListPaymentRequest();
                 $request->setPrivateKey($this->getPrivateKey());
@@ -610,7 +609,6 @@ class BunqRoutine implements RoutineInterface
     }
 
     /**
-     *
      * @throws FireflyException
      */
     private function runStageLoggedIn(): void
@@ -619,7 +617,7 @@ class BunqRoutine implements RoutineInterface
         $config = $this->getConfig();
         $token  = new SessionToken($config['session_token']);
         $user   = new UserPerson($config['user_person']);
-        if ($user->getId() === 0) {
+        if (0 === $user->getId()) {
             $user = new UserCompany($config['user_company']);
         }
 

app/Import/Routine/FileRoutine.php

@@ -113,7 +113,6 @@ class FileRoutine implements RoutineInterface
         $this->addStep();
         Log::debug('Back in run()');
 
-
         Log::debug('Updated job...');
         Log::debug(sprintf('%d journals in $storage->journals', $storage->journals->count()));
         $this->journals = $storage->journals;

app/Import/Routine/SpectreRoutine.php

@@ -118,6 +118,7 @@ class SpectreRoutine implements RoutineInterface
      *
      *
      * @return bool
+     *
      * @throws FireflyException
      * @throws SpectreException
      * @throws \Illuminate\Container\EntryNotFoundException
@@ -166,6 +167,7 @@ class SpectreRoutine implements RoutineInterface
 
     /**
      * @return Customer
+     *
      * @throws \FireflyIII\Exceptions\FireflyException
      * @throws \FireflyIII\Services\Spectre\Exception\SpectreException
      * @throws \Illuminate\Container\EntryNotFoundException
@@ -187,22 +189,21 @@ class SpectreRoutine implements RoutineInterface
             $customers = $getCustomerRequest->getCustomers();
             /** @var Customer $current */
             foreach ($customers as $current) {
-                if ($current->getIdentifier() === 'default_ff3_customer') {
+                if ('default_ff3_customer' === $current->getIdentifier()) {
                     $customer = $current;
                     break;
                 }
             }
         }
 
-
         Preferences::setForUser($this->job->user, 'spectre_customer', $customer->toArray());
 
         return $customer;
-
     }
 
     /**
      * @return Customer
+     *
      * @throws FireflyException
      * @throws SpectreException
      * @throws \Illuminate\Container\EntryNotFoundException
@@ -232,6 +233,7 @@ class SpectreRoutine implements RoutineInterface
      * @param string   $returnUri
      *
      * @return Token
+     *
      * @throws \FireflyIII\Exceptions\FireflyException
      * @throws \FireflyIII\Services\Spectre\Exception\SpectreException
      * @throws \Illuminate\Container\EntryNotFoundException
@@ -245,7 +247,6 @@ class SpectreRoutine implements RoutineInterface
         Log::debug('Call to get token is finished');
 
         return $request->getToken();
-
     }
 
     /**
@@ -460,12 +461,10 @@ class SpectreRoutine implements RoutineInterface
                 // date:
                 $importJournal->setValue(['role' => 'date-transaction', 'value' => $transaction->getMadeOn()->toIso8601String()]);
 
-
                 // amount
                 $importJournal->setValue(['role' => 'amount', 'value' => $transaction->getAmount()]);
                 $importJournal->setValue(['role' => 'currency-code', 'value' => $transaction->getCurrencyCode()]);
 
-
                 // various meta fields:
                 $importJournal->setValue(['role' => 'category-name', 'value' => $transaction->getCategory()]);
                 $importJournal->setValue(['role' => 'note', 'value' => $notes]);
@@ -540,7 +539,7 @@ class SpectreRoutine implements RoutineInterface
         foreach ($accounts as $accountArray) {
             $account  = new Account($accountArray);
             $importId = intval($config['accounts-mapped'][$account->getid()] ?? 0);
-            $doImport = $importId !== 0 ? true : false;
+            $doImport = 0 !== $importId ? true : false;
             if (!$doImport) {
                 Log::debug(sprintf('Will NOT import from Spectre account #%d ("%s")', $account->getId(), $account->getName()));
                 continue;
@@ -560,7 +559,6 @@ class SpectreRoutine implements RoutineInterface
         Log::debug(sprintf('Total number of transactions: %d', $count));
         $this->addStep();
 
-
         $this->importTransactions($all);
     }
 

app/Import/Storage/ImportStorage.php

@@ -132,8 +132,6 @@ class ImportStorage
         }
         Log::debug(sprintf('Value of apply rules is %s', var_export($this->applyRules, true)));
         Log::debug(sprintf('Value of match bills is %s', var_export($this->matchBills, true)));
-
-
     }
 
     /**

app/Import/Storage/ImportSupport.php

@@ -127,7 +127,7 @@ trait ImportSupport
         $transaction->transaction_journal_id  = intval($parameters['id']);
         $transaction->transaction_currency_id = intval($parameters['currency']);
         $transaction->amount                  = $parameters['amount'];
-        $transaction->foreign_currency_id     = intval($parameters['foreign_currency']) === 0 ? null : intval($parameters['foreign_currency']);
+        $transaction->foreign_currency_id     = 0 === intval($parameters['foreign_currency']) ? null : intval($parameters['foreign_currency']);
         $transaction->foreign_amount          = null === $transaction->foreign_currency_id ? null : $parameters['foreign_amount'];
         $transaction->save();
         if (null === $transaction->id) {
@@ -158,6 +158,7 @@ trait ImportSupport
      * @param ImportJournal $importJournal
      *
      * @return int
+     *
      * @throws FireflyException
      */
     private function getCurrencyId(ImportJournal $importJournal): int
@@ -171,7 +172,7 @@ trait ImportSupport
 
         // use given currency
         $currency = $importJournal->currency->getTransactionCurrency();
-        if (null !== $currency->id) {
+        if (null !== $currency) {
             return $currency->id;
         }
 
@@ -196,7 +197,7 @@ trait ImportSupport
     {
         // use given currency by import journal.
         $currency = $importJournal->currency->getTransactionCurrency();
-        if (null !== $currency->id && intval($currency->id) !== intval($currencyId)) {
+        if (null !== $currency && intval($currency->id) !== intval($currencyId)) {
             return $currency->id;
         }
 
@@ -220,6 +221,7 @@ trait ImportSupport
      * @see ImportSupport::getTransactionType
      *
      * @return Account
+     *
      * @throws FireflyException
      */
     private function getOpposingAccount(ImportAccount $account, int $forbiddenAccount, string $amount): Account

app/Repositories/Category/CategoryRepository.php

@@ -23,6 +23,7 @@ declare(strict_types=1);
 namespace FireflyIII\Repositories\Category;
 
 use Carbon\Carbon;
+use FireflyIII\Factory\CategoryFactory;
 use FireflyIII\Helpers\Collector\JournalCollectorInterface;
 use FireflyIII\Models\Category;
 use FireflyIII\Models\Transaction;
@@ -41,6 +42,8 @@ class CategoryRepository implements CategoryRepositoryInterface
     private $user;
 
     /**
+     * TODO move to delete service
+     *
      * @param Category $category
      *
      * @return bool
@@ -444,18 +447,17 @@ class CategoryRepository implements CategoryRepositoryInterface
      */
     public function store(array $data): Category
     {
-        $newCategory = Category::firstOrCreateEncrypted(
+        /** @var CategoryFactory $factory */
-            [
+        $factory = app(CategoryFactory::class);
-                'user_id' => $this->user->id,
+        $factory->setUser($this->user);
-                'name'    => $data['name'],
+        $category = $factory->findOrCreate(null, $data['name']);
-            ]
-        );
-        $newCategory->save();
 
-        return $newCategory;
+        return $category;
     }
 
     /**
+     * TODO move to update service
+     *
      * @param Category $category
      * @param array    $data
      *

app/Repositories/Currency/CurrencyRepository.php

@@ -89,6 +89,8 @@ class CurrencyRepository implements CurrencyRepositoryInterface
     }
 
     /**
+     * TODO use service
+     *
      * @param TransactionCurrency $currency
      *
      * @return bool
@@ -107,6 +109,8 @@ class CurrencyRepository implements CurrencyRepositoryInterface
      *
      * @param int $currencyId
      *
+     * @deprecated
+     *
      * @return TransactionCurrency
      */
     public function find(int $currencyId): TransactionCurrency
@@ -122,6 +126,8 @@ class CurrencyRepository implements CurrencyRepositoryInterface
     /**
      * Find by currency code.
      *
+     * @deprecated
+     *
      * @param string $currencyCode
      *
      * @return TransactionCurrency
@@ -138,6 +144,7 @@ class CurrencyRepository implements CurrencyRepositoryInterface
 
     /**
      * Find by currency code, return NULL if unfound.
+     * Used in Import Currency!
      *
      * @param string $currencyCode
      *
@@ -153,6 +160,8 @@ class CurrencyRepository implements CurrencyRepositoryInterface
      *
      * @param string $currencyName
      *
+     * @deprecated
+     *
      * @return TransactionCurrency
      */
     public function findByName(string $currencyName): TransactionCurrency
@@ -165,9 +174,24 @@ class CurrencyRepository implements CurrencyRepositoryInterface
         return $preferred;
     }
 
+    /**
+     * Find by currency name or return null.
+     * Used in Import Currency!
+     *
+     * @param string $currencyName
+     *
+     * @return TransactionCurrency
+     */
+    public function findByNameNull(string $currencyName): ?TransactionCurrency
+    {
+        return TransactionCurrency::whereName($currencyName)->first();
+    }
+
     /**
      * Find by currency symbol.
      *
+     * @deprecated
+     *
      * @param string $currencySymbol
      *
      * @return TransactionCurrency
@@ -182,8 +206,22 @@ class CurrencyRepository implements CurrencyRepositoryInterface
         return $currency;
     }
 
+    /**
+     * Find by currency symbol or return NULL
+     * Used in Import Currency!
+     *
+     * @param string $currencySymbol
+     *
+     * @return TransactionCurrency
+     */
+    public function findBySymbolNull(string $currencySymbol): ?TransactionCurrency
+    {
+        return TransactionCurrency::whereSymbol($currencySymbol)->first();
+    }
+
     /**
      * Find by ID, return NULL if not found.
+     * Used in Import Currency!
      *
      * @param int $currencyId
      *
@@ -269,6 +307,8 @@ class CurrencyRepository implements CurrencyRepositoryInterface
     }
 
     /**
+     * TODO use factory
+     *
      * @param array $data
      *
      * @return TransactionCurrency
@@ -289,6 +329,8 @@ class CurrencyRepository implements CurrencyRepositoryInterface
     }
 
     /**
+     * TODO use factory
+     *
      * @param TransactionCurrency $currency
      * @param array               $data
      *

app/Repositories/Currency/CurrencyRepositoryInterface.php

@@ -87,13 +87,31 @@ interface CurrencyRepositoryInterface
 
     /**
      * Find by currency name.
-     *
+     * @deprecated
      * @param string $currencyName
      *
      * @return TransactionCurrency
      */
     public function findByName(string $currencyName): TransactionCurrency;
 
+    /**
+     * Find by currency name.
+     *
+     * @param string $currencyName
+     *
+     * @return TransactionCurrency
+     */
+    public function findByNameNull(string $currencyName): ?TransactionCurrency;
+
+    /**
+     * Find by currency symbol.
+     * @deprecated
+     * @param string $currencySymbol
+     *
+     * @return TransactionCurrency
+     */
+    public function findBySymbol(string $currencySymbol): TransactionCurrency;
+
     /**
      * Find by currency symbol.
      *
@@ -101,7 +119,7 @@ interface CurrencyRepositoryInterface
      *
      * @return TransactionCurrency
      */
-    public function findBySymbol(string $currencySymbol): TransactionCurrency;
+    public function findBySymbolNull(string $currencySymbol): ?TransactionCurrency;
 
     /**
      * Find by ID, return NULL if not found.

tests/Feature/Controllers/BudgetControllerTest.php

@@ -65,6 +65,8 @@ class BudgetControllerTest extends TestCase
         $journalRepos->shouldReceive('first')->once()->andReturn(new TransactionJournal);
         $repository->shouldReceive('updateLimitAmount')->andReturn(new BudgetLimit);
         $repository->shouldReceive('spentInPeriod')->andReturn('0');
+        $repository->shouldReceive('budgetedPerDay')->andReturn('10');
+
 
         $data = ['amount' => 200, 'start' => '2017-01-01', 'end' => '2017-01-31'];
         $this->be($this->user());
@@ -83,6 +85,7 @@ class BudgetControllerTest extends TestCase
         $journalRepos->shouldReceive('first')->once()->andReturn(new TransactionJournal);
         $repository->shouldReceive('updateLimitAmount')->andReturn(new BudgetLimit);
         $repository->shouldReceive('spentInPeriod')->andReturn('0');
+        $repository->shouldReceive('budgetedPerDay')->andReturn('10');
 
         $data = ['amount' => 0, 'start' => '2017-01-01', 'end' => '2017-01-31'];
         $this->be($this->user());

tests/Unit/Import/Object/ImportAccountTest.php

@@ -52,7 +52,7 @@ class ImportAccountTest extends TestCase
         // mock calls:
         $repository->shouldReceive('setUser')->once()->withArgs([Mockery::any()]);
         $repository->shouldReceive('getAccountType')->twice()->withArgs([AccountType::ASSET])->andReturn($accountType);
-        $repository->shouldReceive('getAccountsByType')->twice()->withArgs([[AccountType::ASSET]])->andReturn(new Collection());
+        //$repository->shouldReceive('getAccountsByType')->twice()->withArgs([[AccountType::ASSET]])->andReturn(new Collection());
         $repository->shouldReceive('findNull')->once()->withArgs([1])->andReturn($account);
 
         // create import account.