diff --git a/app/Factory/TransactionGroupFactory.php b/app/Factory/TransactionGroupFactory.php index 35630eed7d..df3b67edb3 100644 --- a/app/Factory/TransactionGroupFactory.php +++ b/app/Factory/TransactionGroupFactory.php @@ -28,6 +28,7 @@ use FireflyIII\User; /** * Class TransactionGroupFactory + * * @codeCoverageIgnore */ class TransactionGroupFactory @@ -58,7 +59,12 @@ class TransactionGroupFactory $collection = $this->journalFactory->create($data); $title = $data['group_title'] ?? null; $title = '' === $title ? null : $title; - $group = new TransactionGroup; + + if (null !== $title) { + $title = substr($title, 0, 255); + } + + $group = new TransactionGroup; $group->user()->associate($this->user); $group->title = $title; $group->save(); diff --git a/app/Factory/TransactionJournalFactory.php b/app/Factory/TransactionJournalFactory.php index d18753330e..adfa1e14d4 100644 --- a/app/Factory/TransactionJournalFactory.php +++ b/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.'); } } @@ -171,8 +171,8 @@ class TransactionJournalFactory /** * @param TransactionJournal $journal - * @param NullArrayObject $data - * @param string $field + * @param NullArrayObject $data + * @param string $field */ protected function storeMeta(TransactionJournal $journal, NullArrayObject $data, string $field): void { @@ -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(); -// if (2 !== $count) { -// // @codeCoverageIgnoreStart -// Log::error(sprintf('The journal unexpectedly has %d transaction(s). This is not OK. Cancel operation.', $count)); -// try { -// $journal->delete(); -// } catch (Exception $e) { -// Log::debug(sprintf('Dont care: %s.', $e->getMessage())); -// } -// -// return null; -// // @codeCoverageIgnoreEnd -// } + // $count = $journal->transactions()->count(); + // if (2 !== $count) { + // // @codeCoverageIgnoreStart + // Log::error(sprintf('The journal unexpectedly has %d transaction(s). This is not OK. Cancel operation.', $count)); + // try { + // $journal->delete(); + // } catch (Exception $e) { + // Log::debug(sprintf('Dont care: %s.', $e->getMessage())); + // } + // + // return null; + // // @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 * @@ -391,7 +386,7 @@ class TransactionJournalFactory $dataRow = $row->getArrayCopy(); unset($dataRow['import_hash_v2'], $dataRow['original_source']); - $json = json_encode($dataRow); + $json = json_encode($dataRow); if (false === $json) { // @codeCoverageIgnoreStart $json = json_encode((string)microtime()); @@ -406,7 +401,7 @@ class TransactionJournalFactory /** * @param TransactionJournal $journal - * @param NullArrayObject $transaction + * @param NullArrayObject $transaction */ private function storeMetaFields(TransactionJournal $journal, NullArrayObject $transaction): void { @@ -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; - } } diff --git a/app/Support/Import/Routine/File/ImportableConverter.php b/app/Support/Import/Routine/File/ImportableConverter.php index b762741bd9..f4235d91f8 100644 --- a/app/Support/Import/Routine/File/ImportableConverter.php +++ b/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,13 +178,18 @@ class ImportableConverter throw new FireflyException('No transaction amount information.'); } - $source = $this->assetMapper->map($importable->accountId, $importable->getAccountData()); - $destination = $this->opposingMapper->map($importable->opposingId, $amount, $importable->getOpposingAccountData()); + // 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()); // if the amount is positive, switch source and destination (account and opposing account) if (1 === bccomp($amount, '0')) { - $source = $this->opposingMapper->map($importable->opposingId, $amount, $importable->getOpposingAccountData()); - $destination = $this->assetMapper->map($importable->accountId, $importable->getAccountData()); + $source = $this->opposingMapper->map($importable->opposingId, $amount, $importable->getOpposingAccountData()); + $destination = $this->assetMapper->map($importable->accountId, $importable->getAccountData()); Log::debug( sprintf( '%s is positive, so "%s" (#%d) is now source and "%s" (#%d) is now destination.', @@ -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; - 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()); - } + $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 $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; + return $type; } }