From 796ab4bf2c4c7b067586346a44d6383004e16706 Mon Sep 17 00:00:00 2001
From: James Cole <thegrumpydictator@gmail.com>
Date: Sat, 24 Mar 2018 11:49:26 +0100
Subject: [PATCH] Fixed some small issues in import routine.

---
 app/Import/Object/ImportBudget.php              |  2 +-
 app/Import/Object/ImportCategory.php            |  2 +-
 app/Import/Storage/ImportSupport.php            |  6 +++---
 app/Support/Import/Configuration/File/Roles.php | 17 ++++++++++++++++-
 4 files changed, 21 insertions(+), 6 deletions(-)

diff --git a/app/Import/Object/ImportBudget.php b/app/Import/Object/ImportBudget.php
index d1bc96aabd..18b17a91e0 100644
--- a/app/Import/Object/ImportBudget.php
+++ b/app/Import/Object/ImportBudget.php
@@ -171,7 +171,7 @@ class ImportBudget
             Log::debug(sprintf('Find mapped budget based on field "%s" with value', $field), $array);
             // check if a pre-mapped object exists.
             $mapped = $this->getMappedObject($array);
-            if (null !== $mapped->id) {
+            if (null !== $mapped) {
                 Log::debug(sprintf('Found budget #%d!', $mapped->id));
 
                 return $mapped;
diff --git a/app/Import/Object/ImportCategory.php b/app/Import/Object/ImportCategory.php
index 7845fd878b..e7f21db03f 100644
--- a/app/Import/Object/ImportCategory.php
+++ b/app/Import/Object/ImportCategory.php
@@ -192,7 +192,7 @@ class ImportCategory
      *
      * @return Category
      */
-    private function getMappedObject(array $array): Category
+    private function getMappedObject(array $array): ?Category
     {
         Log::debug('In getMappedObject() for Category');
         if (0 === count($array)) {
diff --git a/app/Import/Storage/ImportSupport.php b/app/Import/Storage/ImportSupport.php
index ed92fbb82c..9e3be430fd 100644
--- a/app/Import/Storage/ImportSupport.php
+++ b/app/Import/Storage/ImportSupport.php
@@ -386,7 +386,7 @@ trait ImportSupport
      */
     private function storeBill(TransactionJournal $journal, Bill $bill)
     {
-        if (null !== $bill->id) {
+        if (null !== $bill) {
             Log::debug(sprintf('Linked bill #%d to journal #%d', $bill->id, $journal->id));
             $journal->bill()->associate($bill);
             $journal->save();
@@ -399,7 +399,7 @@ trait ImportSupport
      */
     private function storeBudget(TransactionJournal $journal, Budget $budget)
     {
-        if (null !== $budget->id) {
+        if (null !== $budget) {
             Log::debug(sprintf('Linked budget #%d to journal #%d', $budget->id, $journal->id));
             $journal->budgets()->save($budget);
         }
@@ -411,7 +411,7 @@ trait ImportSupport
      */
     private function storeCategory(TransactionJournal $journal, Category $category)
     {
-        if (null !== $category->id) {
+        if (null !== $category) {
             Log::debug(sprintf('Linked category #%d to journal #%d', $category->id, $journal->id));
             $journal->categories()->save($category);
         }
diff --git a/app/Support/Import/Configuration/File/Roles.php b/app/Support/Import/Configuration/File/Roles.php
index c5be1c3a6e..3b33805840 100644
--- a/app/Support/Import/Configuration/File/Roles.php
+++ b/app/Support/Import/Configuration/File/Roles.php
@@ -184,6 +184,7 @@ class Roles implements ConfigurationInterface
      */
     private function ignoreUnmappableColumns(): bool
     {
+        Log::debug('Now in ignoreUnmappableColumns()');
         $config = $this->getConfig();
         $count  = $config['column-count'];
         for ($i = 0; $i < $count; ++$i) {
@@ -254,24 +255,38 @@ class Roles implements ConfigurationInterface
                 $hasForeignAmount = true;
             }
         }
-        if ($assigned > 0 && $hasAmount && ($hasForeignCode === false && $hasForeignAmount === false)) {
+        Log::debug(
+            sprintf(
+                'Assigned is %d, hasAmount %s, hasForeignCode %s, hasForeignAmount %s',
+                $assigned,
+                var_export($hasAmount, true),
+                var_export($hasForeignCode, true),
+                var_export($hasForeignAmount, true)
+            )
+        );
+        // all assigned and correct foreign info
+        if ($assigned > 0 && $hasAmount && ($hasForeignCode === $hasForeignAmount)) {
             $this->warning = '';
             $this->saveConfig($config);
+            Log::debug('isRolesComplete() returns true.');
 
             return true;
         }
         // warn if has foreign amount but no currency code:
         if ($hasForeignAmount && !$hasForeignCode) {
             $this->warning = strval(trans('import.foreign_amount_warning'));
+            Log::debug('isRolesComplete() returns FALSE because foreign amount present without foreign code.');
 
             return false;
         }
 
         if (0 === $assigned || !$hasAmount) {
             $this->warning = strval(trans('import.roles_warning'));
+            Log::debug('isRolesComplete() returns FALSE because no amount present.');
 
             return false;
         }
+        Log::debug('isRolesComplete() returns FALSE because no reason.');
 
         return false;
     }