From cabcb9c6d0456a83c0d8d60c1f58db1614a2ad2b Mon Sep 17 00:00:00 2001 From: James Cole Date: Thu, 10 May 2018 09:10:16 +0200 Subject: [PATCH] Basic storage routine works, basic file handling. --- app/Factory/TransactionCurrencyFactory.php | 6 +- app/Factory/TransactionFactory.php | 11 +- app/Factory/TransactionJournalFactory.php | 4 +- .../Import/JobStatusController.php | 4 +- app/Import/Converter/RabobankDebitCredit.php | 6 + .../Prerequisites/FilePrerequisites.php | 81 +-------- app/Import/Storage/ImportArrayStorage.php | 7 +- .../Currency/CurrencyRepository.php | 4 +- .../Currency/CurrencyRepositoryInterface.php | 4 +- .../Support/TransactionServiceTrait.php | 1 + .../Import/Placeholder/ImportTransaction.php | 68 +++++++- .../Import/Routine/File/CSVProcessor.php | 159 ++++++++++++++---- public/js/ff/import/status_v2.js | 23 ++- .../Import/IndexControllerTest.php | 5 +- .../Import/JobStatusControllerTest.php | 44 ++--- tests/Unit/Import/Mapper/BillsTest.php | 8 +- 16 files changed, 278 insertions(+), 157 deletions(-) diff --git a/app/Factory/TransactionCurrencyFactory.php b/app/Factory/TransactionCurrencyFactory.php index 1c95a60e7f..2196cf4594 100644 --- a/app/Factory/TransactionCurrencyFactory.php +++ b/app/Factory/TransactionCurrencyFactory.php @@ -68,7 +68,8 @@ class TransactionCurrencyFactory $currencyCode = (string)$currencyCode; $currencyId = (int)$currencyId; - if (\strlen($currencyCode) === 0 && (int)$currencyId === 0) { + if ('' === $currencyCode && $currencyId === 0) { + Log::warning('Cannot find anything on empty currency code and empty currency ID!'); return null; } @@ -78,6 +79,7 @@ class TransactionCurrencyFactory if (null !== $currency) { return $currency; } + Log::warning(sprintf('Currency ID is %d but found nothing!', $currencyId)); } // then by code: if (\strlen($currencyCode) > 0) { @@ -85,7 +87,9 @@ class TransactionCurrencyFactory if (null !== $currency) { return $currency; } + Log::warning(sprintf('Currency code is %d but found nothing!', $currencyCode)); } + Log::warning('Found nothing for currency.'); return null; } diff --git a/app/Factory/TransactionFactory.php b/app/Factory/TransactionFactory.php index ae8ee25b76..a395793694 100644 --- a/app/Factory/TransactionFactory.php +++ b/app/Factory/TransactionFactory.php @@ -31,6 +31,7 @@ use FireflyIII\Models\TransactionType; use FireflyIII\Services\Internal\Support\TransactionServiceTrait; use FireflyIII\User; use Illuminate\Support\Collection; +use Log; /** * Class TransactionFactory @@ -50,10 +51,17 @@ class TransactionFactory */ public function create(array $data): ?Transaction { - $currencyId = isset($data['currency']) ? $data['currency']->id : $data['currency_id']; + Log::debug('Start of TransactionFactory::create()'); + $currencyId = $data['currency_id'] ?? null; + $currencyId = isset($data['currency']) ? $data['currency']->id : $currencyId; + Log::debug('We dont make it here'); if ('' === $data['amount']) { throw new FireflyException('Amount is an empty string, which Firefly III cannot handle. Apologies.'); } + if (null === $currencyId) { + throw new FireflyException('Cannot store transaction without currency information.'); + } + $data['foreign_amount'] = '' === (string)$data['foreign_amount'] ? null : $data['foreign_amount']; return Transaction::create( [ @@ -81,6 +89,7 @@ class TransactionFactory */ public function createPair(TransactionJournal $journal, array $data): Collection { + Log::debug('Start of TransactionFactory::createPair()'); // all this data is the same for both transactions: $currency = $this->findCurrency($data['currency_id'], $data['currency_code']); $description = $journal->description === $data['description'] ? null : $data['description']; diff --git a/app/Factory/TransactionJournalFactory.php b/app/Factory/TransactionJournalFactory.php index 4afdb35465..816fe9ea58 100644 --- a/app/Factory/TransactionJournalFactory.php +++ b/app/Factory/TransactionJournalFactory.php @@ -70,8 +70,10 @@ class TransactionJournalFactory $factory = app(TransactionFactory::class); $factory->setUser($this->user); + Log::debug(sprintf('Found %d transactions in array.', \count($data['transactions']))); /** @var array $trData */ - foreach ($data['transactions'] as $trData) { + foreach ($data['transactions'] as $index => $trData) { + Log::debug(sprintf('Now storing transaction %d of %d', $index + 1, \count($data['transactions']))); $factory->createPair($journal, $trData); } $journal->completed = true; diff --git a/app/Http/Controllers/Import/JobStatusController.php b/app/Http/Controllers/Import/JobStatusController.php index f45a405b6a..d86faa35c9 100644 --- a/app/Http/Controllers/Import/JobStatusController.php +++ b/app/Http/Controllers/Import/JobStatusController.php @@ -174,11 +174,11 @@ class JobStatusController extends Controller public function store(ImportJob $importJob): JsonResponse { // catch impossible status: - $allowed = ['provider_finished', 'storing_data']; + $allowed = ['provider_finished', 'storing_data','error']; if (null !== $importJob && !\in_array($importJob->status, $allowed, true)) { Log::error('Job is not ready.'); - return response()->json(['status' => 'NOK', 'message' => 'JobStatusController::start expects status "provider_finished".']); + return response()->json(['status' => 'NOK', 'message' => sprintf('JobStatusController::start expects status "provider_finished" instead of "%s".', $importJob->status)]); } // set job to be storing data: diff --git a/app/Import/Converter/RabobankDebitCredit.php b/app/Import/Converter/RabobankDebitCredit.php index 242f4ca38f..d426e52b5b 100644 --- a/app/Import/Converter/RabobankDebitCredit.php +++ b/app/Import/Converter/RabobankDebitCredit.php @@ -43,6 +43,12 @@ class RabobankDebitCredit implements ConverterInterface return -1; } + // old format: + if('A' === $value) { + Log::debug('Return -1'); + + return -1; + } Log::debug('Return 1'); diff --git a/app/Import/Prerequisites/FilePrerequisites.php b/app/Import/Prerequisites/FilePrerequisites.php index 07e236af14..c9200489e3 100644 --- a/app/Import/Prerequisites/FilePrerequisites.php +++ b/app/Import/Prerequisites/FilePrerequisites.php @@ -22,95 +22,16 @@ declare(strict_types=1); namespace FireflyIII\Import\Prerequisites; -use FireflyIII\Exceptions\FireflyException; use FireflyIII\User; -use Illuminate\Http\Request; use Illuminate\Support\MessageBag; /** - * @deprecated * @codeCoverageIgnore + * * This class contains all the routines necessary to import from a file. Hint: there are none. */ class FilePrerequisites implements PrerequisitesInterface { - // /** @var User */ - // private $user; - // - // /** - // * Returns view name that allows user to fill in prerequisites. Currently asks for the API key. - // * - // * @return string - // */ - // public function getView(): string - // { - // return ''; - // } - // - // /** - // * Returns any values required for the prerequisites-view. - // * - // * @return array - // */ - // public function getViewParameters(): array - // { - // return []; - // } - // - // /** - // * Returns if this import method has any special prerequisites such as config - // * variables or other things. The only thing we verify is the presence of the API key. Everything else - // * tumbles into place: no installation token? Will be requested. No device server? Will be created. Etc. - // * - // * True if prerequisites. False if not. - // * - // * @return bool - // * - // * @throws FireflyException - // */ - // public function hasPrerequisites(): bool - // { - // if ($this->user->hasRole('demo')) { - // throw new FireflyException('Apologies, the demo user cannot import files.'); - // } - // - // return false; - // } - // - // /** - // * Indicate if all prerequisites have been met. - // * - // * @return bool - // */ - // public function isComplete(): bool - // { - // // has no prerequisites, so always return true. - // return true; - // } - // - // /** - // * Set the user for this Prerequisites-routine. Class is expected to implement and save this. - // * - // * @param User $user - // */ - // public function setUser(User $user): void - // { - // $this->user = $user; - // - // } - // - // /** - // * This method responds to the user's submission of an API key. It tries to register this instance as a new Firefly III device. - // * If this fails, the error is returned in a message bag and the user is notified (this is fairly friendly). - // * - // * @param Request $request - // * - // * @return MessageBag - // */ - // public function storePrerequisites(Request $request): MessageBag - // { - // return new MessageBag; - // } /** * Returns view name that allows user to fill in prerequisites. * diff --git a/app/Import/Storage/ImportArrayStorage.php b/app/Import/Storage/ImportArrayStorage.php index 35b49cfe80..294dc9c5e5 100644 --- a/app/Import/Storage/ImportArrayStorage.php +++ b/app/Import/Storage/ImportArrayStorage.php @@ -333,8 +333,8 @@ class ImportArrayStorage } $toStore[] = $transaction; } - - if (\count($toStore) === 0) { + $count = \count($toStore); + if ($count === 0) { Log::info('No transactions to store left!'); return new Collection; @@ -343,7 +343,8 @@ class ImportArrayStorage Log::debug('Going to store...'); // now actually store them: $collection = new Collection; - foreach ($toStore as $store) { + foreach ($toStore as $index => $store) { + Log::debug(sprintf('Going to store entry %d of %d', $index + 1, $count)); // convert the date to an object: $store['date'] = Carbon::createFromFormat('Y-m-d', $store['date']); diff --git a/app/Repositories/Currency/CurrencyRepository.php b/app/Repositories/Currency/CurrencyRepository.php index c79aedf8ab..4258432124 100644 --- a/app/Repositories/Currency/CurrencyRepository.php +++ b/app/Repositories/Currency/CurrencyRepository.php @@ -305,9 +305,9 @@ class CurrencyRepository implements CurrencyRepositoryInterface /** * @param array $data * - * @return TransactionCurrency + * @return TransactionCurrency|null */ - public function store(array $data): TransactionCurrency + public function store(array $data): ?TransactionCurrency { /** @var TransactionCurrencyFactory $factory */ $factory = app(TransactionCurrencyFactory::class); diff --git a/app/Repositories/Currency/CurrencyRepositoryInterface.php b/app/Repositories/Currency/CurrencyRepositoryInterface.php index 2462ae8af5..6f1be847d7 100644 --- a/app/Repositories/Currency/CurrencyRepositoryInterface.php +++ b/app/Repositories/Currency/CurrencyRepositoryInterface.php @@ -170,9 +170,9 @@ interface CurrencyRepositoryInterface /** * @param array $data * - * @return TransactionCurrency + * @return TransactionCurrency|null */ - public function store(array $data): TransactionCurrency; + public function store(array $data): ?TransactionCurrency; /** * @param TransactionCurrency $currency diff --git a/app/Services/Internal/Support/TransactionServiceTrait.php b/app/Services/Internal/Support/TransactionServiceTrait.php index 59cb9e6a49..2a0769df52 100644 --- a/app/Services/Internal/Support/TransactionServiceTrait.php +++ b/app/Services/Internal/Support/TransactionServiceTrait.php @@ -247,6 +247,7 @@ trait TransactionServiceTrait */ protected function setForeignAmount(Transaction $transaction, ?string $amount): void { + $amount = '' === (string)$amount ? null : $amount; $transaction->foreign_amount = $amount; $transaction->save(); } diff --git a/app/Support/Import/Placeholder/ImportTransaction.php b/app/Support/Import/Placeholder/ImportTransaction.php index 86b19eb2e9..7b059695bc 100644 --- a/app/Support/Import/Placeholder/ImportTransaction.php +++ b/app/Support/Import/Placeholder/ImportTransaction.php @@ -278,11 +278,15 @@ class ImportTransaction $info = $this->selectAmountInput(); if (0 === \count($info)) { - throw new FireflyException('No amount information for this row.'); + Log::error('No amount information for this row.'); + + return ''; } $class = $info['class'] ?? ''; - if (0 === \strlen($class)) { - throw new FireflyException('No amount information (conversion class) for this row.'); + if ('' === $class) { + Log::error('No amount information (conversion class) for this row.'); + + return ''; } Log::debug(sprintf('Converter class is %s', $info['class'])); @@ -316,6 +320,47 @@ class ImportTransaction return $result; } + /** + * The method that calculates the foreign amount isn't nearly as complex,\ + * because Firefly III only supports one foreign amount field. So the foreign amount is there + * or isn't. That's about it. However, if it's there, modifiers will be applied too. + * + * @return string + */ + public function calculateForeignAmount(): string + { + if (null === $this->foreignAmount) { + Log::debug('ImportTransaction holds no foreign amount info.'); + return ''; + } + /** @var ConverterInterface $amountConverter */ + $amountConverter = app(Amount::class); + $result = $amountConverter->convert($this->foreignAmount); + Log::debug(sprintf('First attempt to convert foreign amount gives "%s"', $result)); + /** + * @var string $role + * @var string $modifier + */ + foreach ($this->modifiers as $role => $modifier) { + $class = sprintf('FireflyIII\Import\Converter\%s', config(sprintf('csv.import_roles.%s.converter', $role))); + /** @var ConverterInterface $converter */ + $converter = app($class); + Log::debug(sprintf('Now launching converter %s', $class)); + $conversion = $converter->convert($modifier); + if ($conversion === -1) { + $result = app('steam')->negative($result); + } + if ($conversion === 1) { + $result = app('steam')->positive($result); + } + Log::debug(sprintf('Foreign amount after conversion is %s', $result)); + } + + Log::debug(sprintf('After modifiers the foreign amount is: "%s"', $result)); + + return $result; + } + /** * This array is being used to map the account the user is using. * @@ -387,6 +432,18 @@ class ImportTransaction return $this->categoryName; } + /** + * @return array + */ + public function getCurrencyData(): array + { + return [ + 'name' => $this->currencyName, + 'code' => $this->currencyCode, + 'symbol' => $this->currencySymbol, + ]; + } + /** * @return int */ @@ -435,6 +492,9 @@ class ImportTransaction return $this->note; } + /** + * @return array + */ public function getOpposingAccountData(): array { return [ @@ -481,7 +541,7 @@ class ImportTransaction * * @return array */ - private function selectAmountInput() + private function selectAmountInput(): array { $info = []; $converterClass = ''; diff --git a/app/Support/Import/Routine/File/CSVProcessor.php b/app/Support/Import/Routine/File/CSVProcessor.php index b7dafd4ab9..f0edce71e2 100644 --- a/app/Support/Import/Routine/File/CSVProcessor.php +++ b/app/Support/Import/Routine/File/CSVProcessor.php @@ -31,6 +31,7 @@ use FireflyIII\Models\Account; use FireflyIII\Models\AccountType; use FireflyIII\Models\Attachment; use FireflyIII\Models\ImportJob; +use FireflyIII\Models\TransactionCurrency; use FireflyIII\Repositories\Account\AccountRepositoryInterface; use FireflyIII\Repositories\Bill\BillRepositoryInterface; use FireflyIII\Repositories\Budget\BudgetRepositoryInterface; @@ -59,6 +60,10 @@ class CSVProcessor implements FileProcessorInterface private $attachments; /** @var array */ private $config; + /** @var CurrencyRepositoryInterface */ + private $currencyRepos; + /** @var TransactionCurrency */ + private $defaultCurrency; /** @var ImportJob */ private $importJob; /** @var array */ @@ -101,13 +106,16 @@ class CSVProcessor implements FileProcessorInterface public function setJob(ImportJob $job): void { Log::debug('Now in setJob()'); - $this->importJob = $job; - $this->config = $job->configuration; - $this->repository = app(ImportJobRepositoryInterface::class); - $this->attachments = app(AttachmentHelperInterface::class); - $this->accountRepos = app(AccountRepositoryInterface::class); + $this->importJob = $job; + $this->config = $job->configuration; + $this->repository = app(ImportJobRepositoryInterface::class); + $this->attachments = app(AttachmentHelperInterface::class); + $this->accountRepos = app(AccountRepositoryInterface::class); + $this->currencyRepos = app(CurrencyRepositoryInterface::class); $this->repository->setUser($job->user); $this->accountRepos->setUser($job->user); + $this->currencyRepos->setUser($job->user); + $this->defaultCurrency = app('amount')->getDefaultCurrencyByUser($job->user); } @@ -237,7 +245,8 @@ class CSVProcessor implements FileProcessorInterface /** * Based upon data in the importable, try to find or create the asset account account. * - * @param $importable + * @param int|null $accountId + * @param array $accountData * * @return Account */ @@ -247,11 +256,16 @@ class CSVProcessor implements FileProcessorInterface if ((int)$accountId > 0) { // find asset account with this ID: $result = $this->accountRepos->findNull($accountId); - if (null !== $result) { - Log::debug(sprintf('Found account "%s" based on given ID %d. Return it!', $result->name, $accountId)); + if (null !== $result && $result->accountType->type === AccountType::ASSET) { + Log::debug(sprintf('Found asset account "%s" based on given ID %d', $result->name, $accountId)); return $result; } + if (null !== $result && $result->accountType->type !== AccountType::ASSET) { + Log::warning( + sprintf('Found account "%s" based on given ID %d but its a %s, return nothing.', $result->name, $accountId, $result->accountType->type) + ); + } } // find by (respectively): // IBAN, accountNumber, name, @@ -264,7 +278,7 @@ class CSVProcessor implements FileProcessorInterface $result = $this->accountRepos->$function($value, [AccountType::ASSET]); Log::debug(sprintf('Going to run %s() with argument "%s" (asset account)', $function, $value)); if (null !== $result) { - Log::debug(sprintf('Found asset account "%s". Return it!', $result->name, $accountId)); + Log::debug(sprintf('Found asset account "%s". Return it!', $result->name)); return $result; } @@ -285,6 +299,49 @@ class CSVProcessor implements FileProcessorInterface return $default; } + /** + * @param int|null $currencyId + * @param array $currencyData + * + * @return TransactionCurrency|null + */ + private function mapCurrency(?int $currencyId, array $currencyData): ?TransactionCurrency + { + if ((int)$currencyId > 0) { + $result = $this->currencyRepos->findNull($currencyId); + if (null !== $result) { + Log::debug(sprintf('Found currency %s based on ID, return it.', $result->code)); + + return $result; + } + } + // try to find it by all other fields. + $fields = ['code' => 'findByCodeNull', 'symbol' => 'findBySymbolNull', 'name' => 'findByNameNull']; + foreach ($fields as $field => $function) { + $value = $currencyData[$field]; + if ('' === (string)$value) { + continue; + } + Log::debug(sprintf('Will search for currency using %s() and argument "%s".', $function, $value)); + $result = $this->currencyRepos->$function($value); + if (null !== $result) { + Log::debug(sprintf('Found result: Currency #%d, code "%s"', $result->id, $result->code)); + + return $result; + } + } + // if still nothing, and fields not null, try to create it + $creation = [ + 'code' => $currencyData['code'], + 'name' => $currencyData['name'], + 'symbol' => $currencyData['symbol'], + 'decimal_places' => 2, + ]; + + // could be NULL + return $this->currencyRepos->store($creation); + } + /** * @param int|null $accountId * @param string $amount @@ -294,18 +351,9 @@ class CSVProcessor implements FileProcessorInterface */ private function mapOpposingAccount(?int $accountId, string $amount, array $accountData): Account { - Log::debug('Now in mapOpposingAccount()'); - if ((int)$accountId > 0) { - // find any account with this ID: - $result = $this->accountRepos->findNull($accountId); - if (null !== $result) { - Log::debug(sprintf('Found account "%s" (%s) based on given ID %d. Return it!', $result->name, $result->accountType->type, $accountId)); - - return $result; - } - } // default assumption is we're looking for an expense account. $expectedType = AccountType::EXPENSE; + $result = null; Log::debug(sprintf('Going to search for accounts of type %s', $expectedType)); if (bccomp($amount, '0') === 1) { // more than zero. @@ -313,6 +361,37 @@ class CSVProcessor implements FileProcessorInterface Log::debug(sprintf('Because amount is %s, will instead search for accounts of type %s', $amount, $expectedType)); } + Log::debug('Now in mapOpposingAccount()'); + if ((int)$accountId > 0) { + // find any account with this ID: + $result = $this->accountRepos->findNull($accountId); + if (null !== $result && $result->accountType->type === $expectedType) { + Log::debug(sprintf('Found account "%s" (%s) based on given ID %d. Return it!', $result->name, $result->accountType->type, $accountId)); + + return $result; + } + if (null !== $result && $result->accountType->type !== $expectedType) { + Log::warning( + sprintf( + 'Found account "%s" (%s) based on given ID %d, but need a %s. Return nothing.', $result->name, $result->accountType->type, $accountId, + $expectedType + ) + ); + } + } + // if result is not null, system has found an account + // but it's of the wrong type. If we dont have a name, use + // the result's name, iban in the search below. + if (null !== $result && '' === (string)$accountData['name']) { + Log::debug(sprintf('Will search for account with name "%s" instead of NULL.', $result->name)); + $accountData['name'] = $result->name; + } + if (null !== $result && '' === $accountData['iban'] && '' !== (string)$result->iban) { + Log::debug(sprintf('Will search for account with IBAN "%s" instead of NULL.', $result->iban)); + $accountData['iban'] = $result->iban; + } + + // first search for $expectedType, then find asset: $searchTypes = [$expectedType, AccountType::ASSET]; foreach ($searchTypes as $type) { @@ -321,10 +400,10 @@ class CSVProcessor implements FileProcessorInterface $fields = ['iban' => 'findByIbanNull', 'number' => 'findByAccountNumber', 'name' => 'findByName']; foreach ($fields as $field => $function) { $value = $accountData[$field]; - if (null === $value) { + if ('' === (string)$value) { continue; } - Log::debug(sprintf('Will search for account of type "%s" using %s() and argument %s.', $type, $function, $value)); + Log::debug(sprintf('Will search for account of type "%s" using %s() and argument "%s".', $type, $function, $value)); $result = $this->accountRepos->$function($value, [$type]); if (null !== $result) { Log::debug(sprintf('Found result: Account #%d, named "%s"', $result->id, $result->name)); @@ -335,7 +414,7 @@ class CSVProcessor implements FileProcessorInterface } // not found? Create it! $creation = [ - 'name' => $accountData['name'], + 'name' => $accountData['name'] ?? '(no name)', 'iban' => $accountData['iban'], 'accountNumber' => $accountData['number'], 'account_type_id' => null, @@ -361,26 +440,40 @@ class CSVProcessor implements FileProcessorInterface { Log::debug('Now in parseImportables()'); $array = []; - $total = count($importables); + $total = \count($importables); /** @var ImportTransaction $importable */ foreach ($importables as $index => $importable) { Log::debug(sprintf('Now going to parse importable %d of %d', $index + 1, $total)); - $array[] = $this->parseSingleImportable($importable); + $result = $this->parseSingleImportable($index, $importable); + if (null !== $result) { + $array[] = $result; + } } return $array; } /** + * @param int $index * @param ImportTransaction $importable * * @return array * @throws FireflyException */ - private function parseSingleImportable(ImportTransaction $importable): array + private function parseSingleImportable(int $index, ImportTransaction $importable): ?array { - $amount = $importable->calculateAmount(); + $amount = $importable->calculateAmount(); + $foreignAmount = $importable->calculateForeignAmount(); + if ('' === $amount) { + $amount = $foreignAmount; + } + if ('' === $amount) { + $this->repository->addErrorMessage($this->importJob, sprintf('No transaction amount information in row %d', $index + 1)); + + return null; + } + /** * first finalise the amount. cehck debit en credit. @@ -397,7 +490,7 @@ class CSVProcessor implements FileProcessorInterface $accountId = $this->verifyObjectId('account-id', $importable->getAccountId()); $billId = $this->verifyObjectId('bill-id', $importable->getForeignCurrencyId()); $budgetId = $this->verifyObjectId('budget-id', $importable->getBudgetId()); - $currencyId = $this->verifyObjectId('currency-id', $importable->getForeignCurrencyId()); + $currencyId = $this->verifyObjectId('currency-id', $importable->getCurrencyId()); $categoryId = $this->verifyObjectId('category-id', $importable->getCategoryId()); $foreignCurrencyId = $this->verifyObjectId('foreign-currency-id', $importable->getForeignCurrencyId()); $opposingId = $this->verifyObjectId('opposing-id', $importable->getOpposingId()); @@ -405,6 +498,12 @@ class CSVProcessor implements FileProcessorInterface //$account = $this->mapAccount($accountId, $importable->getAccountData()); $source = $this->mapAssetAccount($accountId, $importable->getAccountData()); $destination = $this->mapOpposingAccount($opposingId, $amount, $importable->getOpposingAccountData()); + $currency = $this->mapCurrency($currencyId, $importable->getCurrencyData()); + $foreignCurrency = $this->mapCurrency($foreignCurrencyId, $importable->getForeignCurrencyData()); + if (null === $currency) { + Log::debug(sprintf('Could not map currency, use default (%s)', $this->defaultCurrency->code)); + $currency = $this->defaultCurrency; + } if (bccomp($amount, '0') === 1) { // amount is positive? Then switch: @@ -473,7 +572,7 @@ class CSVProcessor implements FileProcessorInterface // transaction data: 'transactions' => [ [ - 'currency_id' => $currencyId, // todo what if null? + 'currency_id' => $currency->id, 'currency_code' => null, 'description' => $importable->getDescription(), 'amount' => $amount, @@ -487,7 +586,7 @@ class CSVProcessor implements FileProcessorInterface 'destination_name' => null, 'foreign_currency_id' => $foreignCurrencyId, 'foreign_currency_code' => null, - 'foreign_amount' => null, // todo get me. + 'foreign_amount' => $foreignAmount, // todo get me. 'reconciled' => false, 'identifier' => 0, ], @@ -537,7 +636,7 @@ class CSVProcessor implements FileProcessorInterface $value = trim($value); $originalRole = $this->config['column-roles'][$column] ?? '_ignore'; Log::debug(sprintf('Now at column #%d (%s), value "%s"', $column, $originalRole, $value)); - if (\strlen($value) > 0 && $originalRole !== '_ignore') { + if ($originalRole !== '_ignore' && \strlen($value) > 0) { // is a mapped value present? $mapped = $this->config['column-mapping-config'][$column][$value] ?? 0; diff --git a/public/js/ff/import/status_v2.js b/public/js/ff/import/status_v2.js index de9a12702c..e99f7061a4 100644 --- a/public/js/ff/import/status_v2.js +++ b/public/js/ff/import/status_v2.js @@ -28,6 +28,7 @@ var checkNextInterval = 500; var maxLoops = 60; var totalLoops = 0; var startCount = 0; +var jobFailed = false; $(function () { "use strict"; @@ -39,7 +40,12 @@ $(function () { */ function checkJobJSONStatus() { console.log('In checkJobJSONStatus()'); - $.getJSON(jobStatusUri).done(reportJobJSONDone).fail(reportJobJSONFailure); + if(jobFailed === false) { + $.getJSON(jobStatusUri).done(reportJobJSONDone).fail(reportJobJSONFailure); + } + if(jobFailed === true) { + console.error('Job has failed, will not check.'); + } } /** @@ -115,12 +121,15 @@ function showJobResults(data) { */ function recheckJobJSONStatus() { console.log('In recheckJobJSONStatus()'); - if (maxLoops !== 0 && totalLoops < maxLoops) { + if (maxLoops !== 0 && totalLoops < maxLoops && jobFailed === false) { timeOutId = setTimeout(checkJobJSONStatus, checkNextInterval); } if (maxLoops !== 0) { console.log('max: ' + maxLoops + ' current: ' + totalLoops); } + if(jobFailed === true) { + console.error('Job has failed, will not do recheck.'); + } totalLoops++; } @@ -133,6 +142,10 @@ function sendJobPOSTStart() { console.log('Import job already started!'); return; } + if(jobFailed === true) { + console.log('Job has failed, will not start again.'); + return; + } console.log('Job was started'); jobRunRoutineStarted = true; $.post(jobStartUri, {_token: token}).fail(reportJobPOSTFailure).done(reportJobPOSTDone) @@ -147,6 +160,10 @@ function sendJobPOSTStore() { console.log('Store job already started!'); return; } + if(jobFailed === true) { + console.log('Job has failed, will not start again.'); + return; + } console.log('Storage job has started!'); jobStorageRoutineStarted = true; $.post(jobStorageStartUri, {_token: token}).fail(reportJobPOSTFailure).done(reportJobPOSTDone) @@ -162,9 +179,11 @@ function sendJobPOSTStore() { */ function reportJobJSONFailure(xhr, status, error) { console.log('In reportJobJSONFailure()'); + jobFailed = true; // cancel checking again for job status: clearTimeout(timeOutId); + // hide status boxes: $('.statusbox').hide(); diff --git a/tests/Feature/Controllers/Import/IndexControllerTest.php b/tests/Feature/Controllers/Import/IndexControllerTest.php index 0632960131..a9e5c2d659 100644 --- a/tests/Feature/Controllers/Import/IndexControllerTest.php +++ b/tests/Feature/Controllers/Import/IndexControllerTest.php @@ -81,7 +81,7 @@ class IndexControllerTest extends TestCase /** * @covers \FireflyIII\Http\Controllers\Import\IndexController */ - public function testCreateFakeNoPrereq() + public function testCreateFakeNoPrereq(): void { // mock stuff: $repository = $this->mock(ImportJobRepositoryInterface::class); @@ -112,18 +112,15 @@ class IndexControllerTest extends TestCase // fake prerequisites providers: $fake = $this->mock(FakePrerequisites::class); - $file = $this->mock(FilePrerequisites::class); $bunq = $this->mock(BunqPrerequisites::class); $spectre = $this->mock(SpectrePrerequisites::class); // call methods: $fake->shouldReceive('setUser')->once(); - $file->shouldReceive('setUser')->once(); $bunq->shouldReceive('setUser')->once(); $spectre->shouldReceive('setUser')->once(); $fake->shouldReceive('isComplete')->once()->andReturn(true); - $file->shouldReceive('isComplete')->once()->andReturn(true); $bunq->shouldReceive('isComplete')->once()->andReturn(true); $spectre->shouldReceive('isComplete')->once()->andReturn(true); diff --git a/tests/Feature/Controllers/Import/JobStatusControllerTest.php b/tests/Feature/Controllers/Import/JobStatusControllerTest.php index 362f878b0c..486100077c 100644 --- a/tests/Feature/Controllers/Import/JobStatusControllerTest.php +++ b/tests/Feature/Controllers/Import/JobStatusControllerTest.php @@ -288,7 +288,7 @@ class JobStatusControllerTest extends TestCase $this->be($this->user()); $response = $this->post(route('import.job.start', [$job->key])); $response->assertStatus(200); - $response->assertExactJson(['status' => 'NOK', 'message' => 'JobStatusController::start expects state "ready_to_run".']); + $response->assertExactJson(['status' => 'NOK', 'message' => 'JobStatusController::start expects status "ready_to_run".']); } /** @@ -322,26 +322,6 @@ class JobStatusControllerTest extends TestCase $response->assertExactJson(['status' => 'OK', 'message' => 'storage_finished']); } - /** - * @covers \FireflyIII\Http\Controllers\Import\JobStatusController - */ - public function testStoreInvalidState(): void - { - $job = new ImportJob; - $job->user_id = $this->user()->id; - $job->key = 'Kfake_job_' . random_int(1, 1000); - $job->status = 'some_bad_state'; - $job->provider = 'fake'; - $job->transactions = []; - $job->file_type = ''; - $job->save(); - - $this->be($this->user()); - $response = $this->post(route('import.job.store', [$job->key])); - $response->assertStatus(200); - $response->assertExactJson(['status' => 'NOK', 'message' => 'JobStatusController::start expects state "provider_finished".']); - } - /** * @covers \FireflyIII\Http\Controllers\Import\JobStatusController */ @@ -372,4 +352,26 @@ class JobStatusControllerTest extends TestCase $response->assertStatus(200); $response->assertExactJson(['status' => 'NOK', 'message' => 'The import storage routine crashed: Some storage exception.']); } + + /** + * @covers \FireflyIII\Http\Controllers\Import\JobStatusController + */ + public function testStoreInvalidState(): void + { + $job = new ImportJob; + $job->user_id = $this->user()->id; + $job->key = 'Kfake_job_' . random_int(1, 1000); + $job->status = 'some_bad_state'; + $job->provider = 'fake'; + $job->transactions = []; + $job->file_type = ''; + $job->save(); + + $this->be($this->user()); + $response = $this->post(route('import.job.store', [$job->key])); + $response->assertStatus(200); + $response->assertExactJson( + ['status' => 'NOK', 'message' => 'JobStatusController::start expects status "provider_finished" instead of "' . $job->status . '".'] + ); + } } diff --git a/tests/Unit/Import/Mapper/BillsTest.php b/tests/Unit/Import/Mapper/BillsTest.php index 355bc43a98..2d25dae93e 100644 --- a/tests/Unit/Import/Mapper/BillsTest.php +++ b/tests/Unit/Import/Mapper/BillsTest.php @@ -40,11 +40,11 @@ class BillsTest extends TestCase */ public function testGetMapBasic() { - $one = new Bill(); + $one = new Bill; $one->id = 5; $one->name = 'Something'; $one->match = 'hi,bye'; - $two = new Account; + $two = new Bill; $two->id = 9; $two->name = 'Else'; $two->match = 'match'; @@ -59,8 +59,8 @@ class BillsTest extends TestCase // assert this is what the result looks like: $result = [ 0 => (string)trans('import.map_do_not_map'), - 9 => 'Else [match]', - 5 => 'Something [hi,bye]', + 9 => 'Else', + 5 => 'Something', ]; $this->assertEquals($result, $mapping); }