Simplify budget report code.

app/Http/Controllers/Report/BudgetController.php

@@ -72,6 +72,7 @@ class BudgetController extends Controller
     }
 
     /**
+     * Partial used in the budget report.
      * @param Collection $accounts
      * @param Collection $budgets
      * @param Carbon     $start
@@ -81,46 +82,17 @@ class BudgetController extends Controller
      */
     public function accountPerBudget(Collection $accounts, Collection $budgets, Carbon $start, Carbon $end)
     {
-        $spent  = $this->opsRepository->listExpenses($start, $end, $accounts, $budgets);
-        $report = [];
-        /** @var Account $account */
-        foreach ($accounts as $account) {
-            $accountId          = $account->id;
-            $report[$accountId] = $report[$accountId] ?? [
-                    'name'       => $account->name,
-                    'id'         => $account->id,
-                    'iban'       => $account->iban,
-                    'currencies' => [],
-                ];
-        }
+        /** @var BudgetReportGenerator $generator */
+        $generator = app(BudgetReportGenerator::class);
 
-        // loop expenses.
-        foreach ($spent as $currency) {
-            $currencyId = $currency['currency_id'];
-
-            foreach ($currency['budgets'] as $budget) {
-                foreach ($budget['transaction_journals'] as $journal) {
-                    $sourceAccountId = $journal['source_account_id'];
-
-
-                    $report[$sourceAccountId]['currencies'][$currencyId]                           = $report[$sourceAccountId]['currencies'][$currencyId] ?? [
-                            'currency_id'             => $currency['currency_id'],
-                            'currency_symbol'         => $currency['currency_symbol'],
-                            'currency_name'           => $currency['currency_name'],
-                            'currency_decimal_places' => $currency['currency_decimal_places'],
-                            'budgets'                 => [],
-                        ];
-                    $report[$sourceAccountId]['currencies'][$currencyId]['budgets'][$budget['id']]
-                                                                                                   = $report[$sourceAccountId]['currencies'][$currencyId]['budgets'][$budget['id']]
-                                                                                                     ?? '0';
-                    $report[$sourceAccountId]['currencies'][$currencyId]['budgets'][$budget['id']] = bcadd(
-                        $report[$sourceAccountId]['currencies'][$currencyId]['budgets'][$budget['id']],
-                        $journal['amount']
-                    );
-                }
-            }
-        }
+        $generator->setUser(auth()->user());
+        $generator->setAccounts($accounts);
+        $generator->setBudgets($budgets);
+        $generator->setStart($start);
+        $generator->setEnd($end);
 
+        $generator->accountPerBudget();
+        $report = $generator->getReport();
         return view('reports.budget.partials.account-per-budget', compact('report', 'budgets'));
     }
 
@@ -302,7 +274,6 @@ class BudgetController extends Controller
 
     /**
      * Show partial overview of budgets.
-     * TODO can be replaced I think.
      *
      * @param Collection $accounts
      * @param Carbon     $start

app/Support/Report/Budget/BudgetReportGenerator.php

@@ -23,6 +23,7 @@ declare(strict_types=1);
 namespace FireflyIII\Support\Report\Budget;
 
 use Carbon\Carbon;
+use FireflyIII\Models\Account;
 use FireflyIII\Models\Budget;
 use FireflyIII\Models\BudgetLimit;
 use FireflyIII\Models\TransactionCurrency;
@@ -42,6 +43,7 @@ class BudgetReportGenerator
 {
     private User                           $user;
     private Collection                     $accounts;
+    private Collection                     $budgets;
     private Carbon                         $start;
     private Carbon                         $end;
     private BudgetRepositoryInterface      $repository;
@@ -79,6 +81,32 @@ class BudgetReportGenerator
         $this->percentageReport();
     }
 
+    /**
+     * Returns the data necessary for the "account per budget" block on the budget report.
+     */
+    public function accountPerBudget(): void
+    {
+
+        $spent        = $this->opsRepository->listExpenses($this->start, $this->end, $this->accounts, $this->budgets);
+        $this->report = [];
+        /** @var Account $account */
+        foreach ($this->accounts as $account) {
+            $accountId                = $account->id;
+            $this->report[$accountId] = $this->report[$accountId] ?? [
+                    'name'       => $account->name,
+                    'id'         => $account->id,
+                    'iban'       => $account->iban,
+                    'currencies' => [],
+                ];
+        }
+
+        // loop expenses.
+        foreach ($spent as $currency) {
+            $this->processExpenses($currency);
+        }
+
+    }
+
     /**
      * @param User $user
      */
@@ -92,6 +120,14 @@ class BudgetReportGenerator
         $this->currency = app('amount')->getDefaultCurrencyByUser($this->user);
     }
 
+    /**
+     * @param Collection $budgets
+     */
+    public function setBudgets(Collection $budgets): void
+    {
+        $this->budgets = $budgets;
+    }
+
     /**
      * @param Collection $accounts
      */
@@ -125,7 +161,7 @@ class BudgetReportGenerator
     }
 
     /**
-     * Start the report by processing every budget.
+     * Start the budgets block on the default report by processing every budget.
      */
     private function generalBudgetReport(): void
     {
@@ -137,7 +173,7 @@ class BudgetReportGenerator
     }
 
     /**
-     * Process expenses etc. for a single budget.
+     * Process expenses etc. for a single budget for the budgets block on the default report.
      *
      * @param Budget $budget
      */
@@ -160,7 +196,8 @@ class BudgetReportGenerator
     }
 
     /**
-     * Process a single budget limit.
+     * Process a single budget limit for the budgets block on the default report.
+     *
      * @param Budget      $budget
      * @param BudgetLimit $limit
      */
@@ -212,7 +249,7 @@ class BudgetReportGenerator
     }
 
     /**
-     *
+     * Calculate the expenses for transactions without a budget. Part of the "budgets" block of the default report.
      */
     private function noBudgetReport(): void
     {
@@ -266,7 +303,7 @@ class BudgetReportGenerator
     }
 
     /**
-     *
+     * Calculate the percentages for each budget. Part of the "budgets" block on the default report.
      */
     private function percentageReport(): void
     {
@@ -295,4 +332,46 @@ class BudgetReportGenerator
             }
         }
     }
+
+    /**
+     * Process each row of expenses collected for the "Account per budget" partial
+     *
+     * @param array $expenses
+     */
+    private function processExpenses(array $expenses): void
+    {
+        foreach ($expenses['budgets'] as $budget) {
+            $this->processBudgetExpenses($expenses, $budget);
+        }
+    }
+
+    /**
+     * Process each set of transactions for each row of expenses.
+     *
+     * @param array $expenses
+     * @param array $budget
+     */
+    private function processBudgetExpenses(array $expenses, array $budget): void
+    {
+        $budgetId   = (int) $budget['id'];
+        $currencyId = (int) $expenses['currency_id'];
+        foreach ($budget['transaction_journals'] as $journal) {
+            $sourceAccountId = $journal['source_account_id'];
+
+            $this->report[$sourceAccountId]['currencies'][$currencyId] =
+                $this->report[$sourceAccountId]['currencies'][$currencyId] ?? [
+                    'currency_id'             => $expenses['currency_id'],
+                    'currency_symbol'         => $expenses['currency_symbol'],
+                    'currency_name'           => $expenses['currency_name'],
+                    'currency_decimal_places' => $expenses['currency_decimal_places'],
+                    'budgets'                 => [],
+                ];
+
+            $this->report[$sourceAccountId]['currencies'][$currencyId]['budgets'][$budgetId] =
+                $this->report[$sourceAccountId]['currencies'][$currencyId]['budgets'][$budgetId] ?? '0';
+
+            $this->report[$sourceAccountId]['currencies'][$currencyId]['budgets'][$budgetId] =
+                bcadd($this->report[$sourceAccountId]['currencies'][$currencyId]['budgets'][$budgetId], $journal['amount']);
+        }
+    }
 }

routes/web.php

@@ -868,8 +868,9 @@ Route::group(
     ['middleware' => 'user-full-auth', 'namespace' => 'FireflyIII\Http\Controllers\Report', 'prefix' => 'report-data/budget', 'as' => 'report-data.budget.'],
     static function () {
 
-        // todo are these two routes still used?
+
         Route::get('general/{accountList}/{start_date}/{end_date}/', ['uses' => 'BudgetController@general', 'as' => 'general']);
+        // TODO is route still used?
         Route::get('period/{accountList}/{start_date}/{end_date}', ['uses' => 'BudgetController@period', 'as' => 'period']);
 
         Route::get('accounts/{accountList}/{budgetList}/{start_date}/{end_date}', ['uses' => 'BudgetController@accounts', 'as' => 'accounts']);