From 90f2e27f1f4bc4bff50d76b0ec04848961801136 Mon Sep 17 00:00:00 2001 From: James Cole Date: Sun, 9 Oct 2016 09:32:12 +0200 Subject: [PATCH] Refactoring income and expense reports. --- app/Crud/Account/AccountCrud.php | 2 - app/Helpers/Collection/Expense.php | 32 +--- app/Helpers/Collection/Income.php | 28 +--- app/Helpers/Report/ReportHelper.php | 29 ++-- app/Http/Controllers/AccountController.php | 8 +- .../Account/AccountRepository.php | 83 ---------- .../Account/AccountRepositoryInterface.php | 30 ---- app/Repositories/Account/AccountTasker.php | 143 ++++++++++++++++++ .../Account/AccountTaskerInterface.php | 26 ++++ 9 files changed, 197 insertions(+), 184 deletions(-) diff --git a/app/Crud/Account/AccountCrud.php b/app/Crud/Account/AccountCrud.php index 8b01d018f8..03b12eba24 100644 --- a/app/Crud/Account/AccountCrud.php +++ b/app/Crud/Account/AccountCrud.php @@ -14,7 +14,6 @@ declare(strict_types = 1); namespace FireflyIII\Crud\Account; use Carbon\Carbon; -use DB; use FireflyIII\Exceptions\FireflyException; use FireflyIII\Models\Account; use FireflyIII\Models\AccountMeta; @@ -53,7 +52,6 @@ class AccountCrud implements AccountCrudInterface } - /** * @param $accountId * diff --git a/app/Helpers/Collection/Expense.php b/app/Helpers/Collection/Expense.php index 86ff175e28..7a06e4f628 100644 --- a/app/Helpers/Collection/Expense.php +++ b/app/Helpers/Collection/Expense.php @@ -12,7 +12,6 @@ declare(strict_types = 1); namespace FireflyIII\Helpers\Collection; -use FireflyIII\Models\TransactionJournal; use Illuminate\Support\Collection; use stdClass; @@ -38,36 +37,11 @@ class Expense } /** - * @param TransactionJournal $entry + * @param stdClass $entry */ - public function addOrCreateExpense(TransactionJournal $entry) + public function addOrCreateExpense(stdClass $entry) { - // add each account individually: - $destinations = TransactionJournal::destinationTransactionList($entry); - - foreach ($destinations as $transaction) { - $amount = strval($transaction->amount); - $account = $transaction->account; - if (bccomp('0', $amount) === -1) { - $amount = bcmul($amount, '-1'); - } - - $object = new stdClass; - $object->amount = $amount; - $object->name = $account->name; - $object->count = 1; - $object->id = $account->id; - - // overrule some properties: - if ($this->expenses->has($account->id)) { - $object = $this->expenses->get($account->id); - $object->amount = bcadd($object->amount, $amount); - $object->count++; - } - $this->expenses->put($account->id, $object); - } - - + $this->expenses->put($entry->id, $entry); } /** diff --git a/app/Helpers/Collection/Income.php b/app/Helpers/Collection/Income.php index 6c1075b606..1bd6036150 100644 --- a/app/Helpers/Collection/Income.php +++ b/app/Helpers/Collection/Income.php @@ -12,7 +12,6 @@ declare(strict_types = 1); namespace FireflyIII\Helpers\Collection; -use FireflyIII\Models\TransactionJournal; use Illuminate\Support\Collection; use stdClass; @@ -39,32 +38,11 @@ class Income } /** - * @param TransactionJournal $entry + * @param stdClass $entry */ - public function addOrCreateIncome(TransactionJournal $entry) + public function addOrCreateIncome(stdClass $entry) { - // add each account individually: - $sources = TransactionJournal::sourceTransactionList($entry); - - foreach ($sources as $transaction) { - $amount = strval($transaction->amount); - $account = $transaction->account; - $amount = bcmul($amount, '-1'); - - $object = new stdClass; - $object->amount = $amount; - $object->name = $account->name; - $object->count = 1; - $object->id = $account->id; - - // overrule some properties: - if ($this->incomes->has($account->id)) { - $object = $this->incomes->get($account->id); - $object->amount = bcadd($object->amount, $amount); - $object->count++; - } - $this->incomes->put($account->id, $object); - } + $this->incomes->put($entry->id, $entry); } diff --git a/app/Helpers/Report/ReportHelper.php b/app/Helpers/Report/ReportHelper.php index d13dd53b70..6f1a1f7529 100644 --- a/app/Helpers/Report/ReportHelper.php +++ b/app/Helpers/Report/ReportHelper.php @@ -24,7 +24,7 @@ use FireflyIII\Models\Bill; use FireflyIII\Models\Category; use FireflyIII\Models\Tag; use FireflyIII\Models\TransactionJournal; -use FireflyIII\Repositories\Account\AccountRepositoryInterface; +use FireflyIII\Repositories\Account\AccountTaskerInterface; use FireflyIII\Repositories\Bill\BillRepositoryInterface; use FireflyIII\Repositories\Budget\BudgetRepositoryInterface; use FireflyIII\Repositories\Category\CategoryRepositoryInterface; @@ -32,6 +32,7 @@ use FireflyIII\Repositories\Tag\TagRepositoryInterface; use Illuminate\Database\Eloquent\Builder; use Illuminate\Database\Query\JoinClause; use Illuminate\Support\Collection; +use stdClass; /** * Class ReportHelper @@ -146,14 +147,14 @@ class ReportHelper implements ReportHelperInterface public function getExpenseReport(Carbon $start, Carbon $end, Collection $accounts): Expense { $object = new Expense; - /** @var AccountRepositoryInterface $repos */ - $repos = app(AccountRepositoryInterface::class); - $journals = $repos->expensesInPeriod($accounts, $start, $end); - /** @var TransactionJournal $entry */ - foreach ($journals as $entry) { - $amount = TransactionJournal::amount($entry); - $object->addToTotal($amount); + /** @var AccountTaskerInterface $tasker */ + $tasker = app(AccountTaskerInterface::class); + $collection = $tasker->expenseReport($accounts, $accounts, $start, $end); + + /** @var stdClass $entry */ + foreach ($collection as $entry) { + $object->addToTotal($entry->amount); $object->addOrCreateExpense($entry); } @@ -172,13 +173,13 @@ class ReportHelper implements ReportHelperInterface public function getIncomeReport(Carbon $start, Carbon $end, Collection $accounts): Income { $object = new Income; - /** @var AccountRepositoryInterface $repos */ - $repos = app(AccountRepositoryInterface::class); - $journals = $repos->incomesInPeriod($accounts, $start, $end); + /** @var AccountTaskerInterface $tasker */ + $tasker = app(AccountTaskerInterface::class); + $collection = $tasker->incomeReport($accounts, $accounts, $start, $end); - foreach ($journals as $entry) { - $amount = TransactionJournal::amount($entry); - $object->addToTotal($amount); + /** @var stdClass $entry */ + foreach ($collection as $entry) { + $object->addToTotal($entry->amount); $object->addOrCreateIncome($entry); } diff --git a/app/Http/Controllers/AccountController.php b/app/Http/Controllers/AccountController.php index aabab3107d..2d0f070bbb 100644 --- a/app/Http/Controllers/AccountController.php +++ b/app/Http/Controllers/AccountController.php @@ -247,7 +247,13 @@ class AccountController extends Controller // $entries = $cache->get(); // return view('accounts.show', compact('account', 'what', 'entries', 'subTitleIcon', 'journals', 'subTitle')); } - $assets = $crud->getAccountsByType([AccountType::ASSET, AccountType::DEFAULT]); + + // only include asset accounts when this account is an asset: + $assets = new Collection; + if (in_array($account->accountType->type, [AccountType::ASSET, AccountType::DEFAULT])) { + $assets = $crud->getAccountsByType([AccountType::ASSET, AccountType::DEFAULT]); + } + while ($end >= $start) { $end = Navigation::startOfPeriod($end, $range); $currentEnd = Navigation::endOfPeriod($end, $range); diff --git a/app/Repositories/Account/AccountRepository.php b/app/Repositories/Account/AccountRepository.php index 0dc0072b50..dd67f0f60d 100644 --- a/app/Repositories/Account/AccountRepository.php +++ b/app/Repositories/Account/AccountRepository.php @@ -84,46 +84,6 @@ class AccountRepository implements AccountRepositoryInterface return true; } - /** - * This method will call AccountRepositoryInterface::journalsInPeriod and get all withdrawaks made from the given $accounts, - * as well as the transfers that move away from those $accounts. This is a slightly sharper selection - * than made by journalsInPeriod itself. - * - * @param Collection $accounts - * @param Carbon $start - * @param Carbon $end - * - * @see AccountRepositoryInterface::journalsInPeriod - * - * @return Collection - */ - public function expensesInPeriod(Collection $accounts, Carbon $start, Carbon $end): Collection - { - $types = [TransactionType::WITHDRAWAL, TransactionType::TRANSFER]; - $journals = $this->journalsInPeriod($accounts, $types, $start, $end); - $accountIds = $accounts->pluck('id')->toArray(); - - // filter because some of these journals are still too much. - $journals = $journals->filter( - function (TransactionJournal $journal) use ($accountIds) { - if ($journal->transaction_type_type == TransactionType::WITHDRAWAL) { - return true; - } - /* - * The source of a transfer must be one of the $accounts in order to - * be included. Otherwise, it would not be an expense. - */ - if (in_array($journal->source_account_id, $accountIds)) { - return true; - } - - return false; - } - ); - - return $journals; - } - /** * @param Account $account * @@ -258,49 +218,6 @@ class AccountRepository implements AccountRepositoryInterface return $accounts; } - /** - * This method will call AccountRepositoryInterface::journalsInPeriod and get all deposits made to the given $accounts, - * as well as the transfers that move away to those $accounts. This is a slightly sharper selection - * than made by journalsInPeriod itself. - * - * @param Collection $accounts - * @param Carbon $start - * @param Carbon $end - * - * @see AccountRepositoryInterface::journalsInPeriod - * - * @return Collection - */ - public function incomesInPeriod(Collection $accounts, Carbon $start, Carbon $end): Collection - { - $types = [TransactionType::DEPOSIT, TransactionType::TRANSFER]; - $journals = $this->journalsInPeriod($accounts, $types, $start, $end); - $accountIds = $accounts->pluck('id')->toArray(); - - // filter because some of these journals are still too much. - $journals = $journals->filter( - function (TransactionJournal $journal) use ($accountIds) { - if ($journal->transaction_type_type == TransactionType::DEPOSIT) { - return true; - } - /* - * The destination of a transfer must be one of the $accounts in order to - * be included. Otherwise, it would not be income. - */ - $destinations = TransactionJournal::destinationAccountList($journal)->pluck('id')->toArray(); - - if (count(array_intersect($destinations, $accountIds)) > 0) { - // at least one of $target is in $haystack - return true; - } - - return false; - } - ); - - return $journals; - } - /** * @param Collection $accounts * @param array $types diff --git a/app/Repositories/Account/AccountRepositoryInterface.php b/app/Repositories/Account/AccountRepositoryInterface.php index ae253ba416..83f4b9b84d 100644 --- a/app/Repositories/Account/AccountRepositoryInterface.php +++ b/app/Repositories/Account/AccountRepositoryInterface.php @@ -46,21 +46,6 @@ interface AccountRepositoryInterface */ public function destroy(Account $account, Account $moveTo): bool; - /** - * This method will call AccountRepositoryInterface::journalsInPeriod and get all withdrawaks made from the given $accounts, - * as well as the transfers that move away from those $accounts. This is a slightly sharper selection - * than made by journalsInPeriod itself. - * - * @param Collection $accounts - * @param Carbon $start - * @param Carbon $end - * - * @see AccountRepositoryInterface::journalsInPeriod - * - * @return Collection - */ - public function expensesInPeriod(Collection $accounts, Carbon $start, Carbon $end): Collection; - /** * @param Account $account * @@ -96,21 +81,6 @@ interface AccountRepositoryInterface */ public function getSavingsAccounts(Carbon $start, Carbon $end): Collection; - /** - * This method will call AccountRepositoryInterface::journalsInPeriod and get all deposits made to the given $accounts, - * as well as the transfers that move to to those $accounts. This is a slightly sharper selection - * than made by journalsInPeriod itself. - * - * @param Collection $accounts - * @param Carbon $start - * @param Carbon $end - * - * @see AccountRepositoryInterface::journalsInPeriod - * - * @return Collection - */ - public function incomesInPeriod(Collection $accounts, Carbon $start, Carbon $end): Collection; - /** * @param Collection $accounts * @param array $types diff --git a/app/Repositories/Account/AccountTasker.php b/app/Repositories/Account/AccountTasker.php index f9933ed89b..f920981e52 100644 --- a/app/Repositories/Account/AccountTasker.php +++ b/app/Repositories/Account/AccountTasker.php @@ -14,6 +14,8 @@ declare(strict_types = 1); namespace FireflyIII\Repositories\Account; use Carbon\Carbon; +use Crypt; +use DB; use FireflyIII\Helpers\Collection\Account as AccountCollection; use FireflyIII\Models\Account; use FireflyIII\Models\Transaction; @@ -21,6 +23,7 @@ use FireflyIII\User; use Illuminate\Database\Query\JoinClause; use Illuminate\Support\Collection; use Log; +use stdClass; use Steam; /** @@ -101,6 +104,35 @@ class AccountTasker implements AccountTaskerInterface } + /** + * @param Collection $accounts + * @param Collection $excluded + * @param Carbon $start + * @param Carbon $end + * + * @return Collection + * @see self::financialReport + * + */ + public function expenseReport(Collection $accounts, Collection $excluded, Carbon $start, Carbon $end): Collection + { + $idList = [ + 'accounts' => $accounts->pluck('id')->toArray(), + 'exclude' => $excluded->pluck('id')->toArray(), + ]; + + Log::debug( + 'Now calling expenseReport.', + ['accounts' => $idList['accounts'], 'excluded' => $idList['exclude'], + 'start' => $start->format('Y-m-d'), + 'end' => $end->format('Y-m-d'), + ] + ); + + return $this->financialReport($idList, $start, $end, false); + + } + /** * @param Carbon $start * @param Carbon $end @@ -157,6 +189,35 @@ class AccountTasker implements AccountTaskerInterface return $object; } + /** + * @param Collection $accounts + * @param Collection $excluded + * @param Carbon $start + * @param Carbon $end + * + * @see AccountTasker::financialReport() + * + * @return Collection + * + */ + public function incomeReport(Collection $accounts, Collection $excluded, Carbon $start, Carbon $end): Collection + { + $idList = [ + 'accounts' => $accounts->pluck('id')->toArray(), + 'exclude' => $excluded->pluck('id')->toArray(), + ]; + + Log::debug( + 'Now calling expenseReport.', + ['accounts' => $idList['accounts'], 'excluded' => $idList['exclude'], + 'start' => $start->format('Y-m-d'), + 'end' => $end->format('Y-m-d'), + ] + ); + + return $this->financialReport($idList, $start, $end, true); + } + /** * Will return how much money has been going out (ie. spent) by the given account(s). * Alternatively, will return how much money has been coming in (ie. earned) by the given accounts. @@ -214,4 +275,86 @@ class AccountTasker implements AccountTaskerInterface return $sum; } + + /** + * + * This method will determin how much has flown (in the given period) from OR to $accounts to/from anywhere else, + * except $excluded. This could be a list of incomes, or a list of expenses. This method shows + * the name, the amount and the number of transactions. It is a summary, and only used in some reports. + * + * $incoming=true a list of incoming money (earnings) + * $incoming=false a list of outgoing money (expenses). + * + * @param array $accounts + * @param Carbon $start + * @param Carbon $end + * @param bool $incoming + * + * @return Collection + */ + protected function financialReport(array $accounts, Carbon $start, Carbon $end, bool $incoming): Collection + { + $collection = new Collection; + $joinModifier = $incoming ? '<' : '>'; + $selection = $incoming ? '>' : '<'; + $query = Transaction + ::distinct() + ->leftJoin('transaction_journals', 'transaction_journals.id', '=', 'transactions.transaction_journal_id') + ->leftJoin( + 'transactions as other_side', function (JoinClause $join) use ($joinModifier) { + $join->on('transaction_journals.id', '=', 'other_side.transaction_journal_id')->where('other_side.amount', $joinModifier, 0); + } + ) + ->leftJoin('accounts as other_account', 'other_account.id', '=', 'other_side.account_id') + ->where('transaction_journals.date', '>=', $start->format('Y-m-d')) + ->where('transaction_journals.date', '<=', $end->format('Y-m-d')) + ->where('transaction_journals.user_id', $this->user->id) + ->whereNull('transactions.deleted_at') + ->whereNull('transaction_journals.deleted_at') + ->whereIn('transactions.account_id', $accounts['accounts']) + ->where('other_side.amount', '=', DB::raw('transactions.amount * -1')) + ->where('transactions.amount', $selection, 0) + ->orderBy('transactions.amount'); + + if (count($accounts['exclude']) > 0) { + $query->whereNotIn('other_side.account_id', $accounts['exclude']); + } + $set = $query->get( + [ + 'other_side.account_id', + 'other_account.name', + 'other_account.encrypted', + 'transactions.amount', + ] + ); + // summarize ourselves: + $temp = []; + foreach ($set as $entry) { + // save into $temp: + $id = intval($entry->account_id); + if (isset($temp[$id])) { + $temp[$id]['count']++; + $temp[$id]['amount'] = bcadd($temp[$id]['amount'], $entry->amount); + } + if (!isset($temp[$id])) { + $temp[$id] = [ + 'name' => intval($entry->encrypted) === 1 ? Crypt::decrypt($entry->name) : $entry->name, + 'amount' => $entry->amount, + 'count' => 1, + ]; + } + } + + // loop $temp and create collection: + foreach ($temp as $key => $entry) { + $object = new stdClass(); + $object->id = $key; + $object->name = $entry['name']; + $object->count = $entry['count']; + $object->amount = $entry['amount']; + $collection->push($object); + } + + return $collection; + } } \ No newline at end of file diff --git a/app/Repositories/Account/AccountTaskerInterface.php b/app/Repositories/Account/AccountTaskerInterface.php index 52dcf95425..15a95041cd 100644 --- a/app/Repositories/Account/AccountTaskerInterface.php +++ b/app/Repositories/Account/AccountTaskerInterface.php @@ -49,6 +49,19 @@ interface AccountTaskerInterface */ public function amountOutInPeriod(Collection $accounts, Collection $excluded, Carbon $start, Carbon $end): string; + /** + * @param Collection $accounts + * @param Collection $excluded + * @param Carbon $start + * @param Carbon $end + * + * @see AccountTasker::financialReport() + * + * @return Collection + * + */ + public function expenseReport(Collection $accounts, Collection $excluded, Carbon $start, Carbon $end): Collection; + /** * @param Carbon $start * @param Carbon $end @@ -58,4 +71,17 @@ interface AccountTaskerInterface */ public function getAccountReport(Carbon $start, Carbon $end, Collection $accounts): AccountCollection; + /** + * @param Collection $accounts + * @param Collection $excluded + * @param Carbon $start + * @param Carbon $end + * + * @see AccountTasker::financialReport() + * + * @return Collection + * + */ + public function incomeReport(Collection $accounts, Collection $excluded, Carbon $start, Carbon $end): Collection; + } \ No newline at end of file