From 44d189d7d336c0c0568b947c14ad9f3c19438924 Mon Sep 17 00:00:00 2001 From: James Cole Date: Wed, 12 Nov 2014 21:19:31 +0100 Subject: [PATCH] Fixed a bug where the transaction list was a giant mess. --- app/controllers/GoogleTableController.php | 139 +++++++++--------- app/controllers/TransactionController.php | 2 +- app/lib/FireflyIII/Database/Account.php | 139 ++++++++++-------- .../Database/TransactionJournal.php | 25 +++- .../FireflyIII/Database/TransactionType.php | 14 +- 5 files changed, 180 insertions(+), 139 deletions(-) diff --git a/app/controllers/GoogleTableController.php b/app/controllers/GoogleTableController.php index 2480dd786c..a3a26d673e 100644 --- a/app/controllers/GoogleTableController.php +++ b/app/controllers/GoogleTableController.php @@ -1,5 +1,4 @@ id); + $edit = route('accounts.edit', $entry->id); $delete = route('accounts.delete', $entry->id); - $show = route('accounts.show', $entry->id); + $show = route('accounts.show', $entry->id); $chart->addRow($entry->id, $edit, $delete, $show, $entry->name, $entry->balance()); } @@ -90,7 +89,7 @@ class GoogleTableController extends BaseController } /** - * @param Component $component + * @param Component $component * @param LimitRepetition $repetition */ public function transactionsByComponent(Component $component, LimitRepetition $repetition = null) @@ -115,26 +114,26 @@ class GoogleTableController extends BaseController if (is_null($repetition)) { $journals = $component->transactionjournals()->with(['budgets', 'categories', 'transactions', 'transactions.account'])->orderBy('date', 'DESC') - ->get(); + ->get(); } else { $journals = $component->transactionjournals()->with(['budgets', 'categories', 'transactions', 'transactions.account'])->after( $repetition->startdate ) - ->before($repetition->enddate)->orderBy('date', 'DESC')->get(); + ->before($repetition->enddate)->orderBy('date', 'DESC')->get(); } /** @var TransactionJournal $transaction */ foreach ($journals as $journal) { - $date = $journal->date; + $date = $journal->date; $descriptionURL = route('transactions.show', $journal->id); - $description = $journal->description; + $description = $journal->description; /** @var Transaction $transaction */ foreach ($journal->transactions as $transaction) { if (floatval($transaction->amount) > 0) { $amount = floatval($transaction->amount); - $to = $transaction->account->name; - $toURL = route('accounts.show', $transaction->account->id); + $to = $transaction->account->name; + $toURL = route('accounts.show', $transaction->account->id); } else { - $from = $transaction->account->name; + $from = $transaction->account->name; $fromURL = route('accounts.show', $transaction->account->id); } @@ -149,15 +148,15 @@ class GoogleTableController extends BaseController if (isset($journal->categories[0])) { $categoryURL = route('categories.show', $journal->categories[0]->id); - $category = $journal->categories[0]->name; + $category = $journal->categories[0]->name; } else { $categoryURL = ''; - $category = ''; + $category = ''; } - $id = $journal->id; - $edit = route('transactions.edit', $journal->id); + $id = $journal->id; + $edit = route('transactions.edit', $journal->id); $delete = route('transactions.delete', $journal->id); $chart->addRow( $id, $edit, $delete, $date, $descriptionURL, $description, $amount, $fromURL, $from, $toURL, $to, $budgetURL, $component, $categoryURL, @@ -232,6 +231,7 @@ class GoogleTableController extends BaseController $chart->addColumn('Description_URL', 'string'); $chart->addColumn('Description', 'string'); $chart->addColumn('Amount', 'number'); + $chart->addColumn('From_URL', 'string'); $chart->addColumn('From', 'string'); $chart->addColumn('To_URL', 'string'); @@ -259,56 +259,53 @@ class GoogleTableController extends BaseController break; } - /** @var Transaction $transaction */ - foreach ($list as $transaction) { - $date = $transaction->transactionJournal->date; - $descriptionURL = route('transactions.show', $transaction->transaction_journal_id); - $description = $transaction->transactionJournal->description; - $amount = floatval($transaction->amount); + /** @var TransactionJournal $journal */ + foreach ($list as $journal) { + $date = $journal->date; + $descriptionURL = route('transactions.show', $journal->id); + $description = $journal->description; + $amount = floatval(-0.01); // TODO + $id = $journal->id; + + + if ($journal->transactions[0]->amount < 0) { + + $fromURL = route('accounts.show', $journal->transactions[0]->account->id); + $fromName = $journal->transactions[0]->account->name; + $amount = floatval($journal->transactions[0]->amount); + + $toURL = route('accounts.show', $journal->transactions[1]->account->id); + $toName = $journal->transactions[1]->account->name; - if ($transaction->transactionJournal->transactions[0]->account->id == $account->id) { - $opposingAccountURI = route('accounts.show', $transaction->transactionJournal->transactions[1]->account->id); - $opposingAccountName = $transaction->transactionJournal->transactions[1]->account->name; } else { - $opposingAccountURI = route('accounts.show', $transaction->transactionJournal->transactions[0]->account->id); - $opposingAccountName = $transaction->transactionJournal->transactions[0]->account->name; + $fromURL = route('accounts.show', $journal->transactions[1]->account->id); + $fromName = $journal->transactions[1]->account->name; + $amount = floatval($journal->transactions[1]->amount); + + $toURL = route('accounts.show', $journal->transactions[0]->account->id); + $toName = $journal->transactions[0]->account->name; } - if (isset($transaction->transactionJournal->budgets[0])) { - $budgetURL = route('budgets.show', $transaction->transactionJournal->budgets[0]->id); - $budget = $transaction->transactionJournal->budgets[0]->name; + if (isset($journal->budgets[0])) { + $budgetURL = route('budgets.show', $journal->budgets[0]->id); + $budget = $journal->budgets[0]->name; } else { $budgetURL = ''; - $budget = ''; + $budget = ''; } - if (isset($transaction->transactionJournal->categories[0])) { - $categoryURL = route('categories.show', $transaction->transactionJournal->categories[0]->id); - $category = $transaction->transactionJournal->categories[0]->name; + if (isset($journal->categories[0])) { + $categoryURL = route('categories.show', $journal->categories[0]->id); + $category = $journal->categories[0]->name; } else { $categoryURL = ''; - $category = ''; + $category = ''; } - - - if ($amount < 0) { - $from = $account->name; - $fromURL = route('accounts.show', $account->id); - - $to = $opposingAccountName; - $toURL = $opposingAccountURI; - } else { - $to = $account->name; - $toURL = route('accounts.show', $account->id); - - $from = $opposingAccountName; - $fromURL = $opposingAccountURI; - } - - $id = $transaction->transactionJournal->id; - $edit = route('transactions.edit', $transaction->transactionJournal->id); - $delete = route('transactions.delete', $transaction->transactionJournal->id); + $edit = route('transactions.edit', $journal->id); + $delete = route('transactions.delete', $journal->id); $chart->addRow( - $id, $edit, $delete, $date, $descriptionURL, $description, $amount, $fromURL, $from, $toURL, $to, $budgetURL, $budget, $categoryURL, $category + $id, $edit, $delete, $date, $descriptionURL, $description, $amount, + $fromURL, $fromName, $toURL, $toName, + $budgetURL, $budget, $categoryURL, $category ); } @@ -344,63 +341,63 @@ class GoogleTableController extends BaseController /* * Find transactions: */ - $accountID = $account->id; + $accountID = $account->id; $transactions = $account->transactions()->with( ['transactionjournal', 'transactionjournal.transactions' => function ($q) use ($accountID) { $q->where('account_id', '!=', $accountID); }, 'transactionjournal.budgets', 'transactionjournal.transactiontype', - 'transactionjournal.categories'] + 'transactionjournal.categories'] )->before(Session::get('end'))->after( Session::get('start') )->orderBy('date', 'DESC')->get(); /** @var Transaction $transaction */ foreach ($transactions as $transaction) { - $date = $transaction->transactionJournal->date; + $date = $transaction->transactionJournal->date; $descriptionURL = route('transactions.show', $transaction->transaction_journal_id); - $description = $transaction->transactionJournal->description; - $amount = floatval($transaction->amount); + $description = $transaction->transactionJournal->description; + $amount = floatval($transaction->amount); if ($transaction->transactionJournal->transactions[0]->account->id == $account->id) { - $opposingAccountURI = route('accounts.show', $transaction->transactionJournal->transactions[1]->account->id); + $opposingAccountURI = route('accounts.show', $transaction->transactionJournal->transactions[1]->account->id); $opposingAccountName = $transaction->transactionJournal->transactions[1]->account->name; } else { - $opposingAccountURI = route('accounts.show', $transaction->transactionJournal->transactions[0]->account->id); + $opposingAccountURI = route('accounts.show', $transaction->transactionJournal->transactions[0]->account->id); $opposingAccountName = $transaction->transactionJournal->transactions[0]->account->name; } if (isset($transaction->transactionJournal->budgets[0])) { $budgetURL = route('budgets.show', $transaction->transactionJournal->budgets[0]->id); - $budget = $transaction->transactionJournal->budgets[0]->name; + $budget = $transaction->transactionJournal->budgets[0]->name; } else { $budgetURL = ''; - $budget = ''; + $budget = ''; } if (isset($transaction->transactionJournal->categories[0])) { $categoryURL = route('categories.show', $transaction->transactionJournal->categories[0]->id); - $category = $transaction->transactionJournal->categories[0]->name; + $category = $transaction->transactionJournal->categories[0]->name; } else { $categoryURL = ''; - $category = ''; + $category = ''; } if ($amount < 0) { - $from = $account->name; + $from = $account->name; $fromURL = route('accounts.show', $account->id); - $to = $opposingAccountName; + $to = $opposingAccountName; $toURL = $opposingAccountURI; } else { - $to = $account->name; + $to = $account->name; $toURL = route('accounts.show', $account->id); - $from = $opposingAccountName; + $from = $opposingAccountName; $fromURL = $opposingAccountURI; } - $id = $transaction->transactionJournal->id; - $edit = route('transactions.edit', $transaction->transactionJournal->id); + $id = $transaction->transactionJournal->id; + $edit = route('transactions.edit', $transaction->transactionJournal->id); $delete = route('transactions.delete', $transaction->transactionJournal->id); $chart->addRow( $id, $edit, $delete, $date, $descriptionURL, $description, $amount, $fromURL, $from, $toURL, $to, $budgetURL, $budget, $categoryURL, $category diff --git a/app/controllers/TransactionController.php b/app/controllers/TransactionController.php index 1f6bb3008a..32af9b1acc 100644 --- a/app/controllers/TransactionController.php +++ b/app/controllers/TransactionController.php @@ -301,7 +301,7 @@ class TransactionController extends BaseController if ($data['post_submit_action'] == 'create_another') { return Redirect::route('transactions.create', $what); } else { - return Redirect::route('transactions.index'); + return Redirect::route('transactions.index',$what); } break; case 'validate_only': diff --git a/app/lib/FireflyIII/Database/Account.php b/app/lib/FireflyIII/Database/Account.php index 60726af53c..015ed63cf1 100644 --- a/app/lib/FireflyIII/Database/Account.php +++ b/app/lib/FireflyIII/Database/Account.php @@ -6,9 +6,9 @@ use Carbon\Carbon; use FireflyIII\Database\Ifaces\AccountInterface; use FireflyIII\Database\Ifaces\CommonDatabaseCalls; use FireflyIII\Database\Ifaces\CUD; +use Illuminate\Support\Collection; use Illuminate\Support\MessageBag; use LaravelBook\Ardent\Ardent; -use Illuminate\Support\Collection; /** * Class Account @@ -29,13 +29,13 @@ class Account implements CUD, CommonDatabaseCalls, AccountInterface /** * @param Ardent $model - * @param array $data + * @param array $data * * @return bool */ public function update(Ardent $model, array $data) { - $model->name = $data['name']; + $model->name = $data['name']; $model->active = isset($data['active']) ? intval($data['active']) : 0; $model->save(); @@ -58,6 +58,23 @@ class Account implements CUD, CommonDatabaseCalls, AccountInterface return true; } + /** + * @param \Account $account + * + * @return \TransactionJournal|null + */ + public function openingBalanceTransaction(\Account $account) + { + return \TransactionJournal::withRelevantData() + ->accountIs($account) + ->leftJoin( + 'transaction_types', 'transaction_types.id', '=', + 'transaction_journals.transaction_type_id' + ) + ->where('transaction_types.type', 'Opening balance') + ->first(['transaction_journals.*']); + } + /** * Get all asset accounts. Optional JSON based parameters. * @@ -71,21 +88,6 @@ class Account implements CUD, CommonDatabaseCalls, AccountInterface } - /** - * @param \Account $account - * - * @return \Account|null - */ - public function findInitialBalanceAccount(\Account $account) - { - /** @var \FireflyIII\Database\AccountType $acctType */ - $acctType = \App::make('FireflyIII\Database\AccountType'); - - $accountType = $acctType->findByWhat('initial'); - - return $this->getUser()->accounts()->where('account_type_id', $accountType->id)->where('name', 'LIKE', $account->name . '%')->first(); - } - /** * @param array $types * @param array $parameters @@ -146,19 +148,26 @@ class Account implements CUD, CommonDatabaseCalls, AccountInterface } /** - * @return int + * @param \Account $account + * + * @return \Account|null */ - public function countAssetAccounts() + public function findInitialBalanceAccount(\Account $account) { - return $this->countAccountsByType(['Default account', 'Asset account']); + /** @var \FireflyIII\Database\AccountType $acctType */ + $acctType = \App::make('FireflyIII\Database\AccountType'); + + $accountType = $acctType->findByWhat('initial'); + + return $this->getUser()->accounts()->where('account_type_id', $accountType->id)->where('name', 'LIKE', $account->name . '%')->first(); } /** * @return int */ - public function countExpenseAccounts() + public function countAssetAccounts() { - return $this->countAccountsByType(['Expense account', 'Beneficiary account']); + return $this->countAccountsByType(['Default account', 'Asset account']); } /** @@ -171,6 +180,14 @@ class Account implements CUD, CommonDatabaseCalls, AccountInterface return $this->getUser()->accounts()->accountTypeIn($types)->count(); } + /** + * @return int + */ + public function countExpenseAccounts() + { + return $this->countAccountsByType(['Expense account', 'Beneficiary account']); + } + /** * @param array $parameters * @@ -200,7 +217,24 @@ class Account implements CUD, CommonDatabaseCalls, AccountInterface */ public function find($id) { - // TODO: Implement find() method. + return $this->getUser()->accounts()->find($id); + } + + public function firstExpenseAccountOrCreate($name) + { + /** @var \FireflyIII\Database\AccountType $accountTypeRepos */ + $accountTypeRepos = \App::make('FireflyIII\Database\AccountType'); + + $accountType = $accountTypeRepos->findByWhat('expense'); + + $data = [ + 'user_id' => $this->getUser()->id, + 'account_type_id' => $accountType->id, + 'name' => $name, + 'active' => 1 + ]; + return \Account::firstOrCreate($data); + } /** @@ -247,23 +281,6 @@ class Account implements CUD, CommonDatabaseCalls, AccountInterface } - /** - * @param \Account $account - * - * @return \TransactionJournal|null - */ - public function openingBalanceTransaction(\Account $account) - { - return \TransactionJournal::withRelevantData() - ->accountIs($account) - ->leftJoin( - 'transaction_types', 'transaction_types.id', '=', - 'transaction_journals.transaction_type_id' - ) - ->where('transaction_types.type', 'Opening balance') - ->first(['transaction_journals.*']); - } - /** * Validates a model. Returns an array containing MessageBags * errors/warnings/successes. @@ -287,9 +304,9 @@ class Account implements CUD, CommonDatabaseCalls, AccountInterface */ public function validate(array $model) { - $warnings = new MessageBag; + $warnings = new MessageBag; $successes = new MessageBag; - $errors = new MessageBag; + $errors = new MessageBag; /* * Name validation: @@ -343,8 +360,8 @@ class Account implements CUD, CommonDatabaseCalls, AccountInterface $successes->add('openingbalancedate', 'OK'); } return [ - 'errors' => $errors, - 'warnings' => $warnings, + 'errors' => $errors, + 'warnings' => $warnings, 'successes' => $successes ]; } @@ -365,12 +382,12 @@ class Account implements CUD, CommonDatabaseCalls, AccountInterface $accountType = $acctType->findByWhat($data['what']); - $data['user_id'] = $this->getUser()->id; + $data['user_id'] = $this->getUser()->id; $data['account_type_id'] = $accountType->id; - $data['active'] = isset($data['active']) && $data['active'] === '1' ? 1 : 0; + $data['active'] = isset($data['active']) && $data['active'] === '1' ? 1 : 0; - $data = array_except($data, array('_token', 'what')); + $data = array_except($data, array('_token', 'what')); $account = new \Account($data); if (!$account->validate()) { var_dump($account->errors()->all()); @@ -391,16 +408,16 @@ class Account implements CUD, CommonDatabaseCalls, AccountInterface /** * @param \Account $account - * @param array $data + * @param array $data * * @return bool */ public function storeInitialBalance(\Account $account, array $data) { - $opposingData = [ - 'name' => $account->name . ' Initial Balance', + $opposingData = [ + 'name' => $account->name . ' Initial Balance', 'active' => 0, - 'what' => 'initial' + 'what' => 'initial' ]; $opposingAccount = $this->store($opposingData); @@ -408,28 +425,28 @@ class Account implements CUD, CommonDatabaseCalls, AccountInterface * Create a journal from opposing to account or vice versa. */ $balance = floatval($data['openingbalance']); - $date = new Carbon($data['openingbalancedate']); + $date = new Carbon($data['openingbalancedate']); /** @var \FireflyIII\Database\TransactionJournal $tj */ $tj = \App::make('FireflyIII\Database\TransactionJournal'); if ($balance < 0) { // first transaction draws money from the new account to the opposing $from = $account; - $to = $opposingAccount; + $to = $opposingAccount; } else { // first transaction puts money into account $from = $opposingAccount; - $to = $account; + $to = $account; } // data for transaction journal: $balance = $balance < 0 ? $balance * -1 : $balance; $opening = [ - 'what' => 'opening', - 'currency' => 'EUR', - 'amount' => $balance, - 'from' => $from, - 'to' => $to, - 'date' => $date, + 'what' => 'opening', + 'currency' => 'EUR', + 'amount' => $balance, + 'from' => $from, + 'to' => $to, + 'date' => $date, 'description' => 'Opening balance for new account ' . $account->name, ]; diff --git a/app/lib/FireflyIII/Database/TransactionJournal.php b/app/lib/FireflyIII/Database/TransactionJournal.php index b97de82083..1603f3386d 100644 --- a/app/lib/FireflyIII/Database/TransactionJournal.php +++ b/app/lib/FireflyIII/Database/TransactionJournal.php @@ -240,7 +240,7 @@ class TransactionJournal implements TransactionJournalInterface, CUD, CommonData $errors->add('account_to_id', 'Invalid account selected.'); } else { - if (intval($model['account_from_id']) == intval($model['account_from_id'])) { + if (intval($model['account_from_id']) == intval($model['account_to_id'])) { $errors->add('account_to_id', 'Cannot be the same as "from" account.'); $errors->add('account_from_id', 'Cannot be the same as "to" account.'); } else { @@ -319,6 +319,9 @@ class TransactionJournal implements TransactionJournalInterface, CUD, CommonData /** @var \FireflyIII\Database\TransactionType $typeRepository */ $typeRepository = \App::make('FireflyIII\Database\TransactionType'); + /** @var \FireflyIII\Database\Account $accountRepository */ + $accountRepository = \App::make('FireflyIII\Database\Account'); + /** @var \FireflyIII\Database\TransactionCurrency $currencyRepository */ $currencyRepository = \App::make('FireflyIII\Database\TransactionCurrency'); @@ -335,17 +338,33 @@ class TransactionJournal implements TransactionJournalInterface, CUD, CommonData $journal->description = $data['description']; $journal->date = $data['date']; $journal->completed = 0; - //$journal->user_id = $this->getUser()->id; /* * This must be enough to store the journal: */ if (!$journal->validate()) { \Log::error($journal->errors()->all()); - throw new FireflyException('store() transactionjournal failed, but it should not!'); + throw new FireflyException('store() transaction journal failed, but it should not!'); } $journal->save(); + /* + * Still need to find the accounts related to the transactions. + * This depends on the type of transaction. + */ + switch ($data['what']) { + case 'withdrawal': + $data['from'] = $accountRepository->find($data['account_id']); + $data['to'] = $accountRepository->firstExpenseAccountOrCreate($data['expense_account']); + break; + case 'opening': + break; + + default: + throw new FireflyException('Cannot save transaction journal with accounts based on $what "' . $data['what'] . '".'); + break; + } + /* * Then store both transactions. */ diff --git a/app/lib/FireflyIII/Database/TransactionType.php b/app/lib/FireflyIII/Database/TransactionType.php index 0022ec36fb..0abf5e9308 100644 --- a/app/lib/FireflyIII/Database/TransactionType.php +++ b/app/lib/FireflyIII/Database/TransactionType.php @@ -3,11 +3,12 @@ namespace FireflyIII\Database; -use Illuminate\Support\Collection; -use LaravelBook\Ardent\Ardent; use FireflyIII\Database\Ifaces\CommonDatabaseCalls; use FireflyIII\Database\Ifaces\CUD; use FireflyIII\Database\Ifaces\TransactionTypeInterface; +use FireflyIII\Exception\FireflyException; +use Illuminate\Support\Collection; +use LaravelBook\Ardent\Ardent; /** * Class TransactionType @@ -98,10 +99,17 @@ class TransactionType implements TransactionTypeInterface, CUD, CommonDatabaseCa case 'opening': return \TransactionType::whereType('Opening balance')->first(); break; + case 'transfer': + return \TransactionType::whereType('Transfer')->first(); + break; + case 'withdrawal': + return \TransactionType::whereType('Withdrawal')->first(); + break; default: throw new FireflyException('Cannot find transaction type described as "' . e($what) . '".'); break; + } return null; } @@ -118,7 +126,7 @@ class TransactionType implements TransactionTypeInterface, CUD, CommonDatabaseCa /** * @param Ardent $model - * @param array $data + * @param array $data * * @return bool */