Various code clean up.

Signed-off-by: James Cole <thegrumpydictator@gmail.com>

app/Console/Commands/Import.php

@@ -11,12 +11,8 @@ declare(strict_types = 1);
 
 namespace FireflyIII\Console\Commands;
 
-use FireflyIII\Crud\Account\AccountCrud;
-use FireflyIII\Import\Importer\ImporterInterface;
 use FireflyIII\Import\ImportProcedure;
 use FireflyIII\Import\ImportResult;
-use FireflyIII\Import\ImportStorage;
-use FireflyIII\Import\ImportValidator;
 use FireflyIII\Import\Logging\CommandHandler;
 use FireflyIII\Models\ImportJob;
 use Illuminate\Console\Command;
@@ -61,21 +57,12 @@ class Import extends Command
     {
         $jobKey = $this->argument('key');
         $job    = ImportJob::whereKey($jobKey)->first();
-        if (is_null($job)) {
+        if (!$this->isValid($job)) {
-            $this->error('This job does not seem to exist.');
-
-            return;
-        }
-
-        if ($job->status != 'settings_complete') {
-            $this->error('This job is not ready to be imported.');
-
             return;
         }
 
         $this->line('Going to import job with key "' . $job->key . '" of type ' . $job->file_type);
 
-        // intercept log entries and print them on the command line
         $monolog = Log::getMonolog();
         $handler = new CommandHandler($this);
         $monolog->pushHandler($handler);
@@ -99,4 +86,26 @@ class Import extends Command
         $this->line('The import has completed.');
 
     }
+
+    /**
+     * @param ImportJob $job
+     *
+     * @return bool
+     */
+    private function isValid(ImportJob $job): bool
+    {
+        if (is_null($job)) {
+            $this->error('This job does not seem to exist.');
+
+            return false;
+        }
+
+        if ($job->status != 'settings_complete') {
+            $this->error('This job is not ready to be imported.');
+
+            return false;
+        }
+
+        return true;
+    }
 }

app/Crud/Account/AccountCrud.php

@@ -27,6 +27,8 @@ use Log;
 /**
  * Class AccountCrud
  *
+ * @SuppressWarnings(PHPMD.ExcessiveClassComplexity)
+ *
  * @package FireflyIII\Crud\Account
  */
 class AccountCrud implements AccountCrudInterface
@@ -56,15 +58,12 @@ class AccountCrud implements AccountCrudInterface
     public function destroy(Account $account, Account $moveTo): bool
     {
         if (!is_null($moveTo->id)) {
-            // update all transactions:
             DB::table('transactions')->where('account_id', $account->id)->update(['account_id' => $moveTo->id]);
         }
         if (!is_null($account)) {
-            Log::debug('Now trigger account delete #' . $account->id);
             $account->delete();
         }
 
-
         return true;
     }
 
@@ -75,7 +74,6 @@ class AccountCrud implements AccountCrudInterface
      */
     public function find(int $accountId): Account
     {
-        Log::debug('Searching for user ', ['user' => $this->user->id]);
         $account = $this->user->accounts()->find($accountId);
         if (is_null($account)) {
             return new Account;
@@ -146,7 +144,6 @@ class AccountCrud implements AccountCrudInterface
     public function findByName(string $name, array $types): Account
     {
         $query = $this->user->accounts();
-        Log::debug('Now in findByName()', ['name' => $name, 'types' => $types]);
 
         if (count($types) > 0) {
             $query->leftJoin('account_types', 'accounts.account_type_id', '=', 'account_types.id');
@@ -154,16 +151,13 @@ class AccountCrud implements AccountCrudInterface
         }
 
         $accounts = $query->get(['accounts.*']);
-        Log::debug(sprintf('Total set count is %d ', $accounts->count()));
         /** @var Account $account */
         foreach ($accounts as $account) {
             if ($account->name === $name) {
-                Log::debug('Account name is an exact match. ', ['db' => $account->name, 'source' => $name, 'id' => $account->id]);
 
                 return $account;
             }
         }
-        Log::debug('Found nothing in findByName()', ['name' => $name, 'types' => $types]);
 
         return new Account;
     }
@@ -187,7 +181,6 @@ class AccountCrud implements AccountCrudInterface
         }
 
         $result = $query->get(['accounts.*']);
-
         $result = $result->sortBy(
             function (Account $account) {
                 return strtolower($account->name);
@@ -215,7 +208,6 @@ class AccountCrud implements AccountCrudInterface
         }
 
         $result = $query->get(['accounts.*']);
-
         $result = $result->sortBy(
             function (Account $account) {
                 return strtolower($account->name);
@@ -229,8 +221,6 @@ class AccountCrud implements AccountCrudInterface
      * @param array $data
      *
      * @return Account
-     *
-     * @SuppressWarnings(PHPMD.CyclomaticComplexity)
      */
     public function store(array $data): Account
     {
@@ -330,6 +320,7 @@ class AccountCrud implements AccountCrudInterface
             $existingAccount = Account::firstOrNullEncrypted($searchData);
             if (!$existingAccount) {
                 Log::error('Account create error', $newAccount->getErrors()->toArray());
+
                 return new Account;
             }
             $newAccount = $existingAccount;
@@ -402,8 +393,6 @@ class AccountCrud implements AccountCrudInterface
                 );
                 $metaData->save();
             }
-
-
         }
     }
 

app/Handlers/Events/UpdateJournalConnection.php

@@ -27,6 +27,7 @@ class UpdateJournalConnection
      * Handle the event.
      *
      * @param  TransactionJournalUpdated $event
+     * @SuppressWarnings(PHPMD.CyclomaticComplexity) // it's exactly 5.
      *
      * @return bool
      */
@@ -46,7 +47,7 @@ class UpdateJournalConnection
         }
         $piggyBank  = $event->piggyBank()->first();
         $repetition = null;
