Clean up two big methods.

app/Support/Import/Placeholder/ImportTransaction.php

@@ -181,13 +181,20 @@ class ImportTransaction
         }
 
         $meta = ['sepa-ct-id', 'sepa-ct-op', 'sepa-db', 'sepa-cc', 'sepa-country', 'sepa-batch-id', 'sepa-ep', 'sepa-ci', 'internal-reference', 'date-interest',
-                 'date-invoice', 'date-book', 'date-payment', 'date-process', 'date-due', 'rabo-debit-credit', 'ing-debit-credit',];
+                 'date-invoice', 'date-book', 'date-payment', 'date-process', 'date-due',];
-        if (isset($meta[$role])) {
+        if (\in_array($role, $meta, true)) {
             $this->meta[$role] = $columnValue->getValue();
 
             return;
         }
 
+        $modifiers = ['rabo-debit-credit', 'ing-debit-credit'];
+        if (\in_array($role, $modifiers, true)) {
+            $this->modifiers[$role] = $columnValue->getValue();
+
+            return;
+        }
+
         switch ($role) {
             default:
                 // @codeCoverageIgnoreStart

app/Support/Import/Routine/File/ImportableConverter.php

@@ -26,6 +26,7 @@ namespace FireflyIII\Support\Import\Routine\File;
 use Carbon\Carbon;
 use Carbon\Exceptions\InvalidDateException;
 use FireflyIII\Exceptions\FireflyException;
+use FireflyIII\Models\Account;
 use FireflyIII\Models\AccountType;
 use FireflyIII\Models\ImportJob;
 use FireflyIII\Models\TransactionCurrency;
@@ -132,23 +133,26 @@ class ImportableConverter
     /**
      * @param string|null $date
      *
-     * @return string|null
+     * @return string
      */
-    private function convertDateValue(string $date = null): ?string
+    private function convertDateValue(string $date = null): string
     {
-        if (null === $date) {
+        $result = null;
-            return null;
+        if (null !== $date) {
+            try {
+                $object = Carbon::createFromFormat($this->config['date-format'] ?? 'Ymd', $date);
+                $result = $object->format('Y-m-d');
+            } catch (InvalidDateException|InvalidArgumentException $e) {
+                Log::error($e->getMessage());
+                Log::error($e->getTraceAsString());
+            }
         }
-        try {
+        if (null === $result) {
-            $object = Carbon::createFromFormat($this->config['date-format'] ?? 'Ymd', $date);
+            $object = new Carbon;
-        } catch (InvalidDateException|InvalidArgumentException $e) {
+            $result = $object->format('Y-m-d');
-            Log::error($e->getMessage());
-            Log::error($e->getTraceAsString());
-
-            return null;
         }
 
-        return $object->format('Y-m-d');
+        return $result;
     }
 
     /**
@@ -160,8 +164,9 @@ class ImportableConverter
     private function convertSingle(ImportTransaction $importable): array
     {
         Log::debug(sprintf('Description is: "%s"', $importable->description));
-        $amount        = $importable->calculateAmount();
         $foreignAmount = $importable->calculateForeignAmount();
+        $amount        = $importable->calculateAmount();
+
         if ('' === $amount) {
             $amount = $foreignAmount;
         }
@@ -169,7 +174,6 @@ class ImportableConverter
             throw new FireflyException('No transaction amount information.');
         }
 
-        $transactionType = 'unknown';
         $source          = $this->assetMapper->map($importable->accountId, $importable->getAccountData());
         $destination     = $this->opposingMapper->map($importable->opposingId, $amount, $importable->getOpposingAccountData());
         $currency        = $this->currencyMapper->map($importable->currencyId, $importable->getCurrencyData());
@@ -178,11 +182,6 @@ class ImportableConverter
         Log::debug(sprintf('"%s" (#%d) is source and "%s" (#%d) is destination.', $source->name, $source->id, $destination->name, $destination->id));
 
 
-        if ($source->accountType->type === AccountType::ASSET && $destination->accountType->type === AccountType::ASSET) {
-            Log::debug('Source and destination are asset accounts. This is a transfer.');
-            $transactionType = 'transfer';
-        }
-
         // amount is positive? Then switch:
         if (1 === bccomp($amount, '0')) {
 
@@ -195,37 +194,6 @@ class ImportableConverter
             );
         }
 
-        // get currency preference from source asset account (preferred)
-        // or destination asset account
-        if (null === $currency) {
-            if ($destination->accountType->type === AccountType::ASSET) {
-                // destination is asset, might have currency preference:
-                $destinationCurrencyId = (int)$this->accountRepository->getMetaValue($destination, 'currency_id');
-                $currency              = $destinationCurrencyId === 0 ? $this->defaultCurrency : $this->currencyMapper->map($destinationCurrencyId, []);
-                Log::debug(sprintf('Destination is an asset account, and has currency preference %s', $currency->code));
-            }
-
-            if ($source->accountType->type === AccountType::ASSET) {
-                // source is asset, might have currency preference:
-                $sourceCurrencyId = (int)$this->accountRepository->getMetaValue($source, 'currency_id');
-                $currency         = $sourceCurrencyId === 0 ? $this->defaultCurrency : $this->currencyMapper->map($sourceCurrencyId, []);
-                Log::debug(sprintf('Source is an asset account, and has currency preference %s', $currency->code));
-            }
-        }
-        if (null === $currency) {
-            Log::debug(sprintf('Could not map currency, use default (%s)', $this->defaultCurrency->code));
-            $currency = $this->defaultCurrency;
-        }
-
-
-        if ($source->accountType->type === AccountType::REVENUE) {
-            Log::debug('Source is a revenue account. This is a deposit.');
-            $transactionType = 'deposit';
-        }
-        if ($destination->accountType->type === AccountType::EXPENSE) {
-            Log::debug('Destination is an expense account. This is a withdrawal.');
-            $transactionType = 'withdrawal';
-        }
         if ($destination->id === $source->id) {
             throw new FireflyException(
                 sprintf(
@@ -234,6 +202,9 @@ class ImportableConverter
             );
         }
 
+        $transactionType = $this->getTransactionType($source->accountType->type, $destination->accountType->type);
+        $currency        = $currency ?? $this->getCurrency($source, $destination);
+
         if ($transactionType === 'unknown') {
             $message = sprintf(
                 'Cannot determine transaction type. Source account is a %s, destination is a %s', $source->accountType->type, $destination->accountType->type
@@ -242,16 +213,9 @@ class ImportableConverter
             throw new FireflyException($message);
         }
 
-        $dateStr = $this->convertDateValue($importable->date);
-        if (null === $dateStr) {
-            $date    = new Carbon;
-            $dateStr = $date->format('Y-m-d');
-            unset($date);
-        }
-
         return [
             'type'               => $transactionType,
-            'date'               => $dateStr,
+            'date'               => $this->convertDateValue($importable->date),
             'tags'               => $importable->tags,
             'user'               => $this->importJob->user_id,
             'notes'              => $importable->note,
@@ -307,26 +271,58 @@ class ImportableConverter
     }
 
     /**
-     * A small function that verifies if this particular key (ID) is present in the list
+     * @param Account $source
-     * of valid keys.
+     * @param Account $destination
      *
-     * @param string $key
+     * @return TransactionCurrency
-     * @param int    $objectId
-     *
-     * @return int|null
      */
-    private function verifyObjectId(string $key, int $objectId): ?int
+    private function getCurrency(Account $source, Account $destination): TransactionCurrency
     {
-        if (isset($this->mappedValues[$key]) && \in_array($objectId, $this->mappedValues[$key], true)) {
+        $currency = null;
-            Log::debug(sprintf('verifyObjectId(%s, %d) is valid!', $key, $objectId));
+        if ($destination->accountType->type === AccountType::ASSET) {
-
+            // destination is asset, might have currency preference:
-            return $objectId;
+            $destinationCurrencyId = (int)$this->accountRepository->getMetaValue($destination, 'currency_id');
+            $currency              = $destinationCurrencyId === 0 ? $this->defaultCurrency : $this->currencyMapper->map($destinationCurrencyId, []);
+            Log::debug(sprintf('Destination is an asset account, and has currency preference %s', $currency->code));
         }
 
-        Log::debug(sprintf('verifyObjectId(%s, %d) is NOT in the list, but it could still be valid.', $key, $objectId));
+        if ($source->accountType->type === AccountType::ASSET) {
+            // source is asset, might have currency preference:
+            $sourceCurrencyId = (int)$this->accountRepository->getMetaValue($source, 'currency_id');
+            $currency         = $sourceCurrencyId === 0 ? $this->defaultCurrency : $this->currencyMapper->map($sourceCurrencyId, []);
+            Log::debug(sprintf('Source is an asset account, and has currency preference %s', $currency->code));
+        }
+        if (null === $currency) {
+            Log::debug(sprintf('Could not map currency, use default (%s)', $this->defaultCurrency->code));
+            $currency = $this->defaultCurrency;
+        }
 
-        return $objectId;
+        return $currency;
     }
 
+    /**
+     * @param string $source
+     * @param string $destination
+     *
+     * @return string
+     */
+    private function getTransactionType(string $source, string $destination): string
+    {
+        $type = 'unknown';
 
+        if ($source === AccountType::ASSET && $destination === AccountType::ASSET) {
+            Log::debug('Source and destination are asset accounts. This is a transfer.');
+            $type = 'transfer';
+        }
+        if ($source === AccountType::REVENUE) {
+            Log::debug('Source is a revenue account. This is a deposit.');
+            $type = 'deposit';
+        }
+        if ($destination === AccountType::EXPENSE) {
+            Log::debug('Destination is an expense account. This is a withdrawal.');
+            $type = 'withdrawal';
+        }
+
+        return $type;
+    }
 }