From 50b72cf229ad2f679a9b78aa22bb3cecc61457ee Mon Sep 17 00:00:00 2001 From: James Cole Date: Sat, 19 Nov 2016 13:37:44 +0100 Subject: [PATCH] More chart optimisations. --- .../Budget/BudgetChartGeneratorInterface.php | 5 +- .../Budget/ChartJsBudgetChartGenerator.php | 13 +- app/Helpers/Report/BudgetReportHelper.php | 127 +----------------- .../Report/BudgetReportHelperInterface.php | 8 -- .../Controllers/Chart/BudgetController.php | 72 ++++++---- .../Controllers/Report/BudgetController.php | 5 +- app/Repositories/Budget/BudgetRepository.php | 102 ++++++++++++++ .../Budget/BudgetRepositoryInterface.php | 26 ++++ app/Support/Navigation.php | 41 +++++- 9 files changed, 229 insertions(+), 170 deletions(-) diff --git a/app/Generator/Chart/Budget/BudgetChartGeneratorInterface.php b/app/Generator/Chart/Budget/BudgetChartGeneratorInterface.php index 4983d9635a..9d17fcb204 100644 --- a/app/Generator/Chart/Budget/BudgetChartGeneratorInterface.php +++ b/app/Generator/Chart/Budget/BudgetChartGeneratorInterface.php @@ -39,12 +39,11 @@ interface BudgetChartGeneratorInterface public function frontpage(Collection $entries): array; /** - * @param Collection $entries - * @param string $viewRange + * @param array $entries * * @return array */ - public function period(Collection $entries, string $viewRange) : array; + public function period(array $entries) : array; /** * @param Collection $budgets diff --git a/app/Generator/Chart/Budget/ChartJsBudgetChartGenerator.php b/app/Generator/Chart/Budget/ChartJsBudgetChartGenerator.php index cf78936c4d..6af797b8a3 100644 --- a/app/Generator/Chart/Budget/ChartJsBudgetChartGenerator.php +++ b/app/Generator/Chart/Budget/ChartJsBudgetChartGenerator.php @@ -101,15 +101,15 @@ class ChartJsBudgetChartGenerator implements BudgetChartGeneratorInterface } /** - * @param Collection $entries - * @param string $viewRange + * @param array $entries * * @return array */ - public function period(Collection $entries, string $viewRange) : array + public function period(array $entries) : array { + $data = [ - 'labels' => [], + 'labels' => array_keys($entries), 'datasets' => [ 0 => [ 'label' => trans('firefly.budgeted'), @@ -122,9 +122,8 @@ class ChartJsBudgetChartGenerator implements BudgetChartGeneratorInterface ], 'count' => 2, ]; - foreach ($entries as $entry) { - $label = Navigation::periodShow($entry['date'], $viewRange); - $data['labels'][] = $label; + + foreach ($entries as $label => $entry) { // data set 0 is budgeted // data set 1 is spent: $data['datasets'][0]['data'][] = $entry['budgeted']; diff --git a/app/Helpers/Report/BudgetReportHelper.php b/app/Helpers/Report/BudgetReportHelper.php index 9bfa5a46d9..675c4e2046 100644 --- a/app/Helpers/Report/BudgetReportHelper.php +++ b/app/Helpers/Report/BudgetReportHelper.php @@ -15,15 +15,14 @@ namespace FireflyIII\Helpers\Report; use Carbon\Carbon; -use DB; use FireflyIII\Helpers\Collection\Budget as BudgetCollection; use FireflyIII\Helpers\Collection\BudgetLine; use FireflyIII\Models\Budget; use FireflyIII\Models\LimitRepetition; use FireflyIII\Models\TransactionJournal; use FireflyIII\Repositories\Budget\BudgetRepositoryInterface; -use Illuminate\Database\Query\JoinClause; use Illuminate\Support\Collection; +use Navigation; use stdClass; /** @@ -47,8 +46,6 @@ class BudgetReportHelper implements BudgetReportHelperInterface } /** - * TODO the query called here must be moved to a repository. - * * @param Carbon $start * @param Carbon $end * @param Collection $accounts @@ -57,50 +54,10 @@ class BudgetReportHelper implements BudgetReportHelperInterface */ public function getBudgetPeriodReport(Carbon $start, Carbon $end, Collection $accounts): array { - // get account ID's. - $accountIds = $accounts->pluck('id')->toArray(); - - // define period to group on: - $sqlDateFormat = '%Y-%m-%d'; - // monthly report (for year) - if ($start->diffInMonths($end) > 1) { - $sqlDateFormat = '%Y-%m'; - } - - // yearly report (for multi year) - if ($start->diffInMonths($end) > 12) { - $sqlDateFormat = '%Y'; - } - - // build query. - $query = TransactionJournal - ::leftJoin('budget_transaction_journal', 'budget_transaction_journal.transaction_journal_id', '=', 'transaction_journals.id') - ->leftJoin('transaction_types', 'transaction_types.id', '=', 'transaction_journals.transaction_type_id') - ->leftJoin( - 'transactions', function (JoinClause $join) { - $join->on('transaction_journals.id', '=', 'transactions.transaction_journal_id')->where('transactions.amount', '<', 0); - } - ) - ->whereNull('transaction_journals.deleted_at') - ->whereNull('transactions.deleted_at') - ->where('transaction_types.type', 'Withdrawal') - ->where('transaction_journals.user_id', auth()->user()->id); - - if (count($accountIds) > 0) { - $query->whereIn('transactions.account_id', $accountIds); - } - $query->groupBy(['budget_transaction_journal.budget_id', 'period_marker']); - $queryResult = $query->get( - [ - 'budget_transaction_journal.budget_id', - DB::raw('DATE_FORMAT(transaction_journals.date,"' . $sqlDateFormat . '") AS period_marker'), - DB::raw('SUM(transactions.amount) as sum_of_period'), - ] - ); - - $data = []; - $budgets = $this->repository->getBudgets(); - $periods = $this->listOfPeriods($start, $end); + $budgets = $this->repository->getBudgets(); + $queryResult = $this->repository->getBudgetPeriodReport($budgets, $accounts, $start, $end); + $data = []; + $periods = Navigation::listOfPeriods($start, $end); // do budget "zero" $emptyBudget = new Budget; @@ -108,12 +65,11 @@ class BudgetReportHelper implements BudgetReportHelperInterface $emptyBudget->name = strval(trans('firefly.no_budget')); $budgets->push($emptyBudget); - // get all budgets and years. foreach ($budgets as $budget) { $data[$budget->id] = [ 'name' => $budget->name, - 'entries' => $this->filterAllAmounts($queryResult, $budget->id, $periods), + 'entries' => $this->repository->filterAmounts($queryResult, $budget->id, $periods), 'sum' => '0', ]; } @@ -208,46 +164,6 @@ class BudgetReportHelper implements BudgetReportHelperInterface return $set; } - /** - * @param Carbon $start - * @param Carbon $end - * - * @return array - */ - public function listOfPeriods(Carbon $start, Carbon $end): array - { - // define period to increment - $increment = 'addDay'; - $format = 'Y-m-d'; - $displayFormat = strval(trans('config.month_and_day')); - // increment by month (for year) - if ($start->diffInMonths($end) > 1) { - $increment = 'addMonth'; - $format = 'Y-m'; - $displayFormat = strval(trans('config.month')); - } - - // increment by year (for multi year) - if ($start->diffInMonths($end) > 12) { - $increment = 'addYear'; - $format = 'Y'; - $displayFormat = strval(trans('config.year')); - } - - $begin = clone $start; - $entries = []; - while ($begin < $end) { - $formatted = $begin->format($format); - $displayed = $begin->formatLocalized($displayFormat); - $entries[$formatted] = $displayed; - - $begin->$increment(); - } - - return $entries; - - } - /** * @param Budget $budget * @param LimitRepetition $repetition @@ -268,37 +184,6 @@ class BudgetReportHelper implements BudgetReportHelperInterface } - /** - * Filters entries from the result set generated by getBudgetPeriodReport - * - * @param Collection $set - * @param int $budgetId - * @param array $periods - * - * @return array - */ - private function filterAllAmounts(Collection $set, int $budgetId, array $periods):array - { - $arr = []; - $keys = array_keys($periods); - foreach ($keys as $period) { - /** @var stdClass $object */ - $result = $set->filter( - function (TransactionJournal $object) use ($budgetId, $period) { - return strval($object->period_marker) === $period && $budgetId === intval($object->budget_id); - } - ); - $amount = '0'; - if (!is_null($result->first())) { - $amount = $result->first()->sum_of_period; - } - - $arr[$period] = $amount; - } - - return $arr; - } - /** * Filters empty results from getBudgetPeriodReport * diff --git a/app/Helpers/Report/BudgetReportHelperInterface.php b/app/Helpers/Report/BudgetReportHelperInterface.php index ee1bb2a1b6..ad8540a22a 100644 --- a/app/Helpers/Report/BudgetReportHelperInterface.php +++ b/app/Helpers/Report/BudgetReportHelperInterface.php @@ -53,12 +53,4 @@ interface BudgetReportHelperInterface */ public function getBudgetsWithExpenses(Carbon $start, Carbon $end, Collection $accounts): Collection; - /** - * @param Carbon $start - * @param Carbon $end - * - * @return array - */ - public function listOfPeriods(Carbon $start, Carbon $end): array; - } diff --git a/app/Http/Controllers/Chart/BudgetController.php b/app/Http/Controllers/Chart/BudgetController.php index fbcc415869..e1cd94e3dc 100644 --- a/app/Http/Controllers/Chart/BudgetController.php +++ b/app/Http/Controllers/Chart/BudgetController.php @@ -24,7 +24,6 @@ use FireflyIII\Models\TransactionType; use FireflyIII\Repositories\Budget\BudgetRepositoryInterface; use FireflyIII\Support\CacheProperties; use Illuminate\Support\Collection; -use Log; use Navigation; use Preferences; use Response; @@ -147,7 +146,6 @@ class BudgetController extends Controller */ public function frontpage(BudgetRepositoryInterface $repository) { - Log::debug('Hello'); $start = session('start', Carbon::now()->startOfMonth()); $end = session('end', Carbon::now()->endOfMonth()); // chart properties for cache: @@ -189,6 +187,7 @@ class BudgetController extends Controller * * TODO use the NEW query that will be in the repository. Because that query will be shared between the budget period report (table for all budgets) * TODO and this chart (a single budget) + * * @param BudgetRepositoryInterface $repository * @param Budget $budget * @param Carbon $start @@ -199,9 +198,6 @@ class BudgetController extends Controller */ public function period(BudgetRepositoryInterface $repository, Budget $budget, Carbon $start, Carbon $end, Collection $accounts) { - - - // chart properties for cache: $cache = new CacheProperties(); $cache->addProperty($start); @@ -211,40 +207,60 @@ class BudgetController extends Controller $cache->addProperty('budget'); $cache->addProperty('period'); if ($cache->has()) { - return Response::json($cache->get()); + //return Response::json($cache->get()); } - // loop over period, add by users range: - $current = clone $start; - $viewRange = Preferences::get('viewRange', '1M')->data; - $set = new Collection; + + // the expenses: + $periods = Navigation::listOfPeriods($start, $end); + $result = $repository->getBudgetPeriodReport(new Collection([$budget]), $accounts, $start, $end); + $entries = $repository->filterAmounts($result, $budget->id, $periods); + $budgeted = []; + + // the budget limits: + $range = '1D'; + $key = 'Y-m-d'; + if ($start->diffInMonths($end) > 1) { + $range = '1M'; + $key = 'Y-m'; + } + + if ($start->diffInMonths($end) > 12) { + $range = '1Y'; + $key = 'Y'; + } + $repetitions = $repository->getAllBudgetLimitRepetitions($start, $end); - - + $current = clone $start; while ($current < $end) { - $currentStart = clone $current; - $currentEnd = Navigation::endOfPeriod($currentStart, $viewRange); - $reps = $repetitions->filter( - function (LimitRepetition $repetition) use ($budget, $currentStart) { - if ($repetition->budget_id === $budget->id && $repetition->startdate == $currentStart) { + $currentStart = Navigation::startOfPeriod($current, $range); + $currentEnd = Navigation::endOfPeriod($current, $range); + $reps = $repetitions->filter( + function (LimitRepetition $repetition) use ($budget, $currentStart, $currentEnd) { + if ($repetition->budget_id === $budget->id && $repetition->startdate >= $currentStart && $repetition->enddate <= $currentEnd) { return true; } return false; } ); - $budgeted = $reps->sum('amount'); - $spent = $repository->spentInPeriod(new Collection([$budget]), $accounts, $currentStart, $currentEnd); - $entry = [ - 'date' => clone $currentStart, - 'budgeted' => $budgeted, - 'spent' => $spent, - ]; - $set->push($entry); + $index = $currentStart->format($key); + $budgeted[$index] = $reps->sum('amount'); $currentEnd->addDay(); $current = clone $currentEnd; - } - $data = $this->generator->period($set, $viewRange); + + // join them: + $result = []; + foreach (array_keys($periods) as $period) { + $nice = $periods[$period]; + $result[$nice] = [ + 'spent' => isset($entries[$period]) ? $entries[$period] : '0', + 'budgeted' => isset($entries[$period]) ? $budgeted[$period] : 0, + ]; + } + + $data = $this->generator->period($result); + $cache->store($data); return Response::json($data); @@ -336,7 +352,7 @@ class BudgetController extends Controller { // collector $collector = new JournalCollector(auth()->user()); - $types = [TransactionType::WITHDRAWAL]; + $types = [TransactionType::WITHDRAWAL]; $collector->setAllAssetAccounts()->setTypes($types)->setRange($start, $end)->withoutBudget(); $journals = $collector->getJournals(); $sum = '0'; diff --git a/app/Http/Controllers/Report/BudgetController.php b/app/Http/Controllers/Report/BudgetController.php index aa1f844f14..09aa4a0475 100644 --- a/app/Http/Controllers/Report/BudgetController.php +++ b/app/Http/Controllers/Report/BudgetController.php @@ -19,6 +19,7 @@ use FireflyIII\Helpers\Report\BudgetReportHelperInterface; use FireflyIII\Http\Controllers\Controller; use FireflyIII\Support\CacheProperties; use Illuminate\Support\Collection; +use Navigation; /** * Class BudgetController @@ -45,10 +46,10 @@ class BudgetController extends Controller $cache->addProperty('budget-period-report'); $cache->addProperty($accounts->pluck('id')->toArray()); if ($cache->has()) { - //return $cache->get(); + return $cache->get(); } - $periods = $helper->listOfPeriods($start, $end); + $periods = Navigation::listOfPeriods($start, $end); $budgets = $helper->getBudgetPeriodReport($start, $end, $accounts); $result = view('reports.partials.budget-period', compact('budgets', 'periods'))->render(); $cache->store($result); diff --git a/app/Repositories/Budget/BudgetRepository.php b/app/Repositories/Budget/BudgetRepository.php index 6726287704..e1cc7fa0f9 100644 --- a/app/Repositories/Budget/BudgetRepository.php +++ b/app/Repositories/Budget/BudgetRepository.php @@ -14,6 +14,7 @@ declare(strict_types = 1); namespace FireflyIII\Repositories\Budget; use Carbon\Carbon; +use DB; use FireflyIII\Events\StoredBudgetLimit; use FireflyIII\Events\UpdatedBudgetLimit; use FireflyIII\Models\Budget; @@ -27,6 +28,7 @@ use Illuminate\Database\Eloquent\Builder; use Illuminate\Database\Query\JoinClause; use Illuminate\Support\Collection; use Log; +use stdClass; /** * Class BudgetRepository @@ -72,6 +74,39 @@ class BudgetRepository implements BudgetRepositoryInterface return true; } + /** + * Filters entries from the result set generated by getBudgetPeriodReport + * + * @param Collection $set + * @param int $budgetId + * @param array $periods + * + * @return array + */ + public function filterAmounts(Collection $set, int $budgetId, array $periods): array + { + $arr = []; + $keys = array_keys($periods); + foreach ($keys as $period) { + /** @var stdClass $object */ + $result = $set->filter( + function (TransactionJournal $object) use ($budgetId, $period) { + $result = strval($object->period_marker) === strval($period) && $budgetId === intval($object->budget_id); + + return $result; + } + ); + $amount = '0'; + if (!is_null($result->first())) { + $amount = $result->first()->sum_of_period; + } + + $arr[$period] = $amount; + } + + return $arr; + } + /** * Find a budget. * @@ -175,6 +210,73 @@ class BudgetRepository implements BudgetRepositoryInterface return $set; } + /** + * This method is being used to generate the budget overview in the year/multi-year report. More specifically, this + * method runs the query and returns the result that is used for this report. + * + * The query is used in both the year/multi-year budget overview AND in the accompanying chart. + * + * @param Collection $budgets + * @param Collection $accounts + * @param Carbon $start + * @param Carbon $end + * + * @return Collection + */ + public function getBudgetPeriodReport(Collection $budgets, Collection $accounts, Carbon $start, Carbon $end): Collection + { + // get account ID's. + $accountIds = $accounts->pluck('id')->toArray(); + + // get budget ID's. + $budgetIds = $budgets->pluck('id')->toArray(); + + // define period to group on: + $sqlDateFormat = '%Y-%m-%d'; + // monthly report (for year) + if ($start->diffInMonths($end) > 1) { + $sqlDateFormat = '%Y-%m'; + } + + // yearly report (for multi year) + if ($start->diffInMonths($end) > 12) { + $sqlDateFormat = '%Y'; + } + + // build query. + $query = TransactionJournal + ::leftJoin('budget_transaction_journal', 'budget_transaction_journal.transaction_journal_id', '=', 'transaction_journals.id') + ->leftJoin('transaction_types', 'transaction_types.id', '=', 'transaction_journals.transaction_type_id') + ->leftJoin( + 'transactions', function (JoinClause $join) { + $join->on('transaction_journals.id', '=', 'transactions.transaction_journal_id')->where('transactions.amount', '<', 0); + } + ) + ->whereNull('transaction_journals.deleted_at') + ->whereNull('transactions.deleted_at') + ->where('transaction_types.type', 'Withdrawal') + ->where('transaction_journals.user_id', auth()->user()->id); + + if (count($accountIds) > 0) { + $query->whereIn('transactions.account_id', $accountIds); + } + + if (count($budgetIds) > 0) { + $query->whereIn('budget_transaction_journal.budget_id', $budgetIds); + } + + $query->groupBy(['budget_transaction_journal.budget_id', 'period_marker']); + + return $query->get( + [ + 'budget_transaction_journal.budget_id', + DB::raw(sprintf('DATE_FORMAT(transaction_journals.date,"%s") AS period_marker', $sqlDateFormat)), + DB::raw('SUM(transactions.amount) as sum_of_period'), + ] + ); + + } + /** * @return Collection */ diff --git a/app/Repositories/Budget/BudgetRepositoryInterface.php b/app/Repositories/Budget/BudgetRepositoryInterface.php index b7116dcbd2..fcc74eac31 100644 --- a/app/Repositories/Budget/BudgetRepositoryInterface.php +++ b/app/Repositories/Budget/BudgetRepositoryInterface.php @@ -38,6 +38,17 @@ interface BudgetRepositoryInterface */ public function destroy(Budget $budget): bool; + /** + * Filters entries from the result set generated by getBudgetPeriodReport + * + * @param Collection $set + * @param int $budgetId + * @param array $periods + * + * @return array + */ + public function filterAmounts(Collection $set, int $budgetId, array $periods): array; + /** * Find a budget. * @@ -79,6 +90,21 @@ interface BudgetRepositoryInterface */ public function getAllBudgetLimitRepetitions(Carbon $start, Carbon $end): Collection; + /** + * This method is being used to generate the budget overview in the year/multi-year report. More specifically, this + * method runs the query and returns the result that is used for this report. + * + * The query is used in both the year/multi-year budget overview AND in the accompanying chart. + * + * @param Collection $budgets + * @param Collection $accounts + * @param Carbon $start + * @param Carbon $end + * + * @return Collection + */ + public function getBudgetPeriodReport(Collection $budgets, Collection $accounts, Carbon $start, Carbon $end): Collection; + /** * @return Collection */ diff --git a/app/Support/Navigation.php b/app/Support/Navigation.php index 665873596e..449ac4e6ae 100644 --- a/app/Support/Navigation.php +++ b/app/Support/Navigation.php @@ -24,7 +24,6 @@ use FireflyIII\Exceptions\FireflyException; class Navigation { - /** * @param \Carbon\Carbon $theDate * @param $repeatFreq @@ -170,6 +169,46 @@ class Navigation return $currentEnd; } + /** + * @param \Carbon\Carbon $start + * @param \Carbon\Carbon $end + * + * @return array + */ + public function listOfPeriods(Carbon $start, Carbon $end): array + { + // define period to increment + $increment = 'addDay'; + $format = 'Y-m-d'; + $displayFormat = strval(trans('config.month_and_day')); + // increment by month (for year) + if ($start->diffInMonths($end) > 1) { + $increment = 'addMonth'; + $format = 'Y-m'; + $displayFormat = strval(trans('config.month')); + } + + // increment by year (for multi year) + if ($start->diffInMonths($end) > 12) { + $increment = 'addYear'; + $format = 'Y'; + $displayFormat = strval(trans('config.year')); + } + + $begin = clone $start; + $entries = []; + while ($begin < $end) { + $formatted = $begin->format($format); + $displayed = $begin->formatLocalized($displayFormat); + $entries[$formatted] = $displayed; + + $begin->$increment(); + } + + return $entries; + + } + /** * @param \Carbon\Carbon $date * @param $repeatFrequency