From 70c922cdc5c9d9dd282888a14a1c9f4e1c1d2d26 Mon Sep 17 00:00:00 2001 From: James Cole Date: Fri, 1 Jan 2016 11:32:08 +0100 Subject: [PATCH] Code cleanup. --- app/Helpers/Report/ReportHelper.php | 52 +---- app/Helpers/Report/ReportQuery.php | 240 ++++++-------------- app/Helpers/Report/ReportQueryInterface.php | 35 ++- app/Http/Controllers/JsonController.php | 12 +- 4 files changed, 92 insertions(+), 247 deletions(-) diff --git a/app/Helpers/Report/ReportHelper.php b/app/Helpers/Report/ReportHelper.php index 00f58a310a..3714350691 100644 --- a/app/Helpers/Report/ReportHelper.php +++ b/app/Helpers/Report/ReportHelper.php @@ -2,7 +2,6 @@ namespace FireflyIII\Helpers\Report; -use Auth; use Carbon\Carbon; use DB; use FireflyIII\Helpers\Collection\Account as AccountCollection; @@ -21,8 +20,6 @@ use FireflyIII\Models\Account; use FireflyIII\Models\Bill; use FireflyIII\Models\Budget as BudgetModel; use FireflyIII\Models\LimitRepetition; -use FireflyIII\Models\TransactionType; -use Illuminate\Database\Query\JoinClause; use Illuminate\Support\Collection; /** @@ -215,29 +212,7 @@ class ReportHelper implements ReportHelperInterface public function getIncomeReport($start, $end, Collection $accounts) { $object = new Income; - - /* - * TODO move to ReportQuery class. - */ - $ids = $accounts->pluck('id')->toArray(); - $set = Auth::user()->transactionjournals() - ->leftJoin( - 'transactions as t_from', function (JoinClause $join) { - $join->on('t_from.transaction_journal_id', '=', 'transaction_journals.id')->where('t_from.amount', '<', 0); - } - ) - ->leftJoin( - 'transactions as t_to', function (JoinClause $join) { - $join->on('t_to.transaction_journal_id', '=', 'transaction_journals.id')->where('t_to.amount', '>', 0); - } - ) - ->leftJoin('accounts', 't_from.account_id', '=', 'accounts.id') - ->transactionTypes([TransactionType::DEPOSIT, TransactionType::TRANSFER, TransactionType::OPENING_BALANCE]) - ->before($end) - ->after($start) - ->whereIn('t_to.account_id', $ids) - ->whereNotIn('t_from.account_id', $ids) - ->get(['transaction_journals.*', 't_to.amount as journalAmount', 'accounts.id as account_id', 'accounts.name as account_name']); + $set = $this->query->income($accounts, $start, $end); foreach ($set as $entry) { $object->addToTotal($entry->journalAmount); @@ -259,30 +234,7 @@ class ReportHelper implements ReportHelperInterface public function getExpenseReport($start, $end, Collection $accounts) { $object = new Expense; - - - /* - * TODO move to ReportQuery class. - */ - $ids = $accounts->pluck('id')->toArray(); - $set = Auth::user()->transactionjournals() - ->leftJoin( - 'transactions as t_from', function (JoinClause $join) { - $join->on('t_from.transaction_journal_id', '=', 'transaction_journals.id')->where('t_from.amount', '<', 0); - } - ) - ->leftJoin( - 'transactions as t_to', function (JoinClause $join) { - $join->on('t_to.transaction_journal_id', '=', 'transaction_journals.id')->where('t_to.amount', '>', 0); - } - ) - ->leftJoin('accounts', 't_to.account_id', '=', 'accounts.id') - ->transactionTypes([TransactionType::WITHDRAWAL, TransactionType::TRANSFER, TransactionType::OPENING_BALANCE]) - ->before($end) - ->after($start) - ->whereIn('t_from.account_id', $ids) - ->whereNotIn('t_to.account_id', $ids) - ->get(['transaction_journals.*', 't_from.amount as journalAmount', 'accounts.id as account_id', 'accounts.name as account_name']); + $set = $this->query->expense($accounts, $start, $end); foreach ($set as $entry) { $object->addToTotal($entry->journalAmount); // can be positive, if it's a transfer diff --git a/app/Helpers/Report/ReportQuery.php b/app/Helpers/Report/ReportQuery.php index de29c6ed29..1807541a32 100644 --- a/app/Helpers/Report/ReportQuery.php +++ b/app/Helpers/Report/ReportQuery.php @@ -4,7 +4,6 @@ namespace FireflyIII\Helpers\Report; use Auth; use Carbon\Carbon; -use Crypt; use DB; use FireflyIII\Models\Account; use FireflyIII\Models\Budget; @@ -65,176 +64,6 @@ class ReportQuery implements ReportQueryInterface ->whereNull('budget_transaction_journal.budget_id')->get(['transaction_journals.*'])->sum('amount'); } - /** - * @param Carbon $start - * @param Carbon $end - * - * @return Builder - */ - protected function queryJournalsWithTransactions(Carbon $start, Carbon $end) - { - $query = TransactionJournal:: - leftJoin( - 'transactions as t_from', function (JoinClause $join) { - $join->on('t_from.transaction_journal_id', '=', 'transaction_journals.id')->where('t_from.amount', '<', 0); - } - ) - ->leftJoin('accounts as ac_from', 't_from.account_id', '=', 'ac_from.id') - ->leftJoin( - 'account_meta as acm_from', function (JoinClause $join) { - $join->on('ac_from.id', '=', 'acm_from.account_id')->where('acm_from.name', '=', 'accountRole'); - } - ) - ->leftJoin( - 'transactions as t_to', function (JoinClause $join) { - $join->on('t_to.transaction_journal_id', '=', 'transaction_journals.id')->where('t_to.amount', '>', 0); - } - ) - ->leftJoin('accounts as ac_to', 't_to.account_id', '=', 'ac_to.id') - ->leftJoin( - 'account_meta as acm_to', function (JoinClause $join) { - $join->on('ac_to.id', '=', 'acm_to.account_id')->where('acm_to.name', '=', 'accountRole'); - } - ) - ->leftJoin('transaction_types', 'transaction_types.id', '=', 'transaction_journals.transaction_type_id'); - $query->before($end)->after($start)->where('transaction_journals.user_id', Auth::user()->id); - - return $query; - } - - -// /** -// * This method works the same way as ReportQueryInterface::incomeInPeriod does, but instead of returning results -// * will simply list the transaction journals only. This should allow any follow up counting to be accurate with -// * regards to tags. It will only get the incomes to the specified accounts. -// * -// * @param Carbon $start -// * @param Carbon $end -// * @param Collection $accounts -// * -// * @return Collection -// */ -// public function incomeInPeriod(Carbon $start, Carbon $end, Collection $accounts) -// { -// $query = $this->queryJournalsWithTransactions($start, $end); -// -// $ids = []; -// /** @var Account $account */ -// foreach ($accounts as $account) { -// $ids[] = $account->id; -// } -// -// // OR is a deposit -// // OR any transfer TO the accounts in $accounts, not FROM any of the accounts in $accounts. -// $query->where( -// function (Builder $query) use ($ids) { -// $query->where( -// function (Builder $q) { -// $q->where('transaction_types.type', TransactionType::DEPOSIT); -// } -// ); -// $query->orWhere( -// function (Builder $q) use ($ids) { -// $q->where('transaction_types.type', TransactionType::TRANSFER); -// $q->whereNotIn('ac_from.id', $ids); -// $q->whereIn('ac_to.id', $ids); -// } -// ); -// } -// ); -// -// // only include selected accounts. -// $query->whereIn('ac_to.id', $ids); -// $query->orderBy('transaction_journals.date'); -// -// // get everything -// $data = $query->get( -// ['transaction_journals.*', -// 'transaction_types.type', 'ac_from.name as name', -// 't_from.amount as from_amount', -// 't_to.amount as to_amount', -// 'ac_from.id as account_id', 'ac_from.encrypted as account_encrypted'] -// ); -// -// $data->each( -// function (TransactionJournal $journal) { -// if (intval($journal->account_encrypted) == 1) { -// $journal->name = Crypt::decrypt($journal->name); -// } -// } -// ); -// -// return $data; -// } - - /** - * See ReportQueryInterface::incomeInPeriod - * - * This method returns all "expense" journals in a certain period, which are both transfers to a shared account - * and "ordinary" withdrawals. The query used is almost equal to ReportQueryInterface::journalsByRevenueAccount but it does - * not group and returns different fields. - * - * @param Carbon $start - * @param Carbon $end - * @param Collection $accounts - * - * @return Collection - * - */ - public function expenseInPeriod(Carbon $start, Carbon $end, Collection $accounts) - { - $ids = []; - - /** @var Account $account */ - foreach ($accounts as $account) { - $ids[] = $account->id; - } - - $query = $this->queryJournalsWithTransactions($start, $end); - - // withdrawals from any account are an expense. - // transfers away, from an account in the list, to an account not in the list, are an expense. - - $query->where( - function (Builder $query) use ($ids) { - $query->where( - function (Builder $q) { - $q->where('transaction_types.type', TransactionType::WITHDRAWAL); - } - ); - $query->orWhere( - function (Builder $q) use ($ids) { - $q->where('transaction_types.type', TransactionType::TRANSFER); - $q->whereIn('ac_from.id', $ids); - $q->whereNotIn('ac_to.id', $ids); - } - ); - } - ); - - // expense goes from the selected accounts: - $query->whereIn('ac_from.id', $ids); - - $query->orderBy('transaction_journals.date'); - $data = $query->get( // get everything - ['transaction_journals.*', 'transaction_types.type', - 't_from.amount as from_amount', - 't_to.amount as to_amount', - 'ac_to.name as name', 'ac_to.id as account_id', 'ac_to.encrypted as account_encrypted'] - ); - - $data->each( - function (TransactionJournal $journal) { - if (intval($journal->account_encrypted) == 1) { - $journal->name = Crypt::decrypt($journal->name); - } - } - ); - - return $data; - } - - /** * Returns an array of the amount of money spent in the given accounts (on withdrawals, opening balances and transfers) * grouped by month like so: "2015-01" => '123.45' @@ -324,4 +153,73 @@ class ReportQuery implements ReportQueryInterface return $array; } + /** + * This method returns all the "in" transaction journals for the given account and given period. The amount + * is stored in "journalAmount". + * + * @param Collection $accounts + * @param Carbon $start + * @param Carbon $end + * + * @return Collection + */ + public function income(Collection $accounts, Carbon $start, Carbon $end) + { + $ids = $accounts->pluck('id')->toArray(); + $set = Auth::user()->transactionjournals() + ->leftJoin( + 'transactions as t_from', function (JoinClause $join) { + $join->on('t_from.transaction_journal_id', '=', 'transaction_journals.id')->where('t_from.amount', '<', 0); + } + ) + ->leftJoin( + 'transactions as t_to', function (JoinClause $join) { + $join->on('t_to.transaction_journal_id', '=', 'transaction_journals.id')->where('t_to.amount', '>', 0); + } + ) + ->leftJoin('accounts', 't_from.account_id', '=', 'accounts.id') + ->transactionTypes([TransactionType::DEPOSIT, TransactionType::TRANSFER, TransactionType::OPENING_BALANCE]) + ->before($end) + ->after($start) + ->whereIn('t_to.account_id', $ids) + ->whereNotIn('t_from.account_id', $ids) + ->get(['transaction_journals.*', 't_to.amount as journalAmount', 'accounts.id as account_id', 'accounts.name as account_name']); + + return $set; + } + + /** + * This method returns all the "out" transaction journals for the given account and given period. The amount + * is stored in "journalAmount". + * + * @param Collection $accounts + * @param Carbon $start + * @param Carbon $end + * + * @return Collection + */ + public function expense(Collection $accounts, Carbon $start, Carbon $end) + { + $ids = $accounts->pluck('id')->toArray(); + $set = Auth::user()->transactionjournals() + ->leftJoin( + 'transactions as t_from', function (JoinClause $join) { + $join->on('t_from.transaction_journal_id', '=', 'transaction_journals.id')->where('t_from.amount', '<', 0); + } + ) + ->leftJoin( + 'transactions as t_to', function (JoinClause $join) { + $join->on('t_to.transaction_journal_id', '=', 'transaction_journals.id')->where('t_to.amount', '>', 0); + } + ) + ->leftJoin('accounts', 't_to.account_id', '=', 'accounts.id') + ->transactionTypes([TransactionType::WITHDRAWAL, TransactionType::TRANSFER, TransactionType::OPENING_BALANCE]) + ->before($end) + ->after($start) + ->whereIn('t_from.account_id', $ids) + ->whereNotIn('t_to.account_id', $ids) + ->get(['transaction_journals.*', 't_from.amount as journalAmount', 'accounts.id as account_id', 'accounts.name as account_name']); + + return $set; + } } diff --git a/app/Helpers/Report/ReportQueryInterface.php b/app/Helpers/Report/ReportQueryInterface.php index 90eb200b6e..bd244a448b 100644 --- a/app/Helpers/Report/ReportQueryInterface.php +++ b/app/Helpers/Report/ReportQueryInterface.php @@ -16,33 +16,28 @@ interface ReportQueryInterface { /** - * See ReportQueryInterface::incomeInPeriod - * - * This method returns all "expense" journals in a certain period, which are both transfers to a shared account - * and "ordinary" withdrawals. The query used is almost equal to ReportQueryInterface::journalsByRevenueAccount but it does - * not group and returns different fields. + * This method returns all the "in" transaction journals for the given account and given period. The amount + * is stored in "journalAmount". * + * @param Collection $accounts * @param Carbon $start * @param Carbon $end - * @param Collection $accounts * * @return Collection - * */ - public function expenseInPeriod(Carbon $start, Carbon $end, Collection $accounts); + public function income(Collection $accounts, Carbon $start, Carbon $end); -// /** -// * This method works the same way as ReportQueryInterface::incomeInPeriod does, but instead of returning results -// * will simply list the transaction journals only. This should allow any follow up counting to be accurate with -// * regards to tags. It will only get the incomes to the specified accounts. -// * -// * @param Carbon $start -// * @param Carbon $end -// * @param Collection $accounts -// * -// * @return Collection -// */ -// public function incomeInPeriod(Carbon $start, Carbon $end, Collection $accounts); + /** + * This method returns all the "out" transaction journals for the given account and given period. The amount + * is stored in "journalAmount". + * + * @param Collection $accounts + * @param Carbon $start + * @param Carbon $end + * + * @return Collection + */ + public function expense(Collection $accounts, Carbon $start, Carbon $end); /** * Covers tags as well. diff --git a/app/Http/Controllers/JsonController.php b/app/Http/Controllers/JsonController.php index 7005084b4b..bc1b25539b 100644 --- a/app/Http/Controllers/JsonController.php +++ b/app/Http/Controllers/JsonController.php @@ -111,9 +111,9 @@ class JsonController extends Controller } /** - * @param ReportQueryInterface $reportQuery + * @param ReportQueryInterface $reportQuery * - * @param ARI $accountRepository + * @param ARI $accountRepository * * @return \Symfony\Component\HttpFoundation\Response */ @@ -131,7 +131,7 @@ class JsonController extends Controller return Response::json($cache->get()); // @codeCoverageIgnore } $accounts = $accountRepository->getAccounts(['Default account', 'Asset account', 'Cash account']); - $amount = $reportQuery->incomeInPeriod($start, $end, $accounts)->sum('to_amount'); + $amount = $reportQuery->income($accounts, $start, $end)->sum('journalAmount'); $data = ['box' => 'in', 'amount' => Amount::format($amount, false), 'amount_raw' => $amount]; $cache->store($data); @@ -140,9 +140,9 @@ class JsonController extends Controller } /** - * @param ReportQueryInterface $reportQuery + * @param ReportQueryInterface $reportQuery * - * @param ARI $accountRepository + * @param ARI $accountRepository * * @return \Symfony\Component\HttpFoundation\Response */ @@ -162,7 +162,7 @@ class JsonController extends Controller return Response::json($cache->get()); // @codeCoverageIgnore } - $amount = $reportQuery->expenseInPeriod($start, $end, $accounts)->sum('to_amount'); + $amount = $reportQuery->expense($accounts, $start, $end)->sum('journalAmount'); $data = ['box' => 'out', 'amount' => Amount::format($amount, false), 'amount_raw' => $amount]; $cache->store($data);