Improve code quality in budget methods.

2025-10-15 16:57:09 +00:00 · 2018-07-14 16:40:46 +02:00
parent 89834baf01
commit ff403dfa2e
4 changed files with 91 additions and 94 deletions
--- a/app/Http/Controllers/Budget/AmountController.php
+++ b/app/Http/Controllers/Budget/AmountController.php
@@ -31,13 +31,11 @@ use FireflyIII\Http\Requests\BudgetIncomeRequest;
 use FireflyIII\Models\Budget;
 use FireflyIII\Models\TransactionType;
 use FireflyIII\Repositories\Budget\BudgetRepositoryInterface;
-use FireflyIII\Support\CacheProperties;
 use FireflyIII\Support\Http\Controllers\DateCalculation;
 use Illuminate\Http\JsonResponse;
 use Illuminate\Http\RedirectResponse;
 use Illuminate\Http\Request;
 use Illuminate\Support\Collection;
-use Log;

 /**
 *
@@ -92,21 +90,13 @@ class AmountController extends Controller
        $periodLength  = $start->diffInDays($end);
        $dayDifference = $this->getDayDifference($start, $end);

-
-        /*
-         * If the user budgets ANY amount per day for this budget (anything but zero)
-         * Firefly III calculates how much he could spend per day.
-         */
+        // If the user budgets ANY amount per day for this budget (anything but zero) Firefly III calculates how much he could spend per day.
        if (1 === bccomp(bcadd($amount, $spent), '0')) {
            $leftPerDay = app('amount')->formatAnything($currency, bcdiv(bcadd($amount, $spent), (string)$dayDifference), true);
        }

-        /*
-         * Get the average amount of money the user budgets for this budget.
-         * And calculate the same for the current amount.
-         *
-         * If the difference is very large, give the user a notification.
-         */
+        // Get the average amount of money the user budgets for this budget. And calculate the same for the current amount.
+        // If the difference is very large, give the user a notification.
        $average = $this->repository->budgetedPerDay($budget);
        $current = bcdiv($amount, (string)$periodLength);
        if (bccomp(bcmul('1.1', $average), $current) === -1) {
@@ -123,17 +113,8 @@ class AmountController extends Controller
        app('preferences')->mark();

        return response()->json(
-            [
-                'left'         => $left,
-                'name'         => $budget->name,
-                'limit'        => $budgetLimit ? $budgetLimit->id : 0,
-                'amount'       => $amount,
-                'current'      => $current,
-                'average'      => $average,
-                'large_diff'   => $largeDiff,
-                'left_per_day' => $leftPerDay,
-                'warn_text'    => $warnText,
-            ]
+            ['left'    => $left, 'name' => $budget->name, 'limit' => $budgetLimit ? $budgetLimit->id : 0, 'amount' => $amount, 'current' => $current,
+             'average' => $average, 'large_diff' => $largeDiff, 'left_per_day' => $leftPerDay, 'warn_text' => $warnText,]
        );
    }

@@ -146,75 +127,51 @@ class AmountController extends Controller
     */
    public function infoIncome(Carbon $start, Carbon $end)
    {
-        // properties for cache
-        $cache = new CacheProperties;
-        $cache->addProperty($start);
-        $cache->addProperty($end);
-        $cache->addProperty('info-income');
+        $range = app('preferences')->get('viewRange', '1M')->data;

-        Log::debug(sprintf('infoIncome start is %s', $start->format('Y-m-d')));
-        Log::debug(sprintf('infoIncome end is %s', $end->format('Y-m-d')));
+        /** @var Carbon $searchBegin */
+        $searchBegin        = app('navigation')->subtractPeriod($start, $range, 3);
+        $searchEnd          = app('navigation')->addPeriod($end, $range, 3);
+        $daysInPeriod       = $start->diffInDays($end);
+        $daysInSearchPeriod = $searchBegin->diffInDays($searchEnd);

-        if ($cache->has()) {
-            // @codeCoverageIgnoreStart
-            $result = $cache->get();
-
-            return view('budgets.info', compact('result'));
-            // @codeCoverageIgnoreEnd
-        }
-        $result   = [
-            'available' => '0',
-            'earned'    => '0',
-            'suggested' => '0',
-        ];
-        $currency = app('amount')->getDefaultCurrency();
-        $range    = app('preferences')->get('viewRange', '1M')->data;
-        /** @var Carbon $begin */
-        $begin = app('navigation')->subtractPeriod($start, $range, 3);
-
-        Log::debug(sprintf('Range is %s', $range));
-        Log::debug(sprintf('infoIncome begin is %s', $begin->format('Y-m-d')));
-
-        // get average amount available.
-        $total        = '0';
-        $count        = 0;
-        $currentStart = clone $begin;
-        while ($currentStart < $start) {
-
-            Log::debug(sprintf('Loop: currentStart is %s', $currentStart->format('Y-m-d')));
-            $currentEnd   = app('navigation')->endOfPeriod($currentStart, $range);
-            $total        = bcadd($total, $this->repository->getAvailableBudget($currency, $currentStart, $currentEnd));
-            $currentStart = app('navigation')->addPeriod($currentStart, $range, 0);
-            ++$count;
-        }
-        Log::debug('Loop end');
-
-        if (0 === $count) {
-            $count = 1;
-        }
-        $result['available'] = bcdiv($total, (string)$count);
+        // get average available budget from repository:
+        $average   = $this->repository->getAverageAvailable($start, $end);
+        $available = bcmul($average, (string)$daysInPeriod);

        // amount earned in this period:
-        $subDay = clone $end;
-        $subDay->subDay();
        /** @var JournalCollectorInterface $collector */
        $collector = app(JournalCollectorInterface::class);
-        $collector->setAllAssetAccounts()->setRange($begin, $subDay)->setTypes([TransactionType::DEPOSIT])->withOpposingAccount();
-        $result['earned'] = bcdiv((string)$collector->getJournals()->sum('transaction_amount'), (string)$count);
+        $collector->setAllAssetAccounts()->setRange($searchBegin, $searchEnd)->setTypes([TransactionType::DEPOSIT])->withOpposingAccount();
+        $earned = (string)$collector->getJournals()->sum('transaction_amount');
+        // Total amount earned divided by the number of days in the whole search period is the average amount earned per day.
+        // This is multiplied by the number of days in the current period, showing you the average.
+        $earnedAverage = bcmul(bcdiv($earned, (string)$daysInSearchPeriod), (string)$daysInPeriod);

        // amount spent in period
        /** @var JournalCollectorInterface $collector */
        $collector = app(JournalCollectorInterface::class);
-        $collector->setAllAssetAccounts()->setRange($begin, $subDay)->setTypes([TransactionType::WITHDRAWAL])->withOpposingAccount();
-        $result['spent'] = bcdiv((string)$collector->getJournals()->sum('transaction_amount'), (string)$count);
-        // suggestion starts with the amount spent
-        $result['suggested'] = bcmul($result['spent'], '-1');
-        $result['suggested'] = 1 === bccomp($result['suggested'], $result['earned']) ? $result['earned'] : $result['suggested'];
-        // unless it's more than you earned. So min() of suggested/earned
+        $collector->setAllAssetAccounts()->setRange($searchBegin, $searchEnd)->setTypes([TransactionType::WITHDRAWAL])->withOpposingAccount();
+        $spent        = (string)$collector->getJournals()->sum('transaction_amount');
+        $spentAverage = app('steam')->positive(bcmul(bcdiv($spent, (string)$daysInSearchPeriod), (string)$daysInPeriod));

-        $cache->store($result);
+        // the default suggestion is the money the user has spent, on average, over this period.
+        $suggested = $spentAverage;

-        return view('budgets.info', compact('result', 'begin', 'currentEnd'));
+        // if the user makes less per period, suggest that amount instead.
+        if (bccomp($spentAverage, $earnedAverage) === 1) {
+            $suggested = $earnedAverage;
+        }
+
+        $result
+            = [
+            'available' => $available,
+            'earned'    => $earnedAverage,
+            'spent'     => $spentAverage,
+            'suggested' => $suggested,
+        ];
+
+        return view('budgets.info', compact('result', 'searchBegin', 'searchEnd', 'start', 'end'));
    }


--- a/app/Repositories/Budget/BudgetRepository.php
+++ b/app/Repositories/Budget/BudgetRepository.php
@@ -439,6 +439,37 @@ class BudgetRepository implements BudgetRepositoryInterface
        return $this->user->availableBudgets()->get();
    }

+    /**
+     * Calculate the average amount in the budgets available in this period.
+     * Grouped by day.
+     *
+     * @param Carbon $start
+     * @param Carbon $end
+     *
+     * @return string
+     */
+    public function getAverageAvailable(Carbon $start, Carbon $end): string
+    {
+        /** @var Collection $list */
+        $list = $this->user->availableBudgets()
+                           ->where('start_date', '>=', $start->format('Y-m-d 00:00:00'))
+                           ->where('end_date', '<=', $end->format('Y-m-d 00:00:00'))
+                           ->get();
+        if (0 === $list->count()) {
+            return '0';
+        }
+        $total = '0';
+        $days  = 0;
+        /** @var AvailableBudget $availableBudget */
+        foreach ($list as $availableBudget) {
+            $total = bcadd($availableBudget->amount, $total);
+            $days  += $availableBudget->start_date->diffInDays($availableBudget->end_date);
+        }
+        $avg = bcdiv($total, (string)$days);
+
+        return $avg;
+    }
+
    /**
     * @param Budget $budget
     * @param Carbon $start
--- a/app/Repositories/Budget/BudgetRepositoryInterface.php
+++ b/app/Repositories/Budget/BudgetRepositoryInterface.php
@@ -46,13 +46,6 @@ interface BudgetRepositoryInterface
     */
    public function budgetedPerDay(Budget $budget): string;

-    /**
-     * Destroy a budget limit.
-     *
-     * @param BudgetLimit $budgetLimit
-     */
-    public function destroyBudgetLimit(BudgetLimit $budgetLimit): void;
-
    /**
     * @return bool
     */
@@ -89,6 +82,13 @@ interface BudgetRepositoryInterface
     */
    public function destroyAvailableBudget(AvailableBudget $availableBudget): void;

+    /**
+     * Destroy a budget limit.
+     *
+     * @param BudgetLimit $budgetLimit
+     */
+    public function destroyBudgetLimit(BudgetLimit $budgetLimit): void;
+
    /**
     * Filters entries from the result set generated by getBudgetPeriodReport.
     *
@@ -166,6 +166,17 @@ interface BudgetRepositoryInterface
     */
    public function getAvailableBudgets(): Collection;

+    /**
+     * Calculate the average amount in the budgets available in this period.
+     * Grouped by day.
+     *
+     * @param Carbon $start
+     * @param Carbon $end
+     *
+     * @return string
+     */
+    public function getAverageAvailable(Carbon $start, Carbon $end): string;
+
    /**
     * @param Budget $budget
     * @param Carbon $start
@@ -238,7 +249,6 @@ interface BudgetRepositoryInterface
     */
    public function spentInPeriod(Collection $budgets, Collection $accounts, Carbon $start, Carbon $end): string;

-
    /**
     * @param Collection $accounts
     * @param Carbon     $start
--- a/resources/views/budgets/info.twig
+++ b/resources/views/budgets/info.twig
@@ -4,8 +4,7 @@
            <button type="button" class="close" data-dismiss="modal"><span>&times;</span><span class="sr-only">{{ 'close'|_ }}</span>
            </button>
            <h4 class="modal-title">
-                {{ trans('firefly.info_on_available_amount',
-                    {start: start.formatLocalized(monthAndDayFormat), end: end.formatLocalized(monthAndDayFormat)}) }}
+                {{ trans('firefly.info_on_available_amount') }}
            </h4>
        </div>

@@ -18,7 +17,7 @@
                    <td>
                        {{ 'budgeted'|_ }}
                        <small><br/>
-                            {{ trans('firefly.average_between', {start:begin.formatLocalized(monthAndDayFormat), end:currentEnd.formatLocalized(monthAndDayFormat)}) }}
+                            {{ trans('firefly.average_between', {start:searchBegin.formatLocalized(monthAndDayFormat), end:searchEnd.formatLocalized(monthAndDayFormat)}) }}
                    </td>
                    <td>
                        <span class="pull-right">{{ result.available|formatAmount }}</span>
@@ -27,7 +26,7 @@
                <tr>
                    <td>{{ 'earned'|_ }}
                        <small><br/>
-                            {{ trans('firefly.average_between', {start:begin.formatLocalized(monthAndDayFormat), end:currentEnd.formatLocalized(monthAndDayFormat)}) }}
+                            {{ trans('firefly.average_between', {start:searchBegin.formatLocalized(monthAndDayFormat), end:searchEnd.formatLocalized(monthAndDayFormat)}) }}
                        </small>
                    </td>
                    <td><span class="pull-right">{{ result.earned|formatAmount }}</span></td>
@@ -35,7 +34,7 @@
                <tr>
                    <td>{{ 'spent'|_ }}
                        <small><br/>
-                            {{ trans('firefly.average_between', {start:begin.formatLocalized(monthAndDayFormat), end:currentEnd.formatLocalized(monthAndDayFormat)}) }}
+                            {{ trans('firefly.average_between', {start:searchBegin.formatLocalized(monthAndDayFormat), end:searchEnd.formatLocalized(monthAndDayFormat)}) }}
                        </small>
                    </td>
                    <td><span class="pull-right">{{ result.spent|formatAmount }}</span></td>