From 3b3579025dbd7350f5a3fefa9b421802b78e947d Mon Sep 17 00:00:00 2001 From: James Cole Date: Fri, 14 Jul 2017 17:57:20 +0200 Subject: [PATCH] Improve results when presented with invalid data. #701 --- app/Import/Object/ImportJournal.php | 13 +++++ app/Import/Storage/ImportStorage.php | 8 ++-- .../Account/AccountRepository.php | 47 ++++++++++++++----- 3 files changed, 53 insertions(+), 15 deletions(-) diff --git a/app/Import/Object/ImportJournal.php b/app/Import/Object/ImportJournal.php index e5846f6b6f..c4aa0feaa3 100644 --- a/app/Import/Object/ImportJournal.php +++ b/app/Import/Object/ImportJournal.php @@ -65,6 +65,19 @@ class ImportJournal /** @var User */ private $user; + /** + * @return string + */ + public function getDescription(): string + { + if ($this->description === '') { + return '(no description)'; + } + + return $this->description; + } + + /** * ImportEntry constructor. */ diff --git a/app/Import/Storage/ImportStorage.php b/app/Import/Storage/ImportStorage.php index 766538bbf9..c3d0723cdf 100644 --- a/app/Import/Storage/ImportStorage.php +++ b/app/Import/Storage/ImportStorage.php @@ -165,7 +165,7 @@ class ImportStorage $errorText = join(', ', $transaction->getErrors()->all()); throw new FireflyException($errorText); } - Log::debug(sprintf('Created transaction with ID #%d and account #%d', $transaction->id, $accountId)); + Log::debug(sprintf('Created transaction with ID #%d, account #%d, amount %s', $transaction->id, $accountId, $amount)); return true; } @@ -301,7 +301,7 @@ class ImportStorage private function storeImportJournal(int $index, ImportJournal $importJournal): bool { - Log::debug(sprintf('Going to store object #%d with description "%s"', $index, $importJournal->description)); + Log::debug(sprintf('Going to store object #%d with description "%s"', $index, $importJournal->getDescription())); $importJournal->asset->setDefaultAccountId($this->job->configuration['import-account']); $asset = $importJournal->asset->getAccount(); $amount = $importJournal->getAmount(); @@ -338,7 +338,7 @@ class ImportStorage $journal->user_id = $this->job->user_id; $journal->transaction_type_id = $transactionType->id; $journal->transaction_currency_id = $currency->id; - $journal->description = $importJournal->description; + $journal->description = $importJournal->getDescription(); $journal->date = $date->format('Y-m-d'); $journal->order = 0; $journal->tag_count = 0; @@ -430,7 +430,7 @@ class ImportStorage $asset = $importJournal->asset->getAccount(); $opposing = $this->getOpposingAccount($importJournal->opposing, $amount); $date = $importJournal->getDate($this->dateFormat); - $description = $importJournal->description; + $description = $importJournal->getDescription(); $set = TransactionJournal:: leftJoin('transaction_types', 'transaction_types.id', '=', 'transaction_journals.transaction_type_id') ->leftJoin( diff --git a/app/Repositories/Account/AccountRepository.php b/app/Repositories/Account/AccountRepository.php index 86bd353c0a..037b73094e 100644 --- a/app/Repositories/Account/AccountRepository.php +++ b/app/Repositories/Account/AccountRepository.php @@ -25,6 +25,7 @@ use FireflyIII\Models\TransactionJournal; use FireflyIII\Models\TransactionType; use FireflyIII\User; use Log; +use Validator; /** @@ -236,7 +237,7 @@ class AccountRepository implements AccountRepositoryInterface $data['accountType'] = $data['accountType'] ?? 'invalid'; $type = config('firefly.accountTypeByIdentifier.' . $data['accountType']); $accountType = AccountType::whereType($type)->first(); - + $data['iban'] = $this->filterIban($data['iban']); // verify account type if (is_null($accountType)) { throw new FireflyException(sprintf('Account type "%s" is invalid. Cannot create account.', $data['accountType'])); @@ -251,16 +252,17 @@ class AccountRepository implements AccountRepositoryInterface } // create it: - $newAccount = new Account( - [ - 'user_id' => $this->user->id, - 'account_type_id' => $accountType->id, - 'name' => $data['name'], - 'virtual_balance' => $data['virtualBalance'], - 'active' => $data['active'] === true ? true : false, - 'iban' => $data['iban'], - ] - ); + $databaseData + = [ + 'user_id' => $this->user->id, + 'account_type_id' => $accountType->id, + 'name' => $data['name'], + 'virtual_balance' => $data['virtualBalance'], + 'active' => $data['active'] === true ? true : false, + 'iban' => $data['iban'], + ]; + $newAccount = new Account($databaseData); + Log::debug('Final account creation dataset', $databaseData); $newAccount->save(); // verify its creation: if (is_null($newAccount->id)) { @@ -268,6 +270,7 @@ class AccountRepository implements AccountRepositoryInterface sprintf('Could not create account "%s" (%d error(s))', $data['name'], $newAccount->getErrors()->count()), $newAccount->getErrors()->toArray() ); throw new FireflyException(sprintf('Tried to create account named "%s" but failed. The logs have more details.', $data['name'])); + } Log::debug(sprintf('Created new account #%d named "%s" of type %s.', $newAccount->id, $newAccount->name, $accountType->type)); @@ -491,4 +494,26 @@ class AccountRepository implements AccountRepositoryInterface return false; } + + /** + * @param string $iban + * + * @return null|string + */ + private function filterIban(string $iban = null) + { + if (is_null($iban)) { + return null; + } + $data = ['iban' => $iban]; + $rules = ['iban' => 'required|iban']; + $validator = Validator::make($data, $rules); + if ($validator->fails()) { + Log::error(sprintf('Detected invalid IBAN ("%s"). Return NULL instead.', $iban)); + + return null; + } + + return $iban; + } }