Various code cleanup.

app/Helpers/Report/AccountReportHelper.php

@@ -26,7 +26,12 @@ class AccountReportHelper implements AccountReportHelperInterface
 {
     /**
      * This method generates a full report for the given period on all
-     * given accounts
+     * given accounts.
+     *
+     * a special consideration for accounts that did exist on this exact day.
+     * we also grab the balance from today just in case, to see if that changes things.
+     * it's a fall back for users who (rightly so) start keeping score at the first of
+     * the month and find the first report lacking / broken.
      *
      * @param Carbon     $start
      * @param Carbon     $end
@@ -42,26 +47,13 @@ class AccountReportHelper implements AccountReportHelperInterface
         $ids         = $accounts->pluck('id')->toArray();
         $yesterday   = clone $start;
         $yesterday->subDay();
-
+        $startSet  = $this->getSet($ids, $yesterday); // get balances for start.
-        // get balances for start.
-        $startSet = $this->getSet($ids, $yesterday);
-
-        // a special consideration for accounts that did exist on this exact day.
-        // we also grab the balance from today just in case, to see if that changes things.
-        // it's a fall back for users who (rightly so) start keeping score at the first of
-        // the month and find the first report lacking / broken.
         $backupSet = $this->getSet($ids, $start);
-
+        $endSet    = $this->getSet($ids, $end);
-        // and end:
-        $endSet = $this->getSet($ids, $end);
 
         $accounts->each(
             function (Account $account) use ($startSet, $endSet, $backupSet) {
-                /**
+                // The balance for today always incorporates transactions made on today. So to get todays "start" balance, we sub one day.
-                 * The balance for today always incorporates transactions
-                 * made on today. So to get todays "start" balance, we sub one
-                 * day.
-                 */
                 $account->startBalance = '0';
                 $account->endBalance   = '0';
                 $currentStart          = $startSet->filter(
@@ -69,28 +61,24 @@ class AccountReportHelper implements AccountReportHelperInterface
                         return $account->id == $entry->id;
                     }
                 );
-                // grab entry from current backup as well:
+
-                $currentBackup = $backupSet->filter(
+                $currentBackup = $backupSet->filter( // grab entry from current backup as well:
                     function (Account $entry) use ($account) {
                         return $account->id == $entry->id;
                     }
                 );
-
+                if (!is_null($currentStart->first())) {
-
-                if ($currentStart->first()) {
                     $account->startBalance = $currentStart->first()->balance;
-                } else {
-                    if (is_null($currentStart->first()) && !is_null($currentBackup->first())) {
-                        $account->startBalance = $currentBackup->first()->balance;
-                    }
                 }
-
+                if (is_null($currentStart->first()) && !is_null($currentBackup->first())) {
+                    $account->startBalance = $currentBackup->first()->balance;
+                }
                 $currentEnd = $endSet->filter(
                     function (Account $entry) use ($account) {
                         return $account->id == $entry->id;
                     }
                 );
-                if ($currentEnd->first()) {
+                if (!is_null($currentEnd->first())) {
                     $account->endBalance = $currentEnd->first()->balance;
                 }
             }
@@ -121,12 +109,12 @@ class AccountReportHelper implements AccountReportHelperInterface
     private function getSet(array $ids, Carbon $date): Collection
     {
         return Account::leftJoin('transactions', 'transactions.account_id', '=', 'accounts.id')
-               ->leftJoin('transaction_journals', 'transaction_journals.id', '=', 'transactions.transaction_journal_id')
+                      ->leftJoin('transaction_journals', 'transaction_journals.id', '=', 'transactions.transaction_journal_id')
-               ->whereIn('accounts.id', $ids)
+                      ->whereIn('accounts.id', $ids)
-               ->whereNull('transaction_journals.deleted_at')
+                      ->whereNull('transaction_journals.deleted_at')
-               ->whereNull('transactions.deleted_at')
+                      ->whereNull('transactions.deleted_at')
-               ->where('transaction_journals.date', '<=', $date->format('Y-m-d'))
+                      ->where('transaction_journals.date', '<=', $date->format('Y-m-d'))
-               ->groupBy('accounts.id')
+                      ->groupBy('accounts.id')
-               ->get(['accounts.id', DB::raw('SUM(`transactions`.`amount`) as `balance`')]);
+                      ->get(['accounts.id', DB::raw('SUM(`transactions`.`amount`) as `balance`')]);
     }
 }

app/Helpers/Report/BalanceReportHelper.php

@@ -17,6 +17,7 @@ use FireflyIII\Helpers\Collection\Balance;
 use FireflyIII\Helpers\Collection\BalanceEntry;
 use FireflyIII\Helpers\Collection\BalanceHeader;
 use FireflyIII\Helpers\Collection\BalanceLine;
+use FireflyIII\Models\Account;
 use FireflyIII\Models\Budget;
 use FireflyIII\Models\Budget as BudgetModel;
 use FireflyIII\Models\Tag;
@@ -172,6 +173,43 @@ class BalanceReportHelper implements BalanceReportHelperInterface
         return $line;
     }
 
+    /**
+     * @param Account    $account
+     * @param Collection $spentData
+     * @param Collection $tagsLeft
+     *
+     * @return BalanceEntry
+     */
+    private function createDifferenceBalanceEntry(Account $account, Collection $spentData, Collection $tagsLeft): BalanceEntry
+    {
+        $entry = $spentData->filter(
+            function (TransactionJournal $model) use ($account) {
+                return $model->account_id == $account->id && is_null($model->budget_id);
+            }
+        );
+        $spent = '0';
+        if (!is_null($entry->first())) {
+            $spent = $entry->first()->spent;
+        }
+        $leftEntry = $tagsLeft->filter(
+            function (Tag $tag) use ($account) {
+                return $tag->account_id == $account->id;
+            }
+        );
+        $left      = '0';
+        if (!is_null($leftEntry->first())) {
+            $left = $leftEntry->first()->sum;
+        }
+        $diffValue = bcadd($spent, $left);
+
+        // difference:
+        $diffEntry = new BalanceEntry;
+        $diffEntry->setAccount($account);
+        $diffEntry->setSpent($diffValue);
+
+        return $diffEntry;
+    }
+
     /**
      * @param Collection $accounts
      * @param Collection $spentData
@@ -189,31 +227,9 @@ class BalanceReportHelper implements BalanceReportHelperInterface
 
         $diff->setRole(BalanceLine::ROLE_DIFFROLE);
 
+        /** @var Account $account */
         foreach ($accounts as $account) {
-            $entry = $spentData->filter(
+            $diffEntry = $this->createDifferenceBalanceEntry($account, $spentData, $tagsLeft);
-                function (TransactionJournal $model) use ($account) {
-                    return $model->account_id == $account->id && is_null($model->budget_id);
-                }
-            );
-            $spent = '0';
-            if (!is_null($entry->first())) {
-                $spent = $entry->first()->spent;
-            }
-            $leftEntry = $tagsLeft->filter(
-                function (Tag $tag) use ($account) {
-                    return $tag->account_id == $account->id;
-                }
-            );
-            $left      = '0';
-            if (!is_null($leftEntry->first())) {
-                $left = $leftEntry->first()->sum;
-            }
-            $diffValue = bcadd($spent, $left);
-
-            // difference:
-            $diffEntry = new BalanceEntry;
-            $diffEntry->setAccount($account);
-            $diffEntry->setSpent($diffValue);
             $diff->addBalanceEntry($diffEntry);
 
         }

app/Helpers/Report/ReportHelper.php

@@ -77,7 +77,7 @@ class ReportHelper implements ReportHelperInterface
             $billLine->setActive(intval($bill->active) == 1);
             $billLine->setMin($bill->amount_min);
             $billLine->setMax($bill->amount_max);
-
+            $billLine->setHit(false);
             // is hit in period?
 
             $entry = $journals->filter(
@@ -90,8 +90,6 @@ class ReportHelper implements ReportHelperInterface
                 $billLine->setTransactionJournalId($first->id);
                 $billLine->setAmount($first->journalAmount);
                 $billLine->setHit(true);
-            } else {
-                $billLine->setHit(false);
             }
             if (!(!$billLine->isHit() && !$billLine->isActive())) {
                 $collection->addBill($billLine);
@@ -219,9 +217,7 @@ class ReportHelper implements ReportHelperInterface
         $months = [];
 
         while ($start <= $end) {
-            // current year:
+            $year = $fiscalHelper->endOfFiscalYear($start)->year; // current year
-            $year = $fiscalHelper->endOfFiscalYear($start)->year;
-
             if (!isset($months[$year])) {
                 $months[$year] = [
                     'fiscal_start' => $fiscalHelper->startOfFiscalYear($start)->format('Y-m-d'),
@@ -234,7 +230,6 @@ class ReportHelper implements ReportHelperInterface
 
             $currentEnd = clone $start;
             $currentEnd->endOfMonth();
-
             $months[$year]['months'][] = [
                 'formatted' => $start->formatLocalized('%B %Y'),
                 'start'     => $start->format('Y-m-d'),
@@ -243,8 +238,7 @@ class ReportHelper implements ReportHelperInterface
                 'year'      => $year,
             ];
 
-            // to make the hop to the next month properly:
+            $start = clone $currentEnd; // to make the hop to the next month properly
-            $start = clone $currentEnd;
             $start->addDay();
         }
 

app/Http/Controllers/AccountController.php

@@ -65,7 +65,7 @@ class AccountController extends Controller
     {
         $typeName    = config('firefly.shortNamesByFullName.' . $account->accountType->type);
         $subTitle    = trans('firefly.delete_' . $typeName . '_account', ['name' => $account->name]);
-        $accountList = ExpandedForm::makeSelectList($repository->getAccounts([$account->accountType->type]), true);
+        $accountList = ExpandedForm::makeSelectListWithEmpty($repository->getAccounts([$account->accountType->type]));
         unset($accountList[$account->id]);
 
         // put previous url in session

app/Http/Controllers/PiggyBankController.php

@@ -183,7 +183,7 @@ class PiggyBankController extends Controller
             if (!isset($accounts[$account->id])) {
                 $accounts[$account->id] = [
                     'name'              => $account->name,
-                    'balance'           => Steam::balance($account, $end, true),
+                    'balance'           => Steam::balanceIgnoreVirtual($account, $end),
                     'leftForPiggyBanks' => $repository->leftOnAccount($account, $end),
                     'sumOfSaved'        => strval($piggyBank->savedSoFar),
                     'sumOfTargets'      => strval(round($piggyBank->targetamount, 2)),

app/Http/Controllers/RuleGroupController.php

@@ -64,7 +64,7 @@ class RuleGroupController extends Controller
     {
         $subTitle = trans('firefly.delete_rule_group', ['title' => $ruleGroup->title]);
 
-        $ruleGroupList = ExpandedForm::makeSelectList($repository->get(), true);
+        $ruleGroupList = ExpandedForm::makeSelectListWithEmpty($repository->get());
         unset($ruleGroupList[$ruleGroup->id]);
 
         // put previous url in session

app/Repositories/Account/AccountRepository.php

@@ -352,8 +352,8 @@ class AccountRepository implements AccountRepositoryInterface
 
         $accounts->each(
             function (Account $account) use ($start, $end) {
-                $account->startBalance = Steam::balance($account, $start, true);
+                $account->startBalance = Steam::balanceIgnoreVirtual($account, $start);
-                $account->endBalance   = Steam::balance($account, $end, true);
+                $account->endBalance   = Steam::balanceIgnoreVirtual($account, $end);
                 $account->piggyBalance = 0;
                 /** @var PiggyBank $piggyBank */
                 foreach ($account->piggyBanks as $piggyBank) {
@@ -426,7 +426,7 @@ class AccountRepository implements AccountRepositoryInterface
     public function leftOnAccount(Account $account, Carbon $date): string
     {
 
-        $balance = Steam::balance($account, $date, true);
+        $balance = Steam::balanceIgnoreVirtual($account, $date);
         /** @var PiggyBank $p */
         foreach ($account->piggybanks()->get() as $p) {
             $balance -= $p->currentRelevantRep()->currentamount;

app/Support/ExpandedForm.php

@@ -20,31 +20,6 @@ use Session;
 class ExpandedForm
 {
 
-    /**
-     * @param       $name
-     * @param null  $value
-     * @param array $options
-     *
-     * @return string
-     */
-    public function amountSmall(string $name, $value = null, array $options = []): string
-    {
-        $label           = $this->label($name, $options);
-        $options         = $this->expandOptionArray($name, $label, $options);
-        $classes         = $this->getHolderClasses($name);
-        $value           = $this->fillFieldValue($name, $value);
-        $options['step'] = 'any';
-        $options['min']  = '0.01';
-        $defaultCurrency = isset($options['currency']) ? $options['currency'] : Amt::getDefaultCurrency();
-        $currencies      = Amt::getAllCurrencies();
-        unset($options['currency']);
-        unset($options['placeholder']);
-        $html = view('form.amount-small', compact('defaultCurrency', 'currencies', 'classes', 'name', 'value', 'options'))->render();
-
-        return $html;
-
-    }
-
     /**
      * @param       $name
      * @param null  $value
@@ -70,6 +45,31 @@ class ExpandedForm
 
     }
 
+    /**
+     * @param       $name
+     * @param null  $value
+     * @param array $options
+     *
+     * @return string
+     */
+    public function amountSmall(string $name, $value = null, array $options = []): string
+    {
+        $label           = $this->label($name, $options);
+        $options         = $this->expandOptionArray($name, $label, $options);
+        $classes         = $this->getHolderClasses($name);
+        $value           = $this->fillFieldValue($name, $value);
+        $options['step'] = 'any';
+        $options['min']  = '0.01';
+        $defaultCurrency = isset($options['currency']) ? $options['currency'] : Amt::getDefaultCurrency();
+        $currencies      = Amt::getAllCurrencies();
+        unset($options['currency']);
+        unset($options['placeholder']);
+        $html = view('form.amount-small', compact('defaultCurrency', 'currencies', 'classes', 'name', 'value', 'options'))->render();
+
+        return $html;
+
+    }
+
     /**
      * @param       $name
      * @param null  $value
@@ -196,17 +196,38 @@ class ExpandedForm
      * Takes any collection and tries to make a sensible select list compatible array of it.
      *
      * @param \Illuminate\Support\Collection $set
-     * @param bool                           $addEmpty
      *
      * @return array
      */
-    public function makeSelectList(Collection $set, bool $addEmpty = false): array
+    public function makeSelectList(Collection $set): array
     {
         $selectList = [];
-        if ($addEmpty) {
+        $fields     = ['title', 'name', 'description'];
-            $selectList[0] = '(none)';
+        /** @var Eloquent $entry */
+        foreach ($set as $entry) {
+            $entryId = intval($entry->id);
+            $title   = null;
+
+            foreach ($fields as $field) {
+                if (isset($entry->$field) && is_null($title)) {
+                    $title = $entry->$field;
+                }
+            }
+            $selectList[$entryId] = $title;
         }
-        $fields = ['title', 'name', 'description'];
+
+        return $selectList;
+    }
+
+    /**
+     * @param Collection $set
+     *
+     * @return array
+     */
+    public function makeSelectListWithEmpty(Collection $set): array
+    {
+        $selectList[0] = '(none)';
+        $fields        = ['title', 'name', 'description'];
         /** @var Eloquent $entry */
         foreach ($set as $entry) {
             $entryId = intval($entry->id);

app/Support/Steam.php

@@ -16,7 +16,6 @@ use FireflyIII\Models\Transaction;
  */
 class Steam
 {
-
     /**
      *
      * @param \FireflyIII\Models\Account $account
@@ -43,10 +42,39 @@ class Steam
                 'transaction_journals', 'transaction_journals.id', '=', 'transactions.transaction_journal_id'
             )->where('transaction_journals.date', '<=', $date->format('Y-m-d'))->sum('transactions.amount')
         );
+        $balance = bcadd($balance, $account->virtual_balance);
+        $cache->store($balance);
 
-        if (!$ignoreVirtualBalance) {
+        return $balance;
-            $balance = bcadd($balance, $account->virtual_balance);
+    }
+
+    /**
+     *
+     * @param \FireflyIII\Models\Account $account
+     * @param \Carbon\Carbon             $date
+     * @param bool                       $ignoreVirtualBalance
+     *
+     * @return string
+     */
+    public function balanceIgnoreVirtual(Account $account, Carbon $date, $ignoreVirtualBalance = false): string
+    {
+
+        // abuse chart properties:
+        $cache = new CacheProperties;
+        $cache->addProperty($account->id);
+        $cache->addProperty('balance-no-virtual');
+        $cache->addProperty($date);
+        $cache->addProperty($ignoreVirtualBalance);
+        if ($cache->has()) {
+            return $cache->get();
         }
+
+        $balance = strval(
+            $account->transactions()->leftJoin(
+                'transaction_journals', 'transaction_journals.id', '=', 'transactions.transaction_journal_id'
+            )->where('transaction_journals.date', '<=', $date->format('Y-m-d'))->sum('transactions.amount')
+        );
+
         $cache->store($balance);
 
         return $balance;