Fix #2516

app/Factory/TransactionGroupFactory.php

@@ -28,6 +28,7 @@ use FireflyIII\User;
 
 /**
  * Class TransactionGroupFactory
+ *
  * @codeCoverageIgnore
  */
 class TransactionGroupFactory
@@ -58,6 +59,11 @@ class TransactionGroupFactory
         $collection = $this->journalFactory->create($data);
         $title      = $data['group_title'] ?? null;
         $title      = '' === $title ? null : $title;
+
+        if (null !== $title) {
+            $title = substr($title, 0, 255);
+        }
+
         $group = new TransactionGroup;
         $group->user()->associate($this->user);
         $group->title = $title;

app/Factory/TransactionJournalFactory.php

@@ -52,10 +52,10 @@ class TransactionJournalFactory
 {
     use JournalServiceTrait;
 
-    /** @var AccountValidator */
-    private $accountValidator;
     /** @var AccountRepositoryInterface */
     private $accountRepository;
+    /** @var AccountValidator */
+    private $accountValidator;
     /** @var BillRepositoryInterface */
     private $billRepository;
     /** @var CurrencyRepositoryInterface */
@@ -143,7 +143,7 @@ class TransactionJournalFactory
             if (null !== $journal) {
                 $collection->push($journal);
             }
-            if(null === $journal) {
+            if (null === $journal) {
                 Log::error('The createJournal() method returned NULL. This may indicate an error.');
             }
         }
@@ -189,32 +189,6 @@ class TransactionJournalFactory
         $factory->updateOrCreate($set);
     }
 
-    /**
-     * Link a piggy bank to this journal.
-     *
-     * @param TransactionJournal $journal
-     * @param NullArrayObject $data
-     */
-    private function storePiggyEvent(TransactionJournal $journal, NullArrayObject $data): void
-    {
-        Log::debug('Will now store piggy event.');
-        if (!$journal->isTransfer()) {
-            Log::debug('Journal is not a transfer, do nothing.');
-
-            return;
-        }
-
-        $piggyBank = $this->piggyRepository->findPiggyBank((int)$data['piggy_bank_id'], $data['piggy_bank_name']);
-
-        if (null !== $piggyBank) {
-            $this->piggyEventFactory->create($journal, $piggyBank);
-            Log::debug('Create piggy event.');
-
-            return;
-        }
-        Log::debug('Create no piggy event');
-    }
-
     /**
      * @param NullArrayObject $row
      *
@@ -302,6 +276,10 @@ class TransactionJournalFactory
             $destForeignCurrency   = $foreignCurrency;
         }
 
+        $description = '' === $description ? '(empty description)' : $description;
+        $description = substr($description, 0, 255);
+
+
         /** Create a basic journal. */
         $journal = TransactionJournal::create(
             [
@@ -309,7 +287,7 @@ class TransactionJournalFactory
                 'transaction_type_id'     => $type->id,
                 'bill_id'                 => $billId,
                 'transaction_currency_id' => $currency->id,
-                'description'             => '' === $description ? '(empty description)' : $description,
+                'description'             => $description,
                 'date'                    => $carbon->format('Y-m-d H:i:s'),
                 'order'                   => $order,
                 'tag_count'               => 0,
@@ -342,19 +320,19 @@ class TransactionJournalFactory
 
         // verify that journal has two transactions. Otherwise, delete and cancel.
         // TODO this can't be faked so it can't be tested.
-//        $count = $journal->transactions()->count();
+        //        $count = $journal->transactions()->count();
-//        if (2 !== $count) {
+        //        if (2 !== $count) {
-//            // @codeCoverageIgnoreStart
+        //            // @codeCoverageIgnoreStart
-//            Log::error(sprintf('The journal unexpectedly has %d transaction(s). This is not OK. Cancel operation.', $count));
+        //            Log::error(sprintf('The journal unexpectedly has %d transaction(s). This is not OK. Cancel operation.', $count));
-//            try {
+        //            try {
-//                $journal->delete();
+        //                $journal->delete();
-//            } catch (Exception $e) {
+        //            } catch (Exception $e) {
-//                Log::debug(sprintf('Dont care: %s.', $e->getMessage()));
+        //                Log::debug(sprintf('Dont care: %s.', $e->getMessage()));
-//            }
+        //            }
-//
+        //
-//            return null;
+        //            return null;
-//            // @codeCoverageIgnoreEnd
+        //            // @codeCoverageIgnoreEnd
-//        }
+        //        }
         $journal->completed = true;
         $journal->save();
 
@@ -381,6 +359,23 @@ class TransactionJournalFactory
         return $journal;
     }
 
+    /**
+     * @param TransactionCurrency|null $currency
+     * @param Account                  $account
+     *
+     * @return TransactionCurrency
+     */
+    private function getCurrency(?TransactionCurrency $currency, Account $account): TransactionCurrency
+    {
+        $preference = $this->accountRepository->getAccountCurrency($account);
+        if (null === $preference && null === $currency) {
+            // return user's default:
+            return app('amount')->getDefaultCurrencyByUser($this->user);
+        }
+
+        return $preference ?? $currency;
+    }
+
     /**
      * @param NullArrayObject $row
      *
@@ -415,9 +410,35 @@ class TransactionJournalFactory
         }
     }
 
+    /**
+     * Link a piggy bank to this journal.
+     *
+     * @param TransactionJournal $journal
+     * @param NullArrayObject    $data
+     */
+    private function storePiggyEvent(TransactionJournal $journal, NullArrayObject $data): void
+    {
+        Log::debug('Will now store piggy event.');
+        if (!$journal->isTransfer()) {
+            Log::debug('Journal is not a transfer, do nothing.');
+
+            return;
+        }
+
+        $piggyBank = $this->piggyRepository->findPiggyBank((int)$data['piggy_bank_id'], $data['piggy_bank_name']);
+
+        if (null !== $piggyBank) {
+            $this->piggyEventFactory->create($journal, $piggyBank);
+            Log::debug('Create piggy event.');
+
+            return;
+        }
+        Log::debug('Create no piggy event');
+    }
 
     /**
      * @param NullArrayObject $data
+     *
      * @throws FireflyException
      */
     private function validateAccounts(NullArrayObject $data): void
@@ -445,20 +466,4 @@ class TransactionJournalFactory
             throw new FireflyException(sprintf('Destination: %s', $this->accountValidator->destError)); // @codeCoverageIgnore
         }
     }
-
-    /**
-     * @param TransactionCurrency|null $currency
-     * @param Account $account
-     * @return TransactionCurrency
-     */
-    private function getCurrency(?TransactionCurrency $currency, Account $account): TransactionCurrency
-    {
-        $preference = $this->accountRepository->getAccountCurrency($account);
-        if (null === $preference && null === $currency) {
-            // return user's default:
-            return app('amount')->getDefaultCurrencyByUser($this->user);
-        }
-
-        return $preference ?? $currency;
-    }
 }

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

@@ -89,6 +89,74 @@ class ImportableConverter
         return $result;
     }
 
+    /**
+     * @param ImportJob $importJob
+     */
+    public function setImportJob(ImportJob $importJob): void
+    {
+        $this->importJob = $importJob;
+        $this->config    = $importJob->configuration;
+
+        // repository is used for error messages
+        $this->repository = app(ImportJobRepositoryInterface::class);
+        $this->repository->setUser($importJob->user);
+
+        // asset account mapper can map asset accounts (makes sense right?)
+        $this->assetMapper = app(AssetAccountMapper::class);
+        $this->assetMapper->setUser($importJob->user);
+        $this->assetMapper->setDefaultAccount($this->config['import-account'] ?? 0);
+
+        // asset account repository is used for currency information
+        $this->accountRepository = app(AccountRepositoryInterface::class);
+        $this->accountRepository->setUser($importJob->user);
+
+        // opposing account mapper:
+        $this->opposingMapper = app(OpposingAccountMapper::class);
+        $this->opposingMapper->setUser($importJob->user);
+
+        // currency mapper:
+        $this->currencyMapper = app(CurrencyMapper::class);
+        $this->currencyMapper->setUser($importJob->user);
+        $this->defaultCurrency = app('amount')->getDefaultCurrencyByUser($importJob->user);
+    }
+
+    /**
+     * @codeCoverageIgnore
+     *
+     * @param array $mappedValues
+     */
+    public function setMappedValues(array $mappedValues): void
+    {
+        $this->mappedValues = $mappedValues;
+    }
+
+    /**
+     * @param string|null $date
+     *
+     * @return string|null
+     */
+    private function convertDateValue(string $date = null): ?string
+    {
+        $result = null;
+        if (null !== $date) {
+            try {
+                // add exclamation mark for better parsing. http://php.net/manual/en/datetime.createfromformat.php
+                $dateFormat = $this->config['date-format'] ?? 'Ymd';
+                if ('!' !== $dateFormat{0}) {
+                    $dateFormat = '!' . $dateFormat;
+                }
+                $object = Carbon::createFromFormat($dateFormat, $date);
+                $result = $object->format('Y-m-d H:i:s');
+                Log::debug(sprintf('createFromFormat: Turning "%s" into "%s" using "%s"', $date, $result, $this->config['date-format'] ?? 'Ymd'));
+            } catch (InvalidDateException|InvalidArgumentException $e) {
+                Log::error($e->getMessage());
+                Log::error($e->getTraceAsString());
+            }
+        }
+
+        return $result;
+    }
+
     /**
      * @param ImportTransaction $importable
      *
@@ -110,6 +178,11 @@ class ImportableConverter
             throw new FireflyException('No transaction amount information.');
         }
 
+        // amount is 0? skip
+        if (0 === bccomp($amount, '0')) {
+            throw new FireflyException('Amount of transaction is zero.');
+        }
+
         $source      = $this->assetMapper->map($importable->accountId, $importable->getAccountData());
         $destination = $this->opposingMapper->map($importable->opposingId, $amount, $importable->getOpposingAccountData());
 
@@ -219,25 +292,6 @@ class ImportableConverter
 
     }
 
-    /**
-     * @param string $source
-     * @param string $destination
-     *
-     * @return string
-     */
-    private function getTransactionType(string $source, string $destination): string
-    {
-        $type = 'unknown';
-
-        $newType = config(sprintf('firefly.account_to_transaction.%s.%s', $source, $destination));
-        if (null !== $newType) {
-            Log::debug(sprintf('Source is %s, destination is %s, so this is a %s.', $source, $destination, $newType));
-
-            return (string)$newType;
-        }
-        return $type;
-    }
-
     /**
      * @param Account $source
      * @param Account $destination
@@ -269,70 +323,22 @@ class ImportableConverter
     }
 
     /**
-     * @param string|null $date
+     * @param string $source
+     * @param string $destination
      *
-     * @return string|null
+     * @return string
      */
-    private function convertDateValue(string $date = null): ?string
+    private function getTransactionType(string $source, string $destination): string
     {
-        $result = null;
+        $type = 'unknown';
-        if (null !== $date) {
+
-            try {
+        $newType = config(sprintf('firefly.account_to_transaction.%s.%s', $source, $destination));
-                // add exclamation mark for better parsing. http://php.net/manual/en/datetime.createfromformat.php
+        if (null !== $newType) {
-                $dateFormat = $this->config['date-format'] ?? 'Ymd';
+            Log::debug(sprintf('Source is %s, destination is %s, so this is a %s.', $source, $destination, $newType));
-                if ('!' !== $dateFormat{0}) {
+
-                    $dateFormat = '!' . $dateFormat;
+            return (string)$newType;
-                }
-                $object = Carbon::createFromFormat($dateFormat, $date);
-                $result = $object->format('Y-m-d H:i:s');
-                Log::debug(sprintf('createFromFormat: Turning "%s" into "%s" using "%s"', $date, $result, $this->config['date-format'] ?? 'Ymd'));
-            } catch (InvalidDateException|InvalidArgumentException $e) {
-                Log::error($e->getMessage());
-                Log::error($e->getTraceAsString());
-            }
         }
 
-        return $result;
+        return $type;
-    }
-
-    /**
-     * @param ImportJob $importJob
-     */
-    public function setImportJob(ImportJob $importJob): void
-    {
-        $this->importJob = $importJob;
-        $this->config    = $importJob->configuration;
-
-        // repository is used for error messages
-        $this->repository = app(ImportJobRepositoryInterface::class);
-        $this->repository->setUser($importJob->user);
-
-        // asset account mapper can map asset accounts (makes sense right?)
-        $this->assetMapper = app(AssetAccountMapper::class);
-        $this->assetMapper->setUser($importJob->user);
-        $this->assetMapper->setDefaultAccount($this->config['import-account'] ?? 0);
-
-        // asset account repository is used for currency information
-        $this->accountRepository = app(AccountRepositoryInterface::class);
-        $this->accountRepository->setUser($importJob->user);
-
-        // opposing account mapper:
-        $this->opposingMapper = app(OpposingAccountMapper::class);
-        $this->opposingMapper->setUser($importJob->user);
-
-        // currency mapper:
-        $this->currencyMapper = app(CurrencyMapper::class);
-        $this->currencyMapper->setUser($importJob->user);
-        $this->defaultCurrency = app('amount')->getDefaultCurrencyByUser($importJob->user);
-    }
-
-    /**
-     * @codeCoverageIgnore
-     *
-     * @param array $mappedValues
-     */
-    public function setMappedValues(array $mappedValues): void
-    {
-        $this->mappedValues = $mappedValues;
     }
 }