-        if ($piggyBank) {
+        if (!is_null($piggyBank)) {
             /** @var PiggyBankRepetition $repetition */
             $repetition = $piggyBank->piggyBankRepetitions()->relevantOnDate($journal->date)->first();
         }

app/Helpers/Collection/Budget.php

@@ -41,46 +41,61 @@ class Budget
 
     /**
      * @param BudgetLine $budgetLine
+     *
+     * @return Budget
      */
-    public function addBudgetLine(BudgetLine $budgetLine)
+    public function addBudgetLine(BudgetLine $budgetLine): Budget
     {
         $this->budgetLines->push($budgetLine);
+        return $this;
     }
 
     /**
      * @param string $add
+     *
+     * @return Budget
      */
-    public function addBudgeted(string $add)
+    public function addBudgeted(string $add): Budget
     {
         $add            = strval(round($add, 2));
         $this->budgeted = bcadd($this->budgeted, $add);
+        return $this;
     }
 
     /**
      * @param string $add
+     *
+     * @return Budget
      */
-    public function addLeft(string $add)
+    public function addLeft(string $add): Budget
     {
         $add        = strval(round($add, 2));
         $this->left = bcadd($this->left, $add);
+        return $this;
     }
 
     /**
      * @param string $add
+     *
+     * @return Budget
      */
-    public function addOverspent(string $add)
+    public function addOverspent(string $add): Budget
     {
         $add             = strval(round($add, 2));
         $this->overspent = bcadd($this->overspent, $add);
+        return $this;
     }
 
     /**
      * @param string $add
+     *
+     * @return Budget
      */
-    public function addSpent(string $add)
+    public function addSpent(string $add): Budget
     {
         $add         = strval(round($add, 2));
         $this->spent = bcadd($this->spent, $add);
+        return $this;
     }
 
     /**
@@ -101,10 +116,14 @@ class Budget
 
     /**
      * @param string $budgeted
+     *
+     * @return Budget
      */
-    public function setBudgeted(string $budgeted)
+    public function setBudgeted(string $budgeted): Budget
     {
         $this->budgeted = $budgeted;
+
+        return $this;
     }
 
     /**
@@ -117,10 +136,14 @@ class Budget
 
     /**
      * @param string $left
+     *
+     * @return Budget
      */
-    public function setLeft(string $left)
+    public function setLeft(string $left): Budget
     {
         $this->left = $left;
+
+        return $this;
     }
 
     /**
@@ -133,10 +156,14 @@ class Budget
 
     /**
      * @param string $overspent
+     *
+     * @return Budget
      */
-    public function setOverspent(string $overspent)
+    public function setOverspent(string $overspent): Budget
     {
         $this->overspent = strval(round($overspent, 2));
+
+        return $this;
     }
 
     /**
@@ -149,10 +176,14 @@ class Budget
 
     /**
      * @param string $spent
+     *
+     * @return Budget
      */
-    public function setSpent(string $spent)
+    public function setSpent(string $spent): Budget
     {
         $this->spent = strval(round($spent, 2));
+
+        return $this;
     }
 
 

app/Helpers/Collection/BudgetLine.php

@@ -45,10 +45,13 @@ class BudgetLine
 
     /**
      * @param BudgetModel $budget
+     *
+     * @return BudgetLine
      */
-    public function setBudget(BudgetModel $budget)
+    public function setBudget(BudgetModel $budget): BudgetLine
     {
         $this->budget = $budget;
+        return $this;
     }
 
     /**
@@ -61,10 +64,13 @@ class BudgetLine
 
     /**
      * @param string $budgeted
+     *
+     * @return BudgetLine
      */
-    public function setBudgeted(string $budgeted)
+    public function setBudgeted(string $budgeted): BudgetLine
     {
         $this->budgeted = $budgeted;
+        return $this;
     }
 
     /**
@@ -77,10 +83,13 @@ class BudgetLine
 
     /**
      * @param string $left
+     *
+     * @return BudgetLine
      */
-    public function setLeft(string $left)
+    public function setLeft(string $left): BudgetLine
     {
         $this->left = $left;
+        return $this;
     }
 
     /**
@@ -93,10 +102,13 @@ class BudgetLine
 
     /**
      * @param string $overspent
+     *
+     * @return BudgetLine
      */
-    public function setOverspent(string $overspent)
+    public function setOverspent(string $overspent): BudgetLine
     {
         $this->overspent = $overspent;
+        return $this;
     }
 
     /**
@@ -109,10 +121,13 @@ class BudgetLine
 
     /**
      * @param LimitRepetition $repetition
+     *
+     * @return BudgetLine
      */
-    public function setRepetition(LimitRepetition $repetition)
+    public function setRepetition(LimitRepetition $repetition): BudgetLine
     {
         $this->repetition = $repetition;
+        return $this;
     }
 
     /**
@@ -125,10 +140,13 @@ class BudgetLine
 
     /**
      * @param string $spent
+     *
+     * @return BudgetLine
      */
-    public function setSpent(string $spent)
+    public function setSpent(string $spent): BudgetLine
     {
         $this->spent = $spent;
+        return $this;
     }
 
 

app/Helpers/Report/BalanceReportHelper.php

@@ -261,7 +261,7 @@ class BalanceReportHelper implements BalanceReportHelperInterface
 
     /**
      * @param Balance $balance
-     *
+     * @SuppressWarnings(PHPMD.CyclomaticComplexity) // it's exactly 5.
      * @return Balance
      */
     private function removeUnusedBudgets(Balance $balance): Balance

app/Helpers/Report/BudgetReportHelper.php

@@ -42,6 +42,9 @@ class BudgetReportHelper implements BudgetReportHelperInterface
     }
 
     /**
+     * @SuppressWarnings(PHPMD.ExcessiveMethodLength) // at 43, its ok.
+     * @SuppressWarnings(PHPMD.CyclomaticComplexity) // it's exactly 5.
+     *
      * @param Carbon     $start
      * @param Carbon     $end
      * @param Collection $accounts
@@ -59,38 +62,22 @@ class BudgetReportHelper implements BudgetReportHelperInterface
         if ($cache->has()) {
             return $cache->get();
         }
-        
+
-        $headers = [];
         $current = clone $start;
         $return  = new Collection;
         $set     = $this->repository->getBudgets();
         $budgets = [];
         $spent   = [];
-        while ($current < $end) {
+        $headers = $this->createYearHeaders($current, $end);
-            $short           = $current->format('m-Y');
-            $headers[$short] = $current->formatLocalized((string)trans('config.month'));
-            $current->addMonth();
-        }
-
 
         /** @var Budget $budget */
         foreach ($set as $budget) {
             $id           = $budget->id;
             $budgets[$id] = $budget->name;
-            $spent[$id]   = [];
             $current      = clone $start;
-            $sum          = '0';
+            $budgetData   = $this->getBudgetSpentData($current, $end, $budget, $accounts);
-
+            $sum          = $budgetData['sum'];
-
+            $spent[$id]   = $budgetData['spent'];
-            while ($current < $end) {
-                $currentEnd = clone $current;
-                $currentEnd->endOfMonth();
-                $format              = $current->format('m-Y');
-                $budgetSpent         = $this->repository->spentInPeriod(new Collection([$budget]), $accounts, $current, $currentEnd);
-                $spent[$id][$format] = $budgetSpent;
-                $sum                 = bcadd($sum, $budgetSpent);
-                $current->addMonth();
-            }
 
             if (bccomp('0', $sum) === 0) {
                 // not spent anything.
@@ -131,10 +118,8 @@ class BudgetReportHelper implements BudgetReportHelperInterface
 
                 if ($spent > 0) {
                     $budgetLine = new BudgetLine;
-                    $budgetLine->setBudget($budget);
+                    $budgetLine->setBudget($budget)->setOverspent($spent);
-                    $budgetLine->setOverspent($spent);
+                    $object->addOverspent($spent)->addBudgetLine($budgetLine);
-                    $object->addOverspent($spent);
-                    $object->addBudgetLine($budgetLine);
                 }
                 continue;
             }
@@ -144,18 +129,12 @@ class BudgetReportHelper implements BudgetReportHelperInterface
                 $data = $this->calculateExpenses($budget, $repetition, $accounts);
 
                 $budgetLine = new BudgetLine;
-                $budgetLine->setBudget($budget);
+                $budgetLine->setBudget($budget)->setRepetition($repetition)
-                $budgetLine->setRepetition($repetition);
+                           ->setLeft($data['left'])->setSpent($data['expenses'])->setOverspent($data['overspent'])
-                $budgetLine->setLeft($data['left']);
+                           ->setBudgeted(strval($repetition->amount));
-                $budgetLine->setSpent($data['expenses']);
-                $budgetLine->setOverspent($data['overspent']);
-                $budgetLine->setBudgeted($repetition->amount);
 
-                $object->addBudgeted($repetition->amount);
+                $object->addBudgeted(strval($repetition->amount))->addSpent($data['spent'])
-                $object->addSpent($data['spent']);
+                       ->addLeft($data['left'])->addOverspent($data['overspent'])->addBudgetLine($budgetLine);
-                $object->addLeft($data['left']);
-                $object->addOverspent($data['overspent']);
-                $object->addBudgetLine($budgetLine);
 
             }
 
@@ -165,10 +144,8 @@ class BudgetReportHelper implements BudgetReportHelperInterface
 
         $noBudget   = $this->repository->spentInPeriodWithoutBudget($accounts, $start, $end);
         $budgetLine = new BudgetLine;
-        $budgetLine->setOverspent($noBudget);
+        $budgetLine->setOverspent($noBudget)->setSpent($noBudget);
-        $budgetLine->setSpent($noBudget);
+        $object->addOverspent($noBudget)->addBudgetLine($budgetLine);
-        $object->addOverspent($noBudget);
-        $object->addBudgetLine($budgetLine);
 
         return $object;
     }
@@ -249,4 +226,50 @@ class BudgetReportHelper implements BudgetReportHelperInterface
         return $array;
 
     }
+
+    /**
+     * @param Carbon $current
+     * @param Carbon $end
+     *
+     * @return array
+     */
+    private function createYearHeaders(Carbon $current, Carbon $end): array
+    {
+        $headers = [];
+        while ($current < $end) {
+            $short           = $current->format('m-Y');
+            $headers[$short] = $current->formatLocalized((string)trans('config.month'));
+            $current->addMonth();
+        }
+
+        return $headers;
+    }
+
+    /**
+     * @param Carbon     $current
+     * @param Carbon     $end
+     * @param Budget     $budget
+     * @param Collection $accounts
+     *
+     * @return array
+     */
+    private function getBudgetSpentData(Carbon $current, Carbon $end, Budget $budget, Collection $accounts): array
+    {
+        $sum   = '0';
+        $spent = [];
+        while ($current < $end) {
+            $currentEnd = clone $current;
+            $currentEnd->endOfMonth();
+            $format         = $current->format('m-Y');
+            $budgetSpent    = $this->repository->spentInPeriod(new Collection([$budget]), $accounts, $current, $currentEnd);
+            $spent[$format] = $budgetSpent;
+            $sum            = bcadd($sum, $budgetSpent);
+            $current->addMonth();
+        }
+
+        return [
+            'spent' => $spent,
+            'sum'   => $sum,
+        ];
+    }
 }

app/Models/ImportJob.php

@@ -45,6 +45,15 @@ use Symfony\Component\HttpKernel\Exception\NotFoundHttpException;
 class ImportJob extends Model
 {
 
+    protected $validStatus
+        = [
+            'import_status_never_started', // initial state
+            'import_configuration_saved', // import configuration saved. This step is going to be obsolete.
+            'settings_complete', // aka: ready for import.
+            'import_running', // import currently underway
+            'import_complete', // done with everything
+        ];
+
     /**
      * @param $value
      *
@@ -139,6 +148,16 @@ class ImportJob extends Model
         $this->attributes['extended_status'] = json_encode($value);
     }
 
+    /**
+     * @param $value
+     */
+    public function setStatusAttribute(string $value)
+    {
+        if (in_array($value, $this->validStatus)) {
+            $this->attributes['status'] = $value;
+        }
+    }
+
     /**
      * @return string
      */