From 3c4abb7b60be6b8ec42200749c3581d544e16687 Mon Sep 17 00:00:00 2001 From: James Cole Date: Fri, 14 Jul 2017 06:41:47 +0200 Subject: [PATCH] Fixed some issues in import, should improve results for #701 --- app/Import/Object/ImportAccount.php | 6 +- app/Import/Object/ImportJournal.php | 2 +- app/Import/Storage/ImportStorage.php | 88 ++++++++++++++++++++ app/Support/CacheProperties.php | 2 - app/Support/Import/Configuration/Csv/Map.php | 2 + app/Support/Steam.php | 16 ++++ 6 files changed, 111 insertions(+), 5 deletions(-) diff --git a/app/Import/Object/ImportAccount.php b/app/Import/Object/ImportAccount.php index 8ab5c77077..ca5dba85d5 100644 --- a/app/Import/Object/ImportAccount.php +++ b/app/Import/Object/ImportAccount.php @@ -264,8 +264,10 @@ class ImportAccount return new Account; } - - if ($account->accountType->type !== $this->expectedType) { + // must be of the same type + // except when mapped is an asset, then it's fair game. + // which only shows that user must map very carefully. + if ($account->accountType->type !== $this->expectedType && $account->accountType->type !== AccountType::ASSET) { Log::error( sprintf( 'Mapped account #%d is of type "%s" but we expect a "%s"-account. Mapping will be ignored.', $account->id, $account->accountType->type, diff --git a/app/Import/Object/ImportJournal.php b/app/Import/Object/ImportJournal.php index adebaea1f6..e5846f6b6f 100644 --- a/app/Import/Object/ImportJournal.php +++ b/app/Import/Object/ImportJournal.php @@ -226,7 +226,7 @@ class ImportJournal $this->date = $array['value']; break; case 'description': - $this->description = $array['value']; + $this->description .= $array['value']; break; case 'sepa-ct-op': case 'sepa-ct-id': diff --git a/app/Import/Storage/ImportStorage.php b/app/Import/Storage/ImportStorage.php index 8bdb6685fe..766538bbf9 100644 --- a/app/Import/Storage/ImportStorage.php +++ b/app/Import/Storage/ImportStorage.php @@ -29,6 +29,7 @@ use FireflyIII\Models\TransactionJournal; use FireflyIII\Models\TransactionType; use FireflyIII\Repositories\Currency\CurrencyRepositoryInterface; use FireflyIII\Rules\Processor; +use Illuminate\Database\Query\JoinClause; use Illuminate\Support\Collection; use Log; use Steam; @@ -323,6 +324,15 @@ class ImportStorage /*** First step done! */ $this->job->addStepsDone(1); + // could be that transfer is double: verify this. + if ($this->verifyDoubleTransfer($transactionType, $importJournal)) { + // add three steps: + $this->job->addStepsDone(3); + // throw error + throw new FireflyException('Detected a possible duplicate, skip this one.'); + + } + // create a journal: $journal = new TransactionJournal; $journal->user_id = $this->job->user_id; @@ -336,7 +346,9 @@ class ImportStorage if (!$journal->save()) { $errorText = join(', ', $journal->getErrors()->all()); + // add three steps: $this->job->addStepsDone(3); + // throw error throw new FireflyException($errorText); } @@ -402,4 +414,80 @@ class ImportStorage } } + /** + * This method checks if the given transaction is a transfer and if so, if it might be a duplicate of an already imported transfer. + * This is important for import files that cover multiple accounts (and include both A<>B and B<>A transactions). + * + * @param TransactionType $transactionType + * @param ImportJournal $importJournal + * + * @return bool + */ + private function verifyDoubleTransfer(TransactionType $transactionType, ImportJournal $importJournal): bool + { + if ($transactionType->type === TransactionType::TRANSFER) { + $amount = Steam::positive($importJournal->getAmount()); + $asset = $importJournal->asset->getAccount(); + $opposing = $this->getOpposingAccount($importJournal->opposing, $amount); + $date = $importJournal->getDate($this->dateFormat); + $description = $importJournal->description; + $set = TransactionJournal:: + leftJoin('transaction_types', 'transaction_types.id', '=', 'transaction_journals.transaction_type_id') + ->leftJoin( + 'transactions AS source', function (JoinClause $join) { + $join->on('transaction_journals.id', '=', 'source.transaction_journal_id')->where('source.amount', '<', 0); + } + ) + ->leftJoin( + 'transactions AS destination', function (JoinClause $join) { + $join->on('transaction_journals.id', '=', 'destination.transaction_journal_id')->where( + 'destination.amount', '>', 0 + ); + } + ) + ->leftJoin('accounts as source_accounts', 'source.account_id', '=', 'source_accounts.id') + ->leftJoin('accounts as destination_accounts', 'destination.account_id', '=', 'destination_accounts.id') + ->where('transaction_journals.user_id', $this->job->user_id) + ->where('transaction_types.type', TransactionType::TRANSFER) + ->where('transaction_journals.date', $date->format('Y-m-d')) + ->where('destination.amount', $amount) + ->get( + ['transaction_journals.id', 'transaction_journals.encrypted', 'transaction_journals.description', + 'source_accounts.name as source_name', 'destination_accounts.name as destination_name'] + ); + if ($set->count() > 0) { + $filtered = $set->filter( + function (TransactionJournal $journal) use ($asset, $opposing, $description) { + $match = true; + $compare = [$asset->name, $opposing->name]; + + if ($journal->description !== $description) { + $match = false; + } + // when both are in array match is true. So reverse: + if (!(in_array(app('steam')->tryDecrypt($journal->source_name), $compare) + && in_array( + app('steam')->tryDecrypt($journal->destination_name), $compare + )) + ) { + $match = false; + } + + if ($match) { + return $journal; + } + + return null; + } + ); + if (count($filtered) > 0) { + return true; + } + } + } + + + return false; + } + } diff --git a/app/Support/CacheProperties.php b/app/Support/CacheProperties.php index 555fe427b0..f4cf7c0984 100644 --- a/app/Support/CacheProperties.php +++ b/app/Support/CacheProperties.php @@ -119,8 +119,6 @@ class CacheProperties $this->md5 .= json_encode($property); } - Log::debug(sprintf('Cache string is %s', $this->md5)); $this->md5 = md5($this->md5); - Log::debug(sprintf('Cache MD5 is %s', $this->md5)); } } diff --git a/app/Support/Import/Configuration/Csv/Map.php b/app/Support/Import/Configuration/Csv/Map.php index 813cecd071..f9cae51a0c 100644 --- a/app/Support/Import/Configuration/Csv/Map.php +++ b/app/Support/Import/Configuration/Csv/Map.php @@ -92,7 +92,9 @@ class Map implements ConfigurationInterface } foreach ($this->data as $index => $entry) { $this->data[$index]['values'] = array_unique($this->data[$index]['values']); + asort($this->data[$index]['values']); } + // save number of rows, thus number of steps, in job: $steps = $rowIndex * 5; $extended = $this->job->extended_status; diff --git a/app/Support/Steam.php b/app/Support/Steam.php index 8a3ca9c885..7b08463261 100644 --- a/app/Support/Steam.php +++ b/app/Support/Steam.php @@ -19,6 +19,7 @@ use Crypt; use DB; use FireflyIII\Models\Account; use FireflyIII\Models\Transaction; +use Illuminate\Contracts\Encryption\DecryptException; use Illuminate\Support\Collection; /** @@ -248,6 +249,21 @@ class Steam return $value; } + /** + * @param $value + * + * @return mixed + */ + public function tryDecrypt($value) + { + try { + $value = Crypt::decrypt($value); + } catch (DecryptException $e) { + // do not care. + } + + return $value; + } /** * @param array $accounts