From bdf7eee72f3e7b25bce9a1276678ca0a259a5e28 Mon Sep 17 00:00:00 2001 From: James Cole Date: Tue, 7 Jul 2015 01:07:19 +0200 Subject: [PATCH] Clean up some code routines. --- .../Csv/Converter/AssetAccountIban.php | 48 ++++---- .../Csv/Converter/OpposingAccountIban.php | 29 +++-- .../Csv/PostProcessing/OpposingAccount.php | 105 +++++++++++++----- app/Http/Controllers/ReportController.php | 2 +- app/Validation/FireflyValidator.php | 7 ++ 5 files changed, 135 insertions(+), 56 deletions(-) diff --git a/app/Helpers/Csv/Converter/AssetAccountIban.php b/app/Helpers/Csv/Converter/AssetAccountIban.php index a2229bdc94..c928534663 100644 --- a/app/Helpers/Csv/Converter/AssetAccountIban.php +++ b/app/Helpers/Csv/Converter/AssetAccountIban.php @@ -5,7 +5,6 @@ namespace FireflyIII\Helpers\Csv\Converter; use Auth; use FireflyIII\Models\Account; use FireflyIII\Models\AccountType; -use Log; /** * Class AssetAccountIban @@ -28,31 +27,42 @@ class AssetAccountIban extends BasicConverter implements ConverterInterface } if (strlen($this->value) > 0) { // find or create new account: + $account = $this->findAccount(); $accountType = AccountType::where('type', 'Asset account')->first(); - $set = Auth::user()->accounts()->accountTypeIn(['Default account', 'Asset account'])->get(['accounts.*']); - /** @var Account $entry */ - foreach ($set as $entry) { - if ($entry->iban == $this->value) { - Log::debug('AssetAccountIban::convert found an Account (#' . $entry->id . ': ' . $entry->name . ') with IBAN ' . $this->value); - return $entry; - } + if (is_null($account)) { + // create it if doesn't exist. + $account = Account::firstOrCreateEncrypted( + [ + 'name' => $this->value, + 'iban' => $this->value, + 'user_id' => Auth::user()->id, + 'account_type_id' => $accountType->id, + 'active' => 1, + ] + ); } - // create it if doesn't exist. - $account = Account::firstOrCreateEncrypted( - [ - 'name' => $this->value, - 'iban' => $this->value, - 'user_id' => Auth::user()->id, - 'account_type_id' => $accountType->id, - 'active' => 1, - ] - ); - return $account; } return null; } + + /** + * @return Account|null + */ + protected function findAccount() + { + $set = Auth::user()->accounts()->accountTypeIn(['Default account', 'Asset account'])->get(['accounts.*']); + /** @var Account $entry */ + foreach ($set as $entry) { + if ($entry->iban == $this->value) { + + return $entry; + } + } + + return null; + } } \ No newline at end of file diff --git a/app/Helpers/Csv/Converter/OpposingAccountIban.php b/app/Helpers/Csv/Converter/OpposingAccountIban.php index 92bad04d5e..ff615e2b82 100644 --- a/app/Helpers/Csv/Converter/OpposingAccountIban.php +++ b/app/Helpers/Csv/Converter/OpposingAccountIban.php @@ -27,18 +27,31 @@ class OpposingAccountIban extends BasicConverter implements ConverterInterface return $account; } else { if (strlen($this->value) > 0) { - $set = Auth::user()->accounts()->get(); - /** @var Account $account */ - foreach ($set as $account) { - if ($account->iban == $this->value) { - Log::debug('OpposingAccountIban::convert found an Account (#' . $account->id . ': ' . $account->name . ') with IBAN ' . $this->value); - - return $account; - } + $account = $this->findAccount(); + if (!is_null($account)) { + return $account; } } return $this->value; } } + + /** + * @return Account|null + */ + protected function findAccount() + { + $set = Auth::user()->accounts()->get(); + /** @var Account $account */ + foreach ($set as $account) { + if ($account->iban == $this->value) { + Log::debug('OpposingAccountIban::convert found an Account (#' . $account->id . ': ' . $account->name . ') with IBAN ' . $this->value); + + return $account; + } + } + + return null; + } } \ No newline at end of file diff --git a/app/Helpers/Csv/PostProcessing/OpposingAccount.php b/app/Helpers/Csv/PostProcessing/OpposingAccount.php index 55d3b67ea4..f7690438be 100644 --- a/app/Helpers/Csv/PostProcessing/OpposingAccount.php +++ b/app/Helpers/Csv/PostProcessing/OpposingAccount.php @@ -24,36 +24,19 @@ class OpposingAccount implements PostProcessorInterface */ public function process() { - if ($this->data['opposing-account-id'] instanceof Account) { // first priority. try to find the account based on ID, if any - $this->data['opposing-account-object'] = $this->data['opposing-account-id']; - - return $this->data; + $result = $this->checkIdNameObject(); + if (!is_null($result)) { + return $result; } - if ($this->data['opposing-account-iban'] instanceof Account) { // second: try to find the account based on IBAN, if any. - $this->data['opposing-account-object'] = $this->data['opposing-account-iban']; - return $this->data; + $result = $this->checkIbanString(); + if (!is_null($result)) { + return $result; } - $rules = ['iban' => 'iban']; - $check = ['iban' => $this->data['opposing-account-iban']]; - $validator = Validator::make($check, $rules); - $result = !$validator->fails(); - if (is_string($this->data['opposing-account-iban']) && strlen($this->data['opposing-account-iban']) > 0) { - if ($result) { - $this->data['opposing-account-object'] = $this->parseIbanString(); - return $this->data; - } - } - if ($this->data['opposing-account-name'] instanceof Account) { // third: try to find account based on name, if any. - $this->data['opposing-account-object'] = $this->data['opposing-account-name']; - - return $this->data; - } - if (is_string($this->data['opposing-account-name'])) { - $this->data['opposing-account-object'] = $this->parseNameString(); - - return $this->data; + $result = $this->checkNameString(); + if (!is_null($result)) { + return $result; } return null; @@ -67,20 +50,67 @@ class OpposingAccount implements PostProcessorInterface $this->data = $data; } + /** + * @return array + */ + protected function checkIdNameObject() + { + if ($this->data['opposing-account-id'] instanceof Account) { // first priority. try to find the account based on ID, if any + $this->data['opposing-account-object'] = $this->data['opposing-account-id']; + + return $this->data; + } + if ($this->data['opposing-account-iban'] instanceof Account) { // second: try to find the account based on IBAN, if any. + $this->data['opposing-account-object'] = $this->data['opposing-account-iban']; + + return $this->data; + } + + return null; + } + + /** + * @return array|null + */ + protected function checkIbanString() + { + $rules = ['iban' => 'iban']; + $check = ['iban' => $this->data['opposing-account-iban']]; + $validator = Validator::make($check, $rules); + if (!$validator->fails()) { + $this->data['opposing-account-object'] = $this->parseIbanString(); + + return $this->data; + } + + return null; + } + /** * @return Account|null */ protected function parseIbanString() { // create by name and/or iban. - $accountType = $this->getAccountType(); - $accounts = Auth::user()->accounts()->get(); + $accounts = Auth::user()->accounts()->get(); foreach ($accounts as $entry) { if ($entry->iban == $this->data['opposing-account-iban']) { return $entry; } } + $account = $this->createAccount(); + + return $account; + } + + /** + * @return Account|null + */ + protected function createAccount() + { + $accountType = $this->getAccountType(); + // create if not exists: $name = is_string($this->data['opposing-account-name']) && strlen($this->data['opposing-account-name']) > 0 ? $this->data['opposing-account-name'] : $this->data['opposing-account-iban']; @@ -117,6 +147,25 @@ class OpposingAccount implements PostProcessorInterface } } + /** + * @return array|null + */ + protected function checkNameString() + { + if ($this->data['opposing-account-name'] instanceof Account) { // third: try to find account based on name, if any. + $this->data['opposing-account-object'] = $this->data['opposing-account-name']; + + return $this->data; + } + if (is_string($this->data['opposing-account-name'])) { + $this->data['opposing-account-object'] = $this->parseNameString(); + + return $this->data; + } + + return null; + } + /** * @return Account|null */ diff --git a/app/Http/Controllers/ReportController.php b/app/Http/Controllers/ReportController.php index 4ed436aabf..4d1ca65470 100644 --- a/app/Http/Controllers/ReportController.php +++ b/app/Http/Controllers/ReportController.php @@ -88,7 +88,7 @@ class ReportController extends Controller $budgets = $this->helper->getBudgetReport($start, $end, $shared); $categories = $this->helper->getCategoryReport($start, $end, $shared); $balance = $this->helper->getBalanceReport($start, $end, $shared); - $bills = $this->helper->getBillReport($start, $end, $shared); + $bills = $this->helper->getBillReport($start, $end); Session::flash('gaEventCategory', 'report'); Session::flash('gaEventAction', 'month'); diff --git a/app/Validation/FireflyValidator.php b/app/Validation/FireflyValidator.php index 3b11fbebb6..faa07a5e49 100644 --- a/app/Validation/FireflyValidator.php +++ b/app/Validation/FireflyValidator.php @@ -62,6 +62,13 @@ class FireflyValidator extends Validator */ public function validateIban($attribute, $value) { + if (!is_string($value)) { + return false; + } + + if (strlen($value) === 0) { + return false; + } $value = strtoupper($value); if (strlen($value) < 6) {