Fix various bugs in the import routine, discovered by Doug.

This commit is contained in:
James Cole
2018-06-13 19:03:18 +02:00
parent f4b66b980b
commit 477a3c7eb2
10 changed files with 50 additions and 7 deletions

View File

@@ -26,6 +26,7 @@ namespace FireflyIII\Factory;
use FireflyIII\Exceptions\FireflyException; use FireflyIII\Exceptions\FireflyException;
use FireflyIII\Models\AccountType;
use FireflyIII\Models\Transaction; use FireflyIII\Models\Transaction;
use FireflyIII\Models\TransactionJournal; use FireflyIII\Models\TransactionJournal;
use FireflyIII\Models\TransactionType; use FireflyIII\Models\TransactionType;
@@ -105,6 +106,16 @@ class TransactionFactory
$destinationType = $this->accountType($journal, 'destination'); $destinationType = $this->accountType($journal, 'destination');
Log::debug(sprintf('Expect source destination to be of type %s', $destinationType)); Log::debug(sprintf('Expect source destination to be of type %s', $destinationType));
$destinationAccount = $this->findAccount($destinationType, $data['destination_id'], $data['destination_name']); $destinationAccount = $this->findAccount($destinationType, $data['destination_id'], $data['destination_name']);
Log::debug(sprintf('Source type is "%s", destination type is "%s"', $sourceType, $destinationType));
// throw big fat error when source type === dest type
if ($sourceAccount->accountType->type === $destinationAccount->accountType->type) {
throw new FireflyException(sprintf('Source and destination account cannot be both of the type "%s"', $destinationAccount->accountType->type));
}
if ($sourceAccount->accountType->type !== AccountType::ASSET && $destinationAccount->accountType->type !== AccountType::ASSET) {
throw new FireflyException('At least one of the accounts must be an asset account.');
}
// first make a "negative" (source) transaction based on the data in the array. // first make a "negative" (source) transaction based on the data in the array.
$source = $this->create( $source = $this->create(
[ [

View File

@@ -768,6 +768,14 @@ class JournalCollector implements JournalCollectorInterface
return $this; return $this;
} }
/**
* @return EloquentBuilder
*/
public function getQuery(): EloquentBuilder
{
return $this->query;
}
/** /**
* @param Collection $set * @param Collection $set
* *

View File

@@ -27,6 +27,7 @@ use FireflyIII\Models\Budget;
use FireflyIII\Models\Category; use FireflyIII\Models\Category;
use FireflyIII\Models\Tag; use FireflyIII\Models\Tag;
use FireflyIII\User; use FireflyIII\User;
use Illuminate\Database\Eloquent\Builder as EloquentBuilder;
use Illuminate\Pagination\LengthAwarePaginator; use Illuminate\Pagination\LengthAwarePaginator;
use Illuminate\Support\Collection; use Illuminate\Support\Collection;
@@ -35,6 +36,7 @@ use Illuminate\Support\Collection;
*/ */
interface JournalCollectorInterface interface JournalCollectorInterface
{ {
/** /**
* @param string $filter * @param string $filter
* *
@@ -78,6 +80,11 @@ interface JournalCollectorInterface
*/ */
public function getPaginatedJournals(): LengthAwarePaginator; public function getPaginatedJournals(): LengthAwarePaginator;
/**
* @return EloquentBuilder
*/
public function getQuery(): EloquentBuilder;
/** /**
* @return JournalCollectorInterface * @return JournalCollectorInterface
*/ */

View File

@@ -303,7 +303,7 @@ class ImportArrayStorage
'existing' => $existingId, 'existing' => $existingId,
'description' => $transaction['description'] ?? '', 'description' => $transaction['description'] ?? '',
'amount' => $transaction['transactions'][0]['amount'] ?? 0, 'amount' => $transaction['transactions'][0]['amount'] ?? 0,
'date' => isset($transaction['date']) ? $transaction['date'] : '', 'date' => $transaction['date'] ?? '',
] ]
); );
@@ -413,7 +413,14 @@ class ImportArrayStorage
$store['date'] = Carbon::createFromFormat('Y-m-d', $store['date']); $store['date'] = Carbon::createFromFormat('Y-m-d', $store['date']);
$store['description'] = $store['description'] === '' ? '(empty description)' : $store['description']; $store['description'] = $store['description'] === '' ? '(empty description)' : $store['description'];
// store the journal. // store the journal.
try {
$journal = $this->journalRepos->store($store); $journal = $this->journalRepos->store($store);
} catch(FireflyException $e) {
Log::error($e->getMessage());
Log::error($e->getTraceAsString());
$this->repository->addErrorMessage($this->importJob, sprintf('Row #%d could not be imported. %s', $index, $e->getMessage()));
continue;
}
Log::debug(sprintf('Stored as journal #%d', $journal->id)); Log::debug(sprintf('Stored as journal #%d', $journal->id));
$collection->push($journal); $collection->push($journal);
} }

View File

@@ -24,6 +24,7 @@ namespace FireflyIII\Repositories\Journal;
use Carbon\Carbon; use Carbon\Carbon;
use Exception; use Exception;
use FireflyIII\Exceptions\FireflyException;
use FireflyIII\Factory\TransactionJournalFactory; use FireflyIII\Factory\TransactionJournalFactory;
use FireflyIII\Factory\TransactionJournalMetaFactory; use FireflyIII\Factory\TransactionJournalMetaFactory;
use FireflyIII\Models\Account; use FireflyIII\Models\Account;
@@ -738,8 +739,7 @@ class JournalRepository implements JournalRepositoryInterface
* *
* @return TransactionJournal * @return TransactionJournal
* *
* @throws \FireflyIII\Exceptions\FireflyException * @throws FireflyException
* @throws \FireflyIII\Exceptions\FireflyException
*/ */
public function store(array $data): TransactionJournal public function store(array $data): TransactionJournal
{ {

View File

@@ -23,6 +23,7 @@ declare(strict_types=1);
namespace FireflyIII\Repositories\Journal; namespace FireflyIII\Repositories\Journal;
use Carbon\Carbon; use Carbon\Carbon;
use FireflyIII\Exceptions\FireflyException;
use FireflyIII\Models\Account; use FireflyIII\Models\Account;
use FireflyIII\Models\Note; use FireflyIII\Models\Note;
use FireflyIII\Models\Transaction; use FireflyIII\Models\Transaction;
@@ -325,7 +326,7 @@ interface JournalRepositoryInterface
/** /**
* @param array $data * @param array $data
* * @throws FireflyException
* @return TransactionJournal * @return TransactionJournal
*/ */
public function store(array $data): TransactionJournal; public function store(array $data): TransactionJournal;

View File

@@ -190,6 +190,7 @@ trait TransactionServiceTrait
*/ */
protected function findCategory(?int $categoryId, ?string $categoryName): ?Category protected function findCategory(?int $categoryId, ?string $categoryName): ?Category
{ {
Log::debug(sprintf('Going to find or create category #%d, with name "%s"', $categoryId, $categoryName));
/** @var CategoryFactory $factory */ /** @var CategoryFactory $factory */
$factory = app(CategoryFactory::class); $factory = app(CategoryFactory::class);
$factory->setUser($this->user); $factory->setUser($this->user);

View File

@@ -179,7 +179,9 @@ class ImportTransaction
$this->budgetName = $columnValue->getValue(); $this->budgetName = $columnValue->getValue();
break; break;
case 'category-id': case 'category-id':
$this->categoryId = $this->getMappedValue($columnValue); $value = $this->getMappedValue($columnValue);
Log::debug(sprintf('Set category ID to %d in ImportTransaction object', $value));
$this->categoryId = $value;
break; break;
case 'category-name': case 'category-name':
$this->categoryName = $columnValue->getValue(); $this->categoryName = $columnValue->getValue();

View File

@@ -279,11 +279,14 @@ class ImportableConverter
*/ */
private function verifyObjectId(string $key, int $objectId): ?int private function verifyObjectId(string $key, int $objectId): ?int
{ {
if (isset($this->mappedValues[$key]) && \in_array($objectId, $this->mappedValues[$key], true)) { if (isset($this->mappedValues[$key]) && \in_array($objectId, $this->mappedValues[$key], true)) {
Log::debug(sprintf('verifyObjectId(%s, %d) is valid!',$key, $objectId));
return $objectId; return $objectId;
} }
return null; Log::debug(sprintf('verifyObjectId(%s, %d) is NOT in the list, but it could still be valid.',$key, $objectId));
return $objectId;
} }

View File

@@ -87,6 +87,7 @@ class MappedValuesValidator
$return = []; $return = [];
Log::debug('Now in validateMappedValues()'); Log::debug('Now in validateMappedValues()');
foreach ($mappings as $role => $values) { foreach ($mappings as $role => $values) {
Log::debug(sprintf('Now at role "%s"', $role));
$values = array_unique($values); $values = array_unique($values);
if (\count($values) > 0) { if (\count($values) > 0) {
switch ($role) { switch ($role) {
@@ -115,9 +116,11 @@ class MappedValuesValidator
$return[$role] = $valid; $return[$role] = $valid;
break; break;
case 'category-id': case 'category-id':
Log::debug('Going to validate these category ids: ', $values);
$set = $this->catRepos->getByIds($values); $set = $this->catRepos->getByIds($values);
$valid = $set->pluck('id')->toArray(); $valid = $set->pluck('id')->toArray();
$return[$role] = $valid; $return[$role] = $valid;
Log::debug('Valid category IDs are: ', $valid);
break; break;
} }
} }