From 44eb67f94e7d6753af3ef2677df8bf91ce4d7765 Mon Sep 17 00:00:00 2001 From: James Cole Date: Fri, 12 Sep 2014 21:47:27 +0200 Subject: [PATCH] Some cleanup, some bug fixes. --- app/controllers/AccountController.php | 34 +++++--- app/lib/Firefly/Queue/Import.php | 39 +++++++-- .../Account/AccountRepositoryInterface.php | 8 ++ .../Account/EloquentAccountRepository.php | 86 +++++++++++-------- .../EloquentTransactionJournalRepository.php | 3 +- app/models/TransactionJournal.php | 26 +++++- app/views/layouts/default.blade.php | 7 +- 7 files changed, 141 insertions(+), 62 deletions(-) diff --git a/app/controllers/AccountController.php b/app/controllers/AccountController.php index 76b13753fe..cba7b36eb1 100644 --- a/app/controllers/AccountController.php +++ b/app/controllers/AccountController.php @@ -22,6 +22,8 @@ class AccountController extends \BaseController { $this->_accounts = $accounts; $this->_repository = $repository; + View::share('mainTitleIcon', 'fa-credit-card'); + View::share('title', 'Accounts'); } /** @@ -29,9 +31,11 @@ class AccountController extends \BaseController */ public function create($what) { - return View::make('accounts.create')->with('title', 'Accounts')->with( - 'subTitle', 'Create a new ' . $what . ' account' - )->with('what', $what); + View::share('subTitleIcon', 'fa-money'); + + return View::make('accounts.create')->with('subTitle', 'Create a new ' . $what . ' account')->with( + 'what', $what + ); } /** @@ -39,11 +43,13 @@ class AccountController extends \BaseController */ public function asset() { + View::share('subTitleIcon','fa-money'); + $accounts = $this->_repository->getOfTypes(['Asset account', 'Default account']); - return View::make('accounts.asset')->with('title', 'Accounts')->with('subTitle', 'Asset accounts')->with( + return View::make('accounts.asset')->with('subTitle', 'Asset accounts')->with( 'accounts', $accounts - )->with('titleIcon','fa-money'); + ); } /** @@ -51,9 +57,11 @@ class AccountController extends \BaseController */ public function expense() { + View::share('subTitleIcon','fa-shopping-cart'); + $accounts = $this->_repository->getOfTypes(['Expense account', 'Beneficiary account']); - return View::make('accounts.expense')->with('title', 'Accounts')->with('subTitle', 'Expense accounts')->with( + return View::make('accounts.expense')->with('subTitle', 'Expense accounts')->with( 'accounts', $accounts ); } @@ -63,9 +71,11 @@ class AccountController extends \BaseController */ public function revenue() { + View::share('subTitleIcon','fa-download'); + $accounts = $this->_repository->getOfTypes(['Revenue account']); - return View::make('accounts.revenue')->with('title', 'Accounts')->with('subTitle', 'Revenue accounts')->with( + return View::make('accounts.revenue')->with('subTitle', 'Revenue accounts')->with( 'accounts', $accounts ); } @@ -119,8 +129,8 @@ class AccountController extends \BaseController { $openingBalance = $this->_accounts->openingBalanceTransaction($account); return View::make('accounts.edit')->with('account', $account)->with('openingBalance', $openingBalance) - ->with('title','Accounts') - ->with('subTitle', 'Edit '.strtolower($account->accountType->type).' "' . $account->name . '"'); + ->with('title', 'Accounts') + ->with('subTitle', 'Edit ' . strtolower($account->accountType->type) . ' "' . $account->name . '"'); } /** @@ -128,7 +138,7 @@ class AccountController extends \BaseController */ public function index() { - return View::make('error')->with('message','This view has been disabled'); + return View::make('error')->with('message', 'This view has been disabled'); // $accounts = $this->_repository->get(); // $set = [ // 'personal' => [], @@ -159,8 +169,8 @@ class AccountController extends \BaseController return View::make('accounts.show')->with('account', $account)->with('show', $data)->with( 'subTitle', - 'Details for '.strtolower($account->accountType->type).' "' . $account->name . '"' - )->with('title','Accounts'); + 'Details for ' . strtolower($account->accountType->type) . ' "' . $account->name . '"' + )->with('title', 'Accounts'); } /** diff --git a/app/lib/Firefly/Queue/Import.php b/app/lib/Firefly/Queue/Import.php index 935bbaa248..65004f0189 100644 --- a/app/lib/Firefly/Queue/Import.php +++ b/app/lib/Firefly/Queue/Import.php @@ -135,17 +135,33 @@ class Import // if Firefly tries to import a beneficiary, Firefly will "merge" already existing ones, // so we don't care: + if (isset($payload['data']['account_type']) && $payload['data']['account_type'] == 'Expense account') { - // store beneficiary - $acct = $this->_accounts->createOrFindBeneficiary($payload['data']['name']); + // unset some data to make firstOrCreate work: + $oldPayloadId = $payload['data']['id']; + unset($payload['data']['type_id'], $payload['data']['parent_component_id'], + $payload['data']['reporting'], $payload['data']['type'], $payload['data']['id'], $payload['data']['account_type']); + // set other data to make it work: + $expenseAccountType = $this->_accounts->findAccountType('Expense account'); + $payload['data']['account_type_id'] = $expenseAccountType->id; + + $acct = $this->_accounts->firstOrCreate((array)$payload['data']); + if (is_null($acct)) { + echo '$acct (1) is null, exit!'; + var_dump($acct); + exit(); + } \Log::debug('Imported ' . $payload['class'] . ' "' . $payload['data']['name'] . '".'); - $this->_repository->store($importMap, 'Account', $payload['data']['id'], $acct->id); + $this->_repository->store($importMap, 'Account', $oldPayloadId, $acct->id); $job->delete(); return; } - // but we cannot merge accounts, so we need to search first: - $acct = $this->_accounts->findByName($payload['data']['name']); + // but Firefly cannot merge other types accounts, so we need to search first: + $assetAccountType = $this->_accounts->findAccountType('Asset account'); + + // we need to find it by name AND type. + $acct = $this->_accounts->findByNameAndAccountType($payload['data']['name'], $assetAccountType); if (is_null($acct)) { // store new one! $acct = $this->_accounts->store((array)$payload['data']); @@ -322,7 +338,7 @@ class Import 'Updated transactions (#' . $journal->id . '), #' . $transaction->id . '\'s Account.' ); } else { - \Log::error('Found account type: "' . $accountType.'" instead of expected "Import account"'); + \Log::error('Found account type: "' . $accountType . '" instead of expected "Import account"'); } } @@ -532,8 +548,14 @@ class Import // find or create the account type for the import account. // find or create the account for the import account. $accountType = $this->_accounts->findAccountType('Import account'); - $importAccount = $this->_accounts->createOrFind('Import account', $accountType); - + $importAccount = $this->_accounts->firstOrCreate( + [ + 'account_type_id' => $accountType->id, + 'name' => 'Import account', + 'user_id' => $user->id, + 'active' => 1, + ] + ); // if amount is more than zero, move from $importAccount $amount = floatval($payload['data']['amount']); @@ -560,6 +582,7 @@ class Import if (is_null($current)) { + $journal = $this->_journals->createSimpleJournal( $accountFrom, $accountTo, $payload['data']['description'], $amount, $date diff --git a/app/lib/Firefly/Storage/Account/AccountRepositoryInterface.php b/app/lib/Firefly/Storage/Account/AccountRepositoryInterface.php index 288fb8ce7e..781d9f41d9 100644 --- a/app/lib/Firefly/Storage/Account/AccountRepositoryInterface.php +++ b/app/lib/Firefly/Storage/Account/AccountRepositoryInterface.php @@ -47,6 +47,14 @@ interface AccountRepositoryInterface */ public function find($accountId); + /** + * @param $name + * @param \AccountType $type + * + * @return \Account + */ + public function findByNameAndAccountType($name, \AccountType $type); + /** * @param $type * @return mixed diff --git a/app/lib/Firefly/Storage/Account/EloquentAccountRepository.php b/app/lib/Firefly/Storage/Account/EloquentAccountRepository.php index 968fa1b841..d79d1552a2 100644 --- a/app/lib/Firefly/Storage/Account/EloquentAccountRepository.php +++ b/app/lib/Firefly/Storage/Account/EloquentAccountRepository.php @@ -43,6 +43,16 @@ class EloquentAccountRepository implements AccountRepositoryInterface return $accounts; } + /** + * @param $name + * @param \AccountType $type + * + * @return \Account + */ + public function findByNameAndAccountType($name, \AccountType $type) { + return $this->_user->accounts()->where('name',$name)->where('account_type_id',$type->id)->first(); + } + /** * @return mixed */ @@ -155,44 +165,44 @@ class EloquentAccountRepository implements AccountRepositoryInterface // whatever the result, return the account. return $account; } -// -// /** -// * @param \Account $account -// * @param int $amount -// * @param Carbon $date -// * -// * @return bool -// * @SuppressWarnings(PHPMD.CamelCaseMethodName) -// */ -// protected function _createInitialBalance(\Account $account, $amount = 0, Carbon $date) -// { -// // get account type: -// $initialBalanceAT = \AccountType::where('type', 'Initial balance account')->first(); -// -// // create new account: -// $initial = new \Account; -// $initial->accountType()->associate($initialBalanceAT); -// $initial->user()->associate($this->_user); -// $initial->name = $account->name . ' initial balance'; -// $initial->active = 0; -// if ($initial->validate()) { -// $initial->save(); -// // create new transaction journal (and transactions): -// /** @var \Firefly\Storage\TransactionJournal\TransactionJournalRepositoryInterface $transactionJournal */ -// $transactionJournal = \App::make( -// 'Firefly\Storage\TransactionJournal\TransactionJournalRepositoryInterface' -// ); -// $transactionJournal->overruleUser($this->_user); -// -// $transactionJournal->createSimpleJournal( -// $initial, $account, 'Initial Balance for ' . $account->name, $amount, $date -// ); -// -// return true; -// } -// -// return false; -// } + + /** + * @param \Account $account + * @param int $amount + * @param Carbon $date + * + * @return bool + * @SuppressWarnings(PHPMD.CamelCaseMethodName) + */ + protected function _createInitialBalance(\Account $account, $amount = 0, Carbon $date) + { + // get account type: + $initialBalanceAT = \AccountType::where('type', 'Initial balance account')->first(); + + // create new account: + $initial = new \Account; + $initial->accountType()->associate($initialBalanceAT); + $initial->user()->associate($this->_user); + $initial->name = $account->name . ' initial balance'; + $initial->active = 0; + if ($initial->validate()) { + $initial->save(); + // create new transaction journal (and transactions): + /** @var \Firefly\Storage\TransactionJournal\TransactionJournalRepositoryInterface $transactionJournal */ + $transactionJournal = \App::make( + 'Firefly\Storage\TransactionJournal\TransactionJournalRepositoryInterface' + ); + $transactionJournal->overruleUser($this->_user); + + $transactionJournal->createSimpleJournal( + $initial, $account, 'Initial Balance for ' . $account->name, $amount, $date + ); + + return true; + } + + return false; + } /** * @param \Account $account diff --git a/app/lib/Firefly/Storage/TransactionJournal/EloquentTransactionJournalRepository.php b/app/lib/Firefly/Storage/TransactionJournal/EloquentTransactionJournalRepository.php index 34553b712d..0e72120ae5 100644 --- a/app/lib/Firefly/Storage/TransactionJournal/EloquentTransactionJournalRepository.php +++ b/app/lib/Firefly/Storage/TransactionJournal/EloquentTransactionJournalRepository.php @@ -411,7 +411,8 @@ class EloquentTransactionJournalRepository implements TransactionJournalReposito $toTransaction->description = null; $toTransaction->amount = $amountTo; if (!$toTransaction->validate()) { - throw new FireflyException('Cannot create valid transaction (to): ' . $toTransaction->errors()->first()); + + throw new FireflyException('Cannot create valid transaction (to): ' . $toTransaction->errors()->first().': ' . print_r($toAccount->toArray(),true)); } $toTransaction->save(); diff --git a/app/models/TransactionJournal.php b/app/models/TransactionJournal.php index dabd1a7364..08b3638d8c 100644 --- a/app/models/TransactionJournal.php +++ b/app/models/TransactionJournal.php @@ -106,7 +106,31 @@ use LaravelBook\Ardent\Builder; * 'Budget[] $budgets * @property-read \Illuminate\Database\Eloquent\Collection|\ * 'Category[] $categories - * @method static \TransactionJournal accountIs($account) + * @method static \TransactionJournal accountIs($account) + * @property-read \Illuminate\Database\Eloquent\Collection|\ + * 'Budget[] $budgets + * @property-read \Illuminate\Database\Eloquent\Collection|\ + * 'Category[] $categories + * @property-read \Illuminate\Database\Eloquent\Collection|\ + * 'Budget[] $budgets + * @property-read \Illuminate\Database\Eloquent\Collection|\ + * 'Category[] $categories + * @property-read \Illuminate\Database\Eloquent\Collection|\ + * 'Budget[] $budgets + * @property-read \Illuminate\Database\Eloquent\Collection|\ + * 'Category[] $categories + * @property-read \Illuminate\Database\Eloquent\Collection|\ + * 'Budget[] $budgets + * @property-read \Illuminate\Database\Eloquent\Collection|\ + * 'Category[] $categories + * @property-read \Illuminate\Database\Eloquent\Collection|\ + * 'Budget[] $budgets + * @property-read \Illuminate\Database\Eloquent\Collection|\ + * 'Category[] $categories + * @property-read \Illuminate\Database\Eloquent\Collection|\ + * 'Budget[] $budgets + * @property-read \Illuminate\Database\Eloquent\Collection|\ + * 'Category[] $categories */ class TransactionJournal extends Ardent { diff --git a/app/views/layouts/default.blade.php b/app/views/layouts/default.blade.php index 1d8e51f57a..13ba63d6d1 100644 --- a/app/views/layouts/default.blade.php +++ b/app/views/layouts/default.blade.php @@ -32,11 +32,14 @@

+ @if(isset($mainTitleIcon)) + + @endif {{$title or '(no title)'}} @if(isset($subTitle)) - @if(isset($titleIcon)) - + @if(isset($subTitleIcon)) + @endif {{$subTitle}}