From c34fb7f03772db58fa8e8e17f909e1a88070ad0f Mon Sep 17 00:00:00 2001 From: James Cole Date: Tue, 3 Jan 2017 17:02:17 +0100 Subject: [PATCH] Cleaned up the category and budget pie charts. --- app/Generator/Report/Support.php | 38 +-- app/Helpers/Chart/MetaPieChart.php | 278 ++++++++++++++++++ app/Helpers/Chart/MetaPieChartInterface.php | 82 ++++++ .../Chart/BudgetReportController.php | 108 ++----- .../Chart/CategoryReportController.php | 208 +++---------- app/Providers/FireflyServiceProvider.php | 3 + resources/views/reports/budget/month.twig | 4 +- resources/views/reports/category/month.twig | 8 +- 8 files changed, 447 insertions(+), 282 deletions(-) create mode 100644 app/Helpers/Chart/MetaPieChart.php create mode 100644 app/Helpers/Chart/MetaPieChartInterface.php diff --git a/app/Generator/Report/Support.php b/app/Generator/Report/Support.php index 1f71ad182a..573129ef97 100644 --- a/app/Generator/Report/Support.php +++ b/app/Generator/Report/Support.php @@ -34,27 +34,7 @@ class Support */ 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; + return self::filterTransactions($collection, $accounts, 1); } /** @@ -64,9 +44,21 @@ class Support * @return Collection */ public static function filterIncome(Collection $collection, array $accounts): Collection + { + return self::filterTransactions($collection, $accounts, -1); + } + + /** + * @param Collection $collection + * @param array $accounts + * @param int $modifier + * + * @return Collection + */ + public static function filterTransactions(Collection $collection, array $accounts, int $modifier): Collection { $result = $collection->filter( - function (Transaction $transaction) use ($accounts) { + function (Transaction $transaction) use ($accounts, $modifier) { $opposing = $transaction->opposing_account_id; // remove internal transfer if (in_array($opposing, $accounts)) { @@ -75,7 +67,7 @@ class Support return null; } // remove positive amount - if (bccomp($transaction->transaction_amount, '0') === -1) { + if (bccomp($transaction->transaction_amount, '0') === $modifier) { Log::debug(sprintf('Filtered #%d because amount is %f.', $transaction->id, $transaction->transaction_amount)); return null; diff --git a/app/Helpers/Chart/MetaPieChart.php b/app/Helpers/Chart/MetaPieChart.php new file mode 100644 index 0000000000..fbeb70cc25 --- /dev/null +++ b/app/Helpers/Chart/MetaPieChart.php @@ -0,0 +1,278 @@ + ['opposing_account_id'], + 'budget' => ['transaction_journal_budget_id', 'transaction_budget_id'], + 'category' => ['transaction_journal_category_id', 'transaction_category_id'], + ]; + + /** @var array */ + protected $repositories + = [ + 'account' => AccountRepositoryInterface::class, + 'budget' => BudgetRepositoryInterface::class, + 'category' => CategoryRepositoryInterface::class, + ]; + + + /** @var Carbon */ + protected $start; + /** @var string */ + protected $total = '0'; + /** @var User */ + protected $user; + + public function __construct() + { + $this->accounts = new Collection; + $this->budgets = new Collection; + $this->categories = new Collection; + } + + /** + * @param string $direction + * @param string $group + * + * @return array + */ + public function generate(string $direction, string $group): array + { + $transactions = $this->getTransactions($direction); + $grouped = $this->groupByFields($transactions, $this->grouping[$group]); + $chartData = $this->organizeByType($group, $grouped); + + // also collect all other transactions + if ($this->collectOtherObjects && $direction === 'expense') { + /** @var JournalCollectorInterface $collector */ + $collector = app(JournalCollectorInterface::class, [$this->user]); + $collector->setAccounts($this->accounts)->setRange($this->start, $this->end)->setTypes([TransactionType::WITHDRAWAL]); + $journals = $collector->getJournals(); + $sum = strval($journals->sum('transaction_amount')); + $sum = bcmul($sum, '-1'); + $sum = bcsub($sum, $this->total); + $chartData[strval(trans('firefly.everything_else'))] = $sum; + } + + if ($this->collectOtherObjects && $direction === 'income') { + /** @var JournalCollectorInterface $collector */ + $collector = app(JournalCollectorInterface::class, [auth()->user()]); + $collector->setAccounts($this->accounts)->setRange($this->start, $this->end)->setTypes([TransactionType::DEPOSIT]); + $journals = $collector->getJournals(); + $sum = strval($journals->sum('transaction_amount')); + $sum = bcsub($sum, $this->total); + $chartData[strval(trans('firefly.everything_else'))] = $sum; + } + + return $chartData; + + } + + /** + * @param Collection $accounts + * + * @return MetaPieChartInterface + */ + public function setAccounts(Collection $accounts): MetaPieChartInterface + { + $this->accounts = $accounts; + + return $this; + } + + /** + * @param Collection $budgets + * + * @return MetaPieChartInterface + */ + public function setBudgets(Collection $budgets): MetaPieChartInterface + { + $this->budgets = $budgets; + + return $this; + } + + /** + * @param Collection $categories + * + * @return MetaPieChartInterface + */ + public function setCategories(Collection $categories): MetaPieChartInterface + { + $this->categories = $categories; + + return $this; + } + + /** + * @param bool $collectOtherObjects + * + * @return MetaPieChartInterface + */ + public function setCollectOtherObjects(bool $collectOtherObjects): MetaPieChartInterface + { + $this->collectOtherObjects = $collectOtherObjects; + + return $this; + } + + /** + * @param Carbon $end + * + * @return MetaPieChartInterface + */ + public function setEnd(Carbon $end): MetaPieChartInterface + { + $this->end = $end; + + return $this; + } + + /** + * @param Carbon $start + * + * @return MetaPieChartInterface + */ + public function setStart(Carbon $start): MetaPieChartInterface + { + $this->start = $start; + + return $this; + } + + /** + * @param User $user + * + * @return MetaPieChartInterface + */ + public function setUser(User $user): MetaPieChartInterface + { + $this->user = $user; + + return $this; + } + + protected function getTransactions(string $direction) + { + $types = [TransactionType::DEPOSIT, TransactionType::TRANSFER]; + $modifier = -1; + if ($direction === 'expense') { + $types = [TransactionType::WITHDRAWAL, TransactionType::TRANSFER]; + $modifier = 1; + } + /** @var JournalCollectorInterface $collector */ + $collector = app(JournalCollectorInterface::class, [auth()->user()]); + $collector->setAccounts($this->accounts); + $collector->setRange($this->start, $this->end); + $collector->setTypes($types); + $collector->withOpposingAccount(); + + if ($direction === 'income') { + $collector->disableFilter(); + } + + if ($this->budgets->count() > 0) { + $collector->setBudgets($this->budgets); + } + if ($this->categories->count() > 0) { + $collector->setCategories($this->categories); + } + + $accountIds = $this->accounts->pluck('id')->toArray(); + $transactions = $collector->getJournals(); + $set = Support::filterTransactions($transactions, $accountIds, $modifier); + + return $set; + } + + /** + * @param Collection $set + * @param array $fields + * + * @return array + */ + protected function groupByFields(Collection $set, array $fields) + { + $grouped = []; + /** @var Transaction $transaction */ + foreach ($set as $transaction) { + $values = []; + foreach ($fields as $field) { + $values[] = intval($transaction->$field); + } + $value = max($values); + $grouped[$value] = $grouped[$value] ?? '0'; + $grouped[$value] = bcadd($transaction->transaction_amount, $grouped[$value]); + } + + return $grouped; + } + + /** + * @param string $type + * @param array $array + * + * @return array + */ + protected function organizeByType(string $type, array $array): array + { + $chartData = []; + $names = []; + $repository = app($this->repositories[$type], [$this->user]); + foreach ($array as $objectId => $amount) { + if (!isset($names[$objectId])) { + $object = $repository->find(intval($objectId)); + $names[$objectId] = $object->name; + } + if (bccomp($amount, '0') === -1) { + $amount = bcmul($amount, '-1'); + } + + $this->total = bcadd($this->total, $amount); + $chartData[$names[$objectId]] = $amount; + } + + return $chartData; + + } +} \ No newline at end of file diff --git a/app/Helpers/Chart/MetaPieChartInterface.php b/app/Helpers/Chart/MetaPieChartInterface.php new file mode 100644 index 0000000000..a466db8a38 --- /dev/null +++ b/app/Helpers/Chart/MetaPieChartInterface.php @@ -0,0 +1,82 @@ +addProperty('chart.budget.report.account-expense'); - $cache->addProperty($accounts); - $cache->addProperty($budgets); - $cache->addProperty($start); - $cache->addProperty($end); - $cache->addProperty($others); - if ($cache->has()) { - return Response::json($cache->get()); - } - - $names = []; - $set = $this->getExpenses($accounts, $budgets, $start, $end); - $grouped = $this->groupByOpposingAccount($set); - $chartData = []; - $total = '0'; - - foreach ($grouped as $accountId => $amount) { - if (!isset($names[$accountId])) { - $account = $this->accountRepository->find(intval($accountId)); - $names[$accountId] = $account->name; - } - $amount = bcmul($amount, '-1'); - $total = bcadd($total, $amount); - $chartData[$names[$accountId]] = $amount; - } - - // also collect all transactions NOT in these budgets. - if ($others) { - /** @var JournalCollectorInterface $collector */ - $collector = app(JournalCollectorInterface::class, [auth()->user()]); - $collector->setAccounts($accounts)->setRange($start, $end)->setTypes([TransactionType::WITHDRAWAL]); - $journals = $collector->getJournals(); - $sum = strval($journals->sum('transaction_amount')); - $sum = bcmul($sum, '-1'); - $sum = bcsub($sum, $total); - $chartData[strval(trans('firefly.everything_else'))] = $sum; - } - - $data = $this->generator->pieChart($chartData); - $cache->store($data); + /** @var MetaPieChartInterface $helper */ + $helper = app(MetaPieChartInterface::class); + $helper->setAccounts($accounts); + $helper->setBudgets($budgets); + $helper->setUser(auth()->user()); + $helper->setStart($start); + $helper->setEnd($end); + $helper->setCollectOtherObjects(intval($others) === 1); + $chartData = $helper->generate('expense', 'account'); + $data = $this->generator->pieChart($chartData); return Response::json($data); + } /** @@ -133,49 +102,16 @@ class BudgetReportController extends Controller */ public function budgetExpense(Collection $accounts, Collection $budgets, Carbon $start, Carbon $end, string $others) { - /** @var bool $others */ - $others = intval($others) === 1; - $cache = new CacheProperties; - $cache->addProperty('chart.budget.report.budget-expense'); - $cache->addProperty($accounts); - $cache->addProperty($budgets); - $cache->addProperty($start); - $cache->addProperty($end); - $cache->addProperty($others); - if ($cache->has()) { - return Response::json($cache->get()); - } - - $names = []; - $set = $this->getExpenses($accounts, $budgets, $start, $end); - $grouped = $this->groupByBudget($set); - $total = '0'; - $chartData = []; - - foreach ($grouped as $budgetId => $amount) { - if (!isset($names[$budgetId])) { - $budget = $this->budgetRepository->find(intval($budgetId)); - $names[$budgetId] = $budget->name; - } - $amount = bcmul($amount, '-1'); - $total = bcadd($total, $amount); - $chartData[$names[$budgetId]] = $amount; - } - - // also collect all transactions NOT in these budgets. - if ($others) { - /** @var JournalCollectorInterface $collector */ - $collector = app(JournalCollectorInterface::class, [auth()->user()]); - $collector->setAccounts($accounts)->setRange($start, $end)->setTypes([TransactionType::WITHDRAWAL]); - $journals = $collector->getJournals(); - $sum = strval($journals->sum('transaction_amount')); - $sum = bcmul($sum, '-1'); - $sum = bcsub($sum, $total); - $chartData[strval(trans('firefly.everything_else'))] = $sum; - } - - $data = $this->generator->pieChart($chartData); - $cache->store($data); + /** @var MetaPieChartInterface $helper */ + $helper = app(MetaPieChartInterface::class); + $helper->setAccounts($accounts); + $helper->setBudgets($budgets); + $helper->setUser(auth()->user()); + $helper->setStart($start); + $helper->setEnd($end); + $helper->setCollectOtherObjects(intval($others) === 1); + $chartData = $helper->generate('expense', 'budget'); + $data = $this->generator->pieChart($chartData); return Response::json($data); } diff --git a/app/Http/Controllers/Chart/CategoryReportController.php b/app/Http/Controllers/Chart/CategoryReportController.php index fb66e25a1d..e156becd7e 100644 --- a/app/Http/Controllers/Chart/CategoryReportController.php +++ b/app/Http/Controllers/Chart/CategoryReportController.php @@ -17,6 +17,7 @@ namespace FireflyIII\Http\Controllers\Chart; use Carbon\Carbon; use FireflyIII\Generator\Chart\Basic\GeneratorInterface; use FireflyIII\Generator\Report\Category\MonthReportGenerator; +use FireflyIII\Helpers\Chart\MetaPieChartInterface; use FireflyIII\Helpers\Collector\JournalCollectorInterface; use FireflyIII\Http\Controllers\Controller; use FireflyIII\Models\Category; @@ -75,49 +76,16 @@ class CategoryReportController extends Controller */ public function accountExpense(Collection $accounts, Collection $categories, Carbon $start, Carbon $end, string $others) { - /** @var bool $others */ - $others = intval($others) === 1; - $cache = new CacheProperties; - $cache->addProperty('chart.category.report.account-expense'); - $cache->addProperty($accounts); - $cache->addProperty($categories); - $cache->addProperty($start); - $cache->addProperty($end); - $cache->addProperty($others); - if ($cache->has()) { - return Response::json($cache->get()); - } - - $names = []; - $set = $this->getExpenses($accounts, $categories, $start, $end); - $grouped = $this->groupByOpposingAccount($set); - $chartData = []; - $total = '0'; - - foreach ($grouped as $accountId => $amount) { - if (!isset($names[$accountId])) { - $account = $this->accountRepository->find(intval($accountId)); - $names[$accountId] = $account->name; - } - $amount = bcmul($amount, '-1'); - $total = bcadd($total, $amount); - $chartData[$names[$accountId]] = $amount; - } - - // also collect all transactions NOT in these categories. - if ($others) { - /** @var JournalCollectorInterface $collector */ - $collector = app(JournalCollectorInterface::class, [auth()->user()]); - $collector->setAccounts($accounts)->setRange($start, $end)->setTypes([TransactionType::WITHDRAWAL]); - $journals = $collector->getJournals(); - $sum = strval($journals->sum('transaction_amount')); - $sum = bcmul($sum, '-1'); - $sum = bcsub($sum, $total); - $chartData[strval(trans('firefly.everything_else'))] = $sum; - } - - $data = $this->generator->pieChart($chartData); - $cache->store($data); + /** @var MetaPieChartInterface $helper */ + $helper = app(MetaPieChartInterface::class); + $helper->setAccounts($accounts); + $helper->setCategories($categories); + $helper->setUser(auth()->user()); + $helper->setStart($start); + $helper->setEnd($end); + $helper->setCollectOtherObjects(intval($others) === 1); + $chartData = $helper->generate('expense', 'account'); + $data = $this->generator->pieChart($chartData); return Response::json($data); } @@ -133,48 +101,16 @@ class CategoryReportController extends Controller */ public function accountIncome(Collection $accounts, Collection $categories, Carbon $start, Carbon $end, string $others) { - /** @var bool $others */ - $others = intval($others) === 1; - $cache = new CacheProperties; - $cache->addProperty('chart.category.report.account-income'); - $cache->addProperty($accounts); - $cache->addProperty($categories); - $cache->addProperty($start); - $cache->addProperty($others); - $cache->addProperty($end); - if ($cache->has()) { - return Response::json($cache->get()); - } - - - $names = []; - $set = $this->getIncome($accounts, $categories, $start, $end); - $grouped = $this->groupByOpposingAccount($set); - $chartData = []; - $total = '0'; - - foreach ($grouped as $accountId => $amount) { - if (!isset($names[$accountId])) { - $account = $this->accountRepository->find(intval($accountId)); - $names[$accountId] = $account->name; - } - $total = bcadd($total, $amount); - $chartData[$names[$accountId]] = $amount; - } - - // also collect others? - if ($others) { - /** @var JournalCollectorInterface $collector */ - $collector = app(JournalCollectorInterface::class, [auth()->user()]); - $collector->setAccounts($accounts)->setRange($start, $end)->setTypes([TransactionType::DEPOSIT]); - $journals = $collector->getJournals(); - $sum = strval($journals->sum('transaction_amount')); - $sum = bcsub($sum, $total); - $chartData[strval(trans('firefly.everything_else'))] = $sum; - } - - $data = $this->generator->pieChart($chartData); - $cache->store($data); + /** @var MetaPieChartInterface $helper */ + $helper = app(MetaPieChartInterface::class); + $helper->setAccounts($accounts); + $helper->setCategories($categories); + $helper->setUser(auth()->user()); + $helper->setStart($start); + $helper->setEnd($end); + $helper->setCollectOtherObjects(intval($others) === 1); + $chartData = $helper->generate('income', 'account'); + $data = $this->generator->pieChart($chartData); return Response::json($data); } @@ -190,49 +126,16 @@ class CategoryReportController extends Controller */ public function categoryExpense(Collection $accounts, Collection $categories, Carbon $start, Carbon $end, string $others) { - /** @var bool $others */ - $others = intval($others) === 1; - $cache = new CacheProperties; - $cache->addProperty('chart.category.report.category-expense'); - $cache->addProperty($accounts); - $cache->addProperty($categories); - $cache->addProperty($start); - $cache->addProperty($end); - $cache->addProperty($others); - if ($cache->has()) { - return Response::json($cache->get()); - } - - $names = []; - $set = $this->getExpenses($accounts, $categories, $start, $end); - $grouped = $this->groupByCategory($set); - $total = '0'; - $chartData = []; - - foreach ($grouped as $categoryId => $amount) { - if (!isset($names[$categoryId])) { - $category = $this->categoryRepository->find(intval($categoryId)); - $names[$categoryId] = $category->name; - } - $amount = bcmul($amount, '-1'); - $total = bcadd($total, $amount); - $chartData[$names[$categoryId]] = $amount; - } - - // also collect all transactions NOT in these categories. - if ($others) { - /** @var JournalCollectorInterface $collector */ - $collector = app(JournalCollectorInterface::class, [auth()->user()]); - $collector->setAccounts($accounts)->setRange($start, $end)->setTypes([TransactionType::WITHDRAWAL]); - $journals = $collector->getJournals(); - $sum = strval($journals->sum('transaction_amount')); - $sum = bcmul($sum, '-1'); - $sum = bcsub($sum, $total); - $chartData[strval(trans('firefly.everything_else'))] = $sum; - } - - $data = $this->generator->pieChart($chartData); - $cache->store($data); + /** @var MetaPieChartInterface $helper */ + $helper = app(MetaPieChartInterface::class); + $helper->setAccounts($accounts); + $helper->setCategories($categories); + $helper->setUser(auth()->user()); + $helper->setStart($start); + $helper->setEnd($end); + $helper->setCollectOtherObjects(intval($others) === 1); + $chartData = $helper->generate('expense', 'category'); + $data = $this->generator->pieChart($chartData); return Response::json($data); } @@ -248,46 +151,17 @@ class CategoryReportController extends Controller */ public function categoryIncome(Collection $accounts, Collection $categories, Carbon $start, Carbon $end, string $others) { - /** @var bool $others */ - $others = intval($others) === 1; - $cache = new CacheProperties; - $cache->addProperty('chart.category.report.category-income'); - $cache->addProperty($accounts); - $cache->addProperty($categories); - $cache->addProperty($start); - $cache->addProperty($end); - $cache->addProperty($others); - if ($cache->has()) { - return Response::json($cache->get()); - } - $names = []; - $set = $this->getIncome($accounts, $categories, $start, $end); - $grouped = $this->groupByCategory($set); - $total = '0'; - $chartData = []; - - foreach ($grouped as $categoryId => $amount) { - if (!isset($names[$categoryId])) { - $category = $this->categoryRepository->find(intval($categoryId)); - $names[$categoryId] = $category->name; - } - $total = bcadd($total, $amount); - $chartData[$names[$categoryId]] = $amount; - } - - if ($others) { - /** @var JournalCollectorInterface $collector */ - $collector = app(JournalCollectorInterface::class, [auth()->user()]); - $collector->setAccounts($accounts)->setRange($start, $end)->setTypes([TransactionType::DEPOSIT]); - $journals = $collector->getJournals(); - $sum = strval($journals->sum('transaction_amount')); - $sum = bcsub($sum, $total); - $chartData[strval(trans('firefly.everything_else'))] = $sum; - } - - $data = $this->generator->pieChart($chartData); - $cache->store($data); + /** @var MetaPieChartInterface $helper */ + $helper = app(MetaPieChartInterface::class); + $helper->setAccounts($accounts); + $helper->setCategories($categories); + $helper->setUser(auth()->user()); + $helper->setStart($start); + $helper->setEnd($end); + $helper->setCollectOtherObjects(intval($others) === 1); + $chartData = $helper->generate('income', 'category'); + $data = $this->generator->pieChart($chartData); return Response::json($data); } diff --git a/app/Providers/FireflyServiceProvider.php b/app/Providers/FireflyServiceProvider.php index 2a49d5b15e..b798f531e0 100644 --- a/app/Providers/FireflyServiceProvider.php +++ b/app/Providers/FireflyServiceProvider.php @@ -96,6 +96,9 @@ class FireflyServiceProvider extends ServiceProvider // chart generator: $this->app->bind('FireflyIII\Generator\Chart\Basic\GeneratorInterface', 'FireflyIII\Generator\Chart\Basic\ChartJsGenerator'); + // chart builder + $this->app->bind('FireflyIII\Helpers\Chart\MetaPieChartInterface', 'FireflyIII\Helpers\Chart\MetaPieChart'); + // other generators $this->app->bind('FireflyIII\Export\ProcessorInterface', 'FireflyIII\Export\Processor'); $this->app->bind('FireflyIII\Import\ImportProcedureInterface', 'FireflyIII\Import\ImportProcedure'); diff --git a/resources/views/reports/budget/month.twig b/resources/views/reports/budget/month.twig index e8de3bee8f..d5efaae2f7 100644 --- a/resources/views/reports/budget/month.twig +++ b/resources/views/reports/budget/month.twig @@ -80,7 +80,7 @@ @@ -97,7 +97,7 @@ diff --git a/resources/views/reports/category/month.twig b/resources/views/reports/category/month.twig index cef7514534..b50d8b4aed 100644 --- a/resources/views/reports/category/month.twig +++ b/resources/views/reports/category/month.twig @@ -90,7 +90,7 @@ @@ -104,7 +104,7 @@ @@ -119,7 +119,7 @@ @@ -133,7 +133,7 @@