Optimised some code.

app/Crud/Account/AccountCrud.php

@@ -11,6 +11,7 @@ declare(strict_types = 1);
 
 namespace FireflyIII\Crud\Account;
 
+use Carbon\Carbon;
 use DB;
 use FireflyIII\Models\Account;
 use FireflyIII\Models\AccountMeta;
@@ -233,21 +234,8 @@ class AccountCrud implements AccountCrudInterface
             $this->storeMetadata($newAccount, $data);
         }
 
-        // continue with the opposing account:
         if ($data['openingBalance'] != 0) {
-            $opposingData = [
+            $this->storeInitialBalance($newAccount, $data);
-                'user'           => $data['user'],
-                'accountType'    => 'initial',
-                'virtualBalance' => 0,
-                'name'           => $data['name'] . ' initial balance',
-                'active'         => false,
-                'iban'           => '',
-            ];
-            $opposing     = $this->storeAccount($opposingData);
-            if (!is_null($opposing) && !is_null($newAccount)) {
-                $this->storeInitialBalance($newAccount, $opposing, $data);
-            }
-
         }
 
         return $newAccount;
@@ -282,35 +270,7 @@ class AccountCrud implements AccountCrudInterface
         $account->save();
 
         $this->updateMetadata($account, $data);
-        $openingBalance = $this->openingBalanceTransaction($account);
+        $this->updateInitialBalance($account, $data);
-        if ($data['openingBalance'] != 0) {
-            if (!is_null($openingBalance->id)) {
-                $this->updateInitialBalance($account, $openingBalance, $data);
-
-                return $account;
-            }
-
-            $type         = $data['openingBalance'] < 0 ? 'expense' : 'revenue';
-            $opposingData = [
-                'user'           => $data['user'],
-                'accountType'    => $type,
-                'name'           => $data['name'] . ' initial balance',
-                'active'         => false,
-                'iban'           => '',
-                'virtualBalance' => 0,
-            ];
-            $opposing     = $this->storeAccount($opposingData);
-            if (!is_null($opposing)) {
-                $this->storeInitialBalance($account, $opposing, $data);
-            }
-
-            return $account;
-
-        }
-
-        if ($openingBalance) { // opening balance is zero, should we delete it?
-            $openingBalance->delete(); // delete existing opening balance.
-        }
 
         return $account;
     }
@@ -359,19 +319,21 @@ class AccountCrud implements AccountCrudInterface
 
     /**
      * @param Account $account
-     * @param Account $opposing
      * @param array   $data
      *
      * @return TransactionJournal
      */
-    protected function storeInitialBalance(Account $account, Account $opposing, array $data): TransactionJournal
+    protected function storeInitialBalance(Account $account, array $data): TransactionJournal
     {
+        $amount          = $data['openingBalance'];
+        $user            = $data['user'];
+        $name            = $data['name'];
+        $opposing        = $this->storeOpposingAccount($amount, $user, $name);
         $transactionType = TransactionType::whereType(TransactionType::OPENING_BALANCE)->first();
         $journal         = TransactionJournal::create(
             [
                 'user_id'                 => $data['user'],
                 'transaction_type_id'     => $transactionType->id,
-                'bill_id'                 => null,
                 'transaction_currency_id' => $data['openingBalanceCurrency'],
                 'description'             => 'Initial balance for "' . $account->name . '"',
                 'completed'               => true,
@@ -382,24 +344,22 @@ class AccountCrud implements AccountCrudInterface
 
         $firstAccount  = $account;
         $secondAccount = $opposing;
-        $firstAmount   = $data['openingBalance'];
+        $firstAmount   = $amount;
-        $secondAmount  = $data['openingBalance'] * -1;
+        $secondAmount  = $amount * -1;
 
         if ($data['openingBalance'] < 0) {
             $firstAccount  = $opposing;
             $secondAccount = $account;
-            $firstAmount   = $data['openingBalance'] * -1;
+            $firstAmount   = $amount * -1;
-            $secondAmount  = $data['openingBalance'];
+            $secondAmount  = $amount;
         }
 
         $one = new Transaction(['account_id' => $firstAccount->id, 'transaction_journal_id' => $journal->id, 'amount' => $firstAmount]);
         $one->save();// first transaction: from
-
         $two = new Transaction(['account_id' => $secondAccount->id, 'transaction_journal_id' => $journal->id, 'amount' => $secondAmount]);
         $two->save(); // second transaction: to
 
         return $journal;
-
     }
 
     /**
@@ -425,30 +385,32 @@ class AccountCrud implements AccountCrudInterface
     }
 
     /**
-     * @param Account            $account
+     * @param Account $account
-     * @param TransactionJournal $journal
+     * @param array   $data
-     * @param array              $data
      *
-     * @return TransactionJournal
+     * @return bool
      */
-    protected function updateInitialBalance(Account $account, TransactionJournal $journal, array $data): TransactionJournal
+    protected function updateInitialBalance(Account $account, array $data): bool
     {
-        $journal->date = $data['openingBalanceDate'];
+        $openingBalance = $this->openingBalanceTransaction($account);
-        $journal->save();
+        if ($data['openingBalance'] != 0) {
+            if (!is_null($openingBalance->id)) {
+                $date   = $data['openingBalanceDate'];
+                $amount = $data['openingBalance'];
 
-        /** @var Transaction $transaction */
+                return $this->updateJournal($account, $openingBalance, $date, $amount);
-        foreach ($journal->transactions()->get() as $transaction) {
-            if ($account->id == $transaction->account_id) {
-                $transaction->amount = $data['openingBalance'];
-                $transaction->save();
-            }
-            if ($account->id != $transaction->account_id) {
-                $transaction->amount = $data['openingBalance'] * -1;
-                $transaction->save();
             }
+
+            $this->storeInitialBalance($account, $data);
+
+            return true;
+        }
+        // else, delete it:
+        if ($openingBalance) { // opening balance is zero, should we delete it?
+            $openingBalance->delete(); // delete existing opening balance.
         }
 
-        return $journal;
+        return true;
     }
 
     /**
@@ -501,4 +463,56 @@ class AccountCrud implements AccountCrudInterface
 
         return $journal;
     }
+
+    /**
+     * @param float  $amount
+     * @param int    $user
+     * @param string $name
+     *
+     * @return Account
+     */
+    private function storeOpposingAccount(float $amount, int $user, string $name):Account
+    {
+        $type         = $amount < 0 ? 'expense' : 'revenue';
+        $opposingData = [
+            'user'           => $user,
+            'accountType'    => $type,
+            'name'           => $name . ' initial balance',
+            'active'         => false,
+            'iban'           => '',
+            'virtualBalance' => 0,
+        ];
+
+        return $this->storeAccount($opposingData);
+    }
+
+    /**
+     * @param Account            $account
+     * @param TransactionJournal $journal
+     * @param Carbon             $date
+     * @param float              $amount
+     *
+     * @return bool
+     */
+    private function updateJournal(Account $account, TransactionJournal $journal, Carbon $date, float $amount): bool
+    {
+        // update date:
+        $journal->date = $date;
+        $journal->save();
+        // update transactions:
+        /** @var Transaction $transaction */
+        foreach ($journal->transactions()->get() as $transaction) {
+            if ($account->id == $transaction->account_id) {
+                $transaction->amount = $amount;
+                $transaction->save();
+            }
+            if ($account->id != $transaction->account_id) {
+                $transaction->amount = $amount * -1;
+                $transaction->save();
+            }
+        }
+
+        return true;
+
+    }
 }

app/Helpers/Report/BalanceReportHelper.php

@@ -58,8 +58,7 @@ class BalanceReportHelper implements BalanceReportHelperInterface
      */
     public function getBalanceReport(Carbon $start, Carbon $end, Collection $accounts): Balance
     {
-        $balance = new Balance;
+        $balance          = new Balance;
-        // build a balance header:
         $header           = new BalanceHeader;
         $limitRepetitions = $this->budgetRepository->getAllBudgetLimitRepetitions($start, $end);
         foreach ($accounts as $account) {

app/Http/Controllers/Chart/BudgetController.php

@@ -32,9 +32,12 @@ use Response;
 class BudgetController extends Controller
 {
 
-    /** @var  \FireflyIII\Generator\Chart\Budget\BudgetChartGeneratorInterface */
+    /** @var BudgetChartGeneratorInterface */
     protected $generator;
 
+    /** @var BudgetRepositoryInterface */
+    protected $repository;
+
     /**
      *
      */
@@ -43,6 +46,8 @@ class BudgetController extends Controller
         parent::__construct();
         // create chart generator:
         $this->generator = app(BudgetChartGeneratorInterface::class);
+
+        $this->repository = app(BudgetRepositoryInterface::class);
     }
 
     /**
@@ -136,11 +141,9 @@ class BudgetController extends Controller
     /**
      * Shows a budget list with spent/left/overspent.
      *
-     * @param BudgetRepositoryInterface $repository
-     *
      * @return \Symfony\Component\HttpFoundation\Response
      */
-    public function frontpage(BudgetRepositoryInterface $repository)
+    public function frontpage()
     {
         $start = session('start', Carbon::now()->startOfMonth());
         $end   = session('end', Carbon::now()->endOfMonth());
@@ -153,54 +156,26 @@ class BudgetController extends Controller
         if ($cache->has()) {
             return Response::json($cache->get());
         }
-        $budgets     = $repository->getActiveBudgets();
+        $budgets     = $this->repository->getActiveBudgets();
-        $repetitions = $repository->getAllBudgetLimitRepetitions($start, $end);
+        $repetitions = $this->repository->getAllBudgetLimitRepetitions($start, $end);
         $allEntries  = new Collection;
-        $format      = strval(trans('config.month_and_day'));
 
         /** @var Budget $budget */
         foreach ($budgets as $budget) {
             // get relevant repetitions:
-            $name = $budget->name;
+            $reps = $this->filterRepetitions($repetitions, $budget, $start, $end);
-            $reps = $repetitions->filter(
+
-                function (LimitRepetition $repetition) use ($budget, $start, $end) {
-                    if ($repetition->startdate < $end && $repetition->enddate > $start && $repetition->budget_id === $budget->id) {
-                        return $repetition;
-                    }
-                }
-            );
             if ($reps->count() === 0) {
-                $amount    = '0';
+                $collection = $this->spentInPeriodSingle($budget, $start, $end);
-                $left      = '0';
+                $allEntries = $allEntries->merge($collection);
-                $spent     = $repository->spentInPeriod(new Collection([$budget]), new Collection, $start, $end);
+                continue;
-                $overspent = '0';
-                $allEntries->push([$name, $left, $spent, $overspent, $amount, $spent]);
-            }
-            /** @var LimitRepetition $repetition */
-            foreach ($reps as $repetition) {
-                $expenses = $repository->spentInPeriod(new Collection([$budget]), new Collection, $repetition->startdate, $repetition->enddate);
-                if ($reps->count() > 1) {
-                    $name = $budget->name . ' ' . trans(
-                            'firefly.between_dates',
-                            ['start' => $repetition->startdate->formatLocalized($format), 'end' => $repetition->enddate->formatLocalized($format)]
-                        );
-                }
-                $amount    = $repetition->amount;
-                $left      = bccomp(bcadd($amount, $expenses), '0') < 1 ? '0' : bcadd($amount, $expenses);
-                $spent     = bccomp(bcadd($amount, $expenses), '0') < 1 ? bcmul($amount, '-1') : $expenses;
-                $overspent = bccomp(bcadd($amount, $expenses), '0') < 1 ? bcadd($amount, $expenses) : '0';
-                $allEntries->push([$name, $left, $spent, $overspent, $amount, $spent]);
             }
+            $collection = $this->spentInPeriodMulti($budget, $reps);
+            $allEntries = $allEntries->merge($collection);
 
         }
-
+        $entry = $this->spentInPeriodWithout($start, $end);
-        $list = $repository->journalsInPeriodWithoutBudget(new Collection, $start, $end);
+        $allEntries->push($entry);
-        $sum  = '0';
-        /** @var TransactionJournal $entry */
-        foreach ($list as $entry) {
-            $sum = bcadd(TransactionJournal::amount($entry), $sum);
-        }
-        $allEntries->push([trans('firefly.no_budget'), '0', '0', $sum, '0', '0']);
         $data = $this->generator->frontpage($allEntries);
         $cache->store($data);
 
@@ -335,4 +310,94 @@ class BudgetController extends Controller
 
         return Response::json($data);
     }
+
+    /**
+     * @param Collection $repetitions
+     * @param Budget     $budget
+     * @param Carbon     $start
+     * @param Carbon     $end
+     *
+     * @return Collection
+     */
+    private function filterRepetitions(Collection $repetitions, Budget $budget, Carbon $start, Carbon $end): Collection
+    {
+
+        return $repetitions->filter(
+            function (LimitRepetition $repetition) use ($budget, $start, $end) {
+                if ($repetition->startdate < $end && $repetition->enddate > $start && $repetition->budget_id === $budget->id) {
+                    return $repetition;
+                }
+            }
+        );
+    }
+
+    /**
+     * @param Budget     $budget
+     * @param Collection $repetitions
+     *
+     * @return Collection
+     */
+    private function spentInPeriodMulti(Budget $budget, Collection $repetitions): Collection
+    {
+        $format     = strval(trans('config.month_and_day'));
+        $collection = new Collection;
+        $name       = $budget->name;
+        /** @var LimitRepetition $repetition */
+        foreach ($repetitions as $repetition) {
+            $expenses = $this->repository->spentInPeriod(new Collection([$budget]), new Collection, $repetition->startdate, $repetition->enddate);
+
+            if ($repetitions->count() > 1) {
+                $name = $budget->name . ' ' . trans(
+                        'firefly.between_dates',
+                        ['start' => $repetition->startdate->formatLocalized($format), 'end' => $repetition->enddate->formatLocalized($format)]
+                    );
+            }
+            $amount    = $repetition->amount;
+            $left      = bccomp(bcadd($amount, $expenses), '0') < 1 ? '0' : bcadd($amount, $expenses);
+            $spent     = bccomp(bcadd($amount, $expenses), '0') < 1 ? bcmul($amount, '-1') : $expenses;
+            $overspent = bccomp(bcadd($amount, $expenses), '0') < 1 ? bcadd($amount, $expenses) : '0';
+            $array     = [$name, $left, $spent, $overspent, $amount, $spent];
+            $collection->push($array);
+        }
+
+        return $collection;
+    }
+
+    /**
+     * @param Budget $budget
+     * @param Carbon $start
+     * @param Carbon $end
+     *
+     * @return Collection
+     */
+    private function spentInPeriodSingle(Budget $budget, Carbon $start, Carbon $end): Collection
+    {
+        $collection = new Collection;
+        $amount     = '0';
+        $left       = '0';
+        $spent      = $this->repository->spentInPeriod(new Collection([$budget]), new Collection, $start, $end);
+        $overspent  = '0';
+        $array      = [$budget->name, $left, $spent, $overspent, $amount, $spent];
+        $collection->push($array);
+
+        return $collection;
+    }
+
+    /**
+     * @param Carbon $start
+     * @param Carbon $end
+     *
+     * @return array
+     */
+    private function spentInPeriodWithout(Carbon $start, Carbon $end):array
+    {
+        $list = $this->repository->journalsInPeriodWithoutBudget(new Collection, $start, $end);
+        $sum  = '0';
+        /** @var TransactionJournal $entry */
+        foreach ($list as $entry) {
+            $sum = bcadd(TransactionJournal::amount($entry), $sum);
+        }
+
+        return [trans('firefly.no_budget'), '0', '0', $sum, '0', '0'];
+    }
 }

app/Import/Converter/AccountId.php

@@ -32,17 +32,13 @@ class AccountId extends BasicConverter implements ConverterInterface
     {
         $value = intval(trim($value));
         Log::debug('Going to convert using AssetAccountId', ['value' => $value]);
-
         if ($value === 0) {
             $this->setCertainty(0);
 
             return new Account;
         }
-
         /** @var AccountCrudInterface $repository */
         $repository = app(AccountCrudInterface::class, [$this->user]);
-
-
         if (isset($this->mapping[$value])) {
             Log::debug('Found account in mapping. Should exist.', ['value' => $value, 'map' => $this->mapping[$value]]);
             $account = $repository->find(intval($this->mapping[$value]));
@@ -54,20 +50,15 @@ class AccountId extends BasicConverter implements ConverterInterface
                 return $account;
             }
         }
-
+        $account = $repository->find($value);// not mapped? Still try to find it first:
-        // not mapped? Still try to find it first:
-        $account = $repository->find($value);
         if (!is_null($account->id)) {
             $this->setCertainty(90);
-
             Log::debug('Found account by ID ', ['id' => $account->id]);
 
             return $account;
         }
+        $this->setCertainty(0); // should not really happen. If the ID does not match FF, what is FF supposed to do?
 
-        $this->setCertainty(0);
-
-        // should not really happen. If the ID does not match FF, what is FF supposed to do?
         return new Account;
 
     }

app/Import/Converter/CurrencySymbol.php

@@ -33,7 +33,7 @@ class CurrencySymbol extends BasicConverter implements ConverterInterface
         $value = trim($value);
         Log::debug('Going to convert using CurrencySymbol', ['value' => $value]);
 
-        if ($value === 0) {
+        if (strlen($value) === 0) {
             $this->setCertainty(0);
             return new TransactionCurrency;
         }

app/Import/ImportEntry.php

@@ -24,6 +24,8 @@ class ImportEntry
     public $amount;
 
 
+
+
     /**
      * @param string $role
      * @param string $value
@@ -34,6 +36,8 @@ class ImportEntry
      */
     public function importValue(string $role, string $value, int $certainty, $convertedValue)
     {
+        Log::debug('Going to import', ['role' => $role, 'value' => $value, 'certainty' => $certainty]);
+
         switch ($role) {
             default:
                 Log::error('Import entry cannot handle object.', ['role' => $role]);
@@ -41,11 +45,12 @@ class ImportEntry
                 break;
 
             case 'amount':
+                /*
+                 * Easy enough.
+                 */
                 $this->setAmount($convertedValue);
-                return;
-            case 'account-id':
 
-                break;
+                return;
         }
     }