From 04515da0bcf4b269b475651dc03fa95381bbb673 Mon Sep 17 00:00:00 2001 From: James Cole Date: Sat, 12 Nov 2016 07:02:32 +0100 Subject: [PATCH] Fixed the charts --- .../Report/Category/MonthReportGenerator.php | 108 +++++++++++------- .../Controllers/Chart/CategoryController.php | 77 ++++++++++--- 2 files changed, 128 insertions(+), 57 deletions(-) diff --git a/app/Generator/Report/Category/MonthReportGenerator.php b/app/Generator/Report/Category/MonthReportGenerator.php index 623d65fef1..298d1ca278 100644 --- a/app/Generator/Report/Category/MonthReportGenerator.php +++ b/app/Generator/Report/Category/MonthReportGenerator.php @@ -38,6 +38,68 @@ class MonthReportGenerator implements ReportGeneratorInterface /** @var Carbon */ private $start; + /** + * @param Collection $collection + * @param array $accounts + * + * @return Collection + */ + public static function filterExpenses(Collection $collection, array $accounts): Collection + { + $result = $collection->filter( + function (Transaction $transaction) use ($accounts) { + $opposing = $transaction->opposing_account_id; + // remove internal transfer + if (in_array($opposing, $accounts)) { + Log::debug(sprintf('Filtered #%d because its opposite is in accounts.', $transaction->id)); + + return null; + } + // remove positive amount + if (bccomp($transaction->transaction_amount, '0') === 1) { + Log::debug(sprintf('Filtered #%d because amount is %f.', $transaction->id, $transaction->transaction_amount)); + + return null; + } + + return $transaction; + } + ); + + return $result; + } + + /** + * @param Collection $collection + * @param array $accounts + * + * @return Collection + */ + public static function filterIncome(Collection $collection, array $accounts): Collection + { + $result = $collection->filter( + function (Transaction $transaction) use ($accounts) { + $opposing = $transaction->opposing_account_id; + // remove internal transfer + if (in_array($opposing, $accounts)) { + Log::debug(sprintf('Filtered #%d because its opposite is in accounts.', $transaction->id)); + + return null; + } + // remove positive amount + if (bccomp($transaction->transaction_amount, '0') === -1) { + Log::debug(sprintf('Filtered #%d because amount is %f.', $transaction->id, $transaction->transaction_amount)); + + return null; + } + + return $transaction; + } + ); + + return $result; + } + /** * @return string */ @@ -186,7 +248,7 @@ class MonthReportGenerator implements ReportGeneratorInterface */ private function getEarnedAccountSummary(): array { - $transactions = $this->getIncomes(); + $transactions = $this->getIncome(); $result = []; /** @var Transaction $transaction */ foreach ($transactions as $transaction) { @@ -203,7 +265,7 @@ class MonthReportGenerator implements ReportGeneratorInterface */ private function getEarnedCategorySummary(): array { - $transactions = $this->getIncomes(); + $transactions = $this->getIncome(); $result = []; /** @var Transaction $transaction */ foreach ($transactions as $transaction) { @@ -230,26 +292,8 @@ class MonthReportGenerator implements ReportGeneratorInterface $accountIds = $this->accounts->pluck('id')->toArray(); $transactions = $collector->getJournals(); + $transactions = self::filterExpenses($transactions, $accountIds); - $transactions = $transactions->filter( - function (Transaction $transaction) use ($accountIds) { - $opposing = $transaction->opposing_account_id; - // remove internal transfer - if (in_array($opposing, $accountIds)) { - Log::debug(sprintf('Filtered #%d because its opposite is in accounts.', $transaction->id)); - - return null; - } - // remove positive amount - if (bccomp($transaction->transaction_amount, '0') === 1) { - Log::debug(sprintf('Filtered #%d because amount is %f.', $transaction->id, $transaction->transaction_amount)); - - return null; - } - - return $transaction; - } - ); return $transactions; } @@ -257,7 +301,7 @@ class MonthReportGenerator implements ReportGeneratorInterface /** * @return Collection */ - private function getIncomes(): Collection + private function getIncome(): Collection { $collector = new JournalCollector(auth()->user()); $collector->setAccounts($this->accounts)->setRange($this->start, $this->end) @@ -265,25 +309,7 @@ class MonthReportGenerator implements ReportGeneratorInterface ->setCategories($this->categories)->getOpposingAccount(); $accountIds = $this->accounts->pluck('id')->toArray(); $transactions = $collector->getJournals(); - $transactions = $transactions->filter( - function (Transaction $transaction) use ($accountIds) { - $opposing = $transaction->opposing_account_id; - // remove internal transfer - if (in_array($opposing, $accountIds)) { - Log::debug(sprintf('Filtered #%d because its opposite is in accounts.', $transaction->id)); - - return null; - } - // remove positive amount - if (bccomp($transaction->transaction_amount, '0') === -1) { - Log::debug(sprintf('Filtered #%d because amount is %f.', $transaction->id, $transaction->transaction_amount)); - - return null; - } - - return $transaction; - } - ); + $transactions = self::filterIncome($transactions, $accountIds); return $transactions; } diff --git a/app/Http/Controllers/Chart/CategoryController.php b/app/Http/Controllers/Chart/CategoryController.php index d38be5f4db..45741349b0 100644 --- a/app/Http/Controllers/Chart/CategoryController.php +++ b/app/Http/Controllers/Chart/CategoryController.php @@ -16,10 +16,12 @@ namespace FireflyIII\Http\Controllers\Chart; use Carbon\Carbon; use FireflyIII\Generator\Chart\Category\CategoryChartGeneratorInterface; +use FireflyIII\Generator\Report\Category\MonthReportGenerator; use FireflyIII\Helpers\Collector\JournalCollector; use FireflyIII\Http\Controllers\Controller; use FireflyIII\Models\AccountType; use FireflyIII\Models\Category; +use FireflyIII\Models\Transaction; use FireflyIII\Models\TransactionType; use FireflyIII\Repositories\Account\AccountRepositoryInterface; use FireflyIII\Repositories\Category\CategoryRepositoryInterface; @@ -124,29 +126,50 @@ class CategoryController extends Controller { /** @var CategoryRepositoryInterface $repository */ $repository = app(CategoryRepositoryInterface::class); - $others = intval($others) === 1; - $names = []; - $collector = new JournalCollector(auth()->user()); - $collector->setAccounts($accounts)->setRange($start, $end) - ->setTypes([TransactionType::WITHDRAWAL]) - ->setCategories($categories); - $set = $collector->getSumPerCategory(); + /** @var bool $others */ + $others = intval($others) === 1; + $names = []; + + // collect journals (just like the category report does): + $collector = new JournalCollector(auth()->user()); + $collector->setAccounts($accounts)->setRange($start, $end)->setTypes([TransactionType::WITHDRAWAL, TransactionType::TRANSFER]) + ->setCategories($categories)->getOpposingAccount()->disableFilter(); + $accountIds = $accounts->pluck('id')->toArray(); + $transactions = $collector->getJournals(); + $set = MonthReportGenerator::filterExpenses($transactions, $accountIds); + + // group by category ID: + $grouped = []; + /** @var Transaction $transaction */ + foreach ($set as $transaction) { + $jrnlCatId = intval($transaction->transaction_journal_category_id); + $transCatId = intval($transaction->transaction_category_id); + $categoryId = max($jrnlCatId, $transCatId); + + $grouped[$categoryId] = $grouped[$categoryId] ?? '0'; + $amount = bcmul($transaction->transaction_amount, '-1'); + $grouped[$categoryId] = bcadd($amount, $grouped[$categoryId]); + } + + // loop and show the grouped results: $result = []; $total = '0'; - foreach ($set as $categoryId => $amount) { + foreach ($grouped as $categoryId => $amount) { if (!isset($names[$categoryId])) { $category = $repository->find(intval($categoryId)); $names[$categoryId] = $category->name; } - $amount = bcmul($amount, '-1'); $total = bcadd($total, $amount); $result[] = ['name' => $names[$categoryId], 'id' => $categoryId, 'amount' => $amount]; } + // also collect others? + // TODO include transfers if ($others) { $collector = new JournalCollector(auth()->user()); $collector->setAccounts($accounts)->setRange($start, $end)->setTypes([TransactionType::WITHDRAWAL]); - $sum = bcmul($collector->getSum(), '-1'); + $journals = $collector->getJournals(); + $sum = bcmul(strval($journals->sum('transaction_amount')), '-1'); $sum = bcsub($sum, $total); $result[] = ['name' => trans('firefly.everything_else'), 'id' => 0, 'amount' => $sum]; } @@ -214,14 +237,33 @@ class CategoryController extends Controller /** @var CategoryRepositoryInterface $repository */ $repository = app(CategoryRepositoryInterface::class); /** @var bool $others */ - $others = intval($others) === 1; - $names = []; + $others = intval($others) === 1; + $names = []; + + // collect journals (just like the category report does): $collector = new JournalCollector(auth()->user()); - $collector->setAccounts($accounts)->setRange($start, $end)->setTypes([TransactionType::DEPOSIT])->setCategories($categories); - $set = $collector->getSumPerCategory(); + $collector->setAccounts($accounts)->setRange($start, $end)->setTypes([TransactionType::DEPOSIT, TransactionType::TRANSFER]) + ->setCategories($categories)->getOpposingAccount(); + $accountIds = $accounts->pluck('id')->toArray(); + $transactions = $collector->getJournals(); + $set = MonthReportGenerator::filterIncome($transactions, $accountIds); + + // group by category ID: + $grouped = []; + /** @var Transaction $transaction */ + foreach ($set as $transaction) { + $jrnlCatId = intval($transaction->transaction_journal_category_id); + $transCatId = intval($transaction->transaction_category_id); + $categoryId = max($jrnlCatId, $transCatId); + + $grouped[$categoryId] = $grouped[$categoryId] ?? '0'; + $grouped[$categoryId] = bcadd($transaction->transaction_amount, $grouped[$categoryId]); + } + + // loop and show the grouped results: $result = []; $total = '0'; - foreach ($set as $categoryId => $amount) { + foreach ($grouped as $categoryId => $amount) { if (!isset($names[$categoryId])) { $category = $repository->find(intval($categoryId)); $names[$categoryId] = $category->name; @@ -230,10 +272,13 @@ class CategoryController extends Controller $result[] = ['name' => $names[$categoryId], 'id' => $categoryId, 'amount' => $amount]; } + // also collect others? + // TODO include transfers if ($others) { $collector = new JournalCollector(auth()->user()); $collector->setAccounts($accounts)->setRange($start, $end)->setTypes([TransactionType::DEPOSIT]); - $sum = $collector->getSum(); + $journals = $collector->getJournals(); + $sum = strval($journals->sum('transaction_amount')); $sum = bcsub($sum, $total); $result[] = ['name' => trans('firefly.everything_else'), 'id' => 0, 'amount' => $sum]; }