diff --git a/app/Import/Configuration/BunqConfigurator.php b/app/Import/Configuration/BunqConfigurator.php index 27604e46b5..e52455401c 100644 --- a/app/Import/Configuration/BunqConfigurator.php +++ b/app/Import/Configuration/BunqConfigurator.php @@ -25,7 +25,6 @@ namespace FireflyIII\Import\Configuration; use FireflyIII\Exceptions\FireflyException; use FireflyIII\Models\ImportJob; use FireflyIII\Repositories\ImportJob\ImportJobRepositoryInterface; -use FireflyIII\Support\Import\Configuration\Bunq\HasAccounts; use FireflyIII\Support\Import\Configuration\Bunq\HaveAccounts; use Log; @@ -56,6 +55,7 @@ class BunqConfigurator implements ConfiguratorInterface * @param array $data * * @return bool + * * @throws FireflyException */ public function configureJob(array $data): bool @@ -89,6 +89,7 @@ class BunqConfigurator implements ConfiguratorInterface * Return the data required for the next step in the job configuration. * * @return array + * * @throws FireflyException */ public function getNextData(): array @@ -116,6 +117,7 @@ class BunqConfigurator implements ConfiguratorInterface /** * @return string + * * @throws FireflyException */ public function getNextView(): string @@ -131,7 +133,6 @@ class BunqConfigurator implements ConfiguratorInterface return 'import.bunq.accounts'; default: return ''; - } } @@ -147,6 +148,7 @@ class BunqConfigurator implements ConfiguratorInterface /** * @return bool + * * @throws FireflyException */ public function isJobConfigured(): bool diff --git a/app/Import/Configuration/FileConfigurator.php b/app/Import/Configuration/FileConfigurator.php index c5070e5f73..38e1113d01 100644 --- a/app/Import/Configuration/FileConfigurator.php +++ b/app/Import/Configuration/FileConfigurator.php @@ -61,7 +61,6 @@ class FileConfigurator implements ConfiguratorInterface /** @var ImportJobRepositoryInterface */ private $repository; - // give job default config: /** @var string */ private $warning = ''; @@ -145,6 +144,7 @@ class FileConfigurator implements ConfiguratorInterface * Return possible warning to user. * * @return string + * * @throws FireflyException */ public function getWarningMessage(): string @@ -158,6 +158,7 @@ class FileConfigurator implements ConfiguratorInterface /** * @return bool + * * @throws FireflyException */ public function isJobConfigured(): bool @@ -167,7 +168,7 @@ class FileConfigurator implements ConfiguratorInterface } $config = $this->getConfig(); $stage = $config['stage'] ?? 'initial'; - if ($stage === 'ready') { + if ('ready' === $stage) { Log::debug('isJobConfigured returns true'); return true; @@ -252,6 +253,7 @@ class FileConfigurator implements ConfiguratorInterface * Shorthand method to return the extended status. * * @codeCoverageIgnore + * * @return array */ private function getExtendedStatus(): array diff --git a/app/Import/Configuration/SpectreConfigurator.php b/app/Import/Configuration/SpectreConfigurator.php index d3ed6d5b87..0c03c5adb9 100644 --- a/app/Import/Configuration/SpectreConfigurator.php +++ b/app/Import/Configuration/SpectreConfigurator.php @@ -55,6 +55,7 @@ class SpectreConfigurator implements ConfiguratorInterface * @param array $data * * @return bool + * * @throws FireflyException */ public function configureJob(array $data): bool @@ -87,6 +88,7 @@ class SpectreConfigurator implements ConfiguratorInterface * Return the data required for the next step in the job configuration. * * @return array + * * @throws FireflyException */ public function getNextData(): array @@ -124,6 +126,7 @@ class SpectreConfigurator implements ConfiguratorInterface /** * @return string + * * @throws FireflyException */ public function getNextView(): string @@ -143,7 +146,6 @@ class SpectreConfigurator implements ConfiguratorInterface return 'import.spectre.accounts'; default: return ''; - } } @@ -159,6 +161,7 @@ class SpectreConfigurator implements ConfiguratorInterface /** * @return bool + * * @throws FireflyException */ public function isJobConfigured(): bool diff --git a/app/Import/Converter/Amount.php b/app/Import/Converter/Amount.php index ccf7f6f76c..ee13f8c565 100644 --- a/app/Import/Converter/Amount.php +++ b/app/Import/Converter/Amount.php @@ -113,13 +113,12 @@ class Amount implements ConverterInterface { $str = preg_replace('/[^\-\(\)\.\,0-9 ]/', '', $value); $len = strlen($str); - if ($str{0} === '(' && $str{$len - 1} === ')') { + if ('(' === $str[0] && ')' === $str[$len - 1]) { $str = '-' . substr($str, 1, ($len - 2)); } Log::debug(sprintf('Stripped "%s" away to "%s"', $value, $str)); return $str; - } } diff --git a/app/Import/FileProcessor/CsvProcessor.php b/app/Import/FileProcessor/CsvProcessor.php index fa4605e1db..6201a96f23 100644 --- a/app/Import/FileProcessor/CsvProcessor.php +++ b/app/Import/FileProcessor/CsvProcessor.php @@ -62,6 +62,7 @@ class CsvProcessor implements FileProcessorInterface /** * @return Collection + * * @throws FireflyException */ public function getObjects(): Collection @@ -192,6 +193,7 @@ class CsvProcessor implements FileProcessorInterface * Shorthand method to return configuration. * * @codeCoverageIgnore + * * @return array */ private function getConfig(): array @@ -237,6 +239,7 @@ class CsvProcessor implements FileProcessorInterface * @param int $jsonError * * @codeCoverageIgnore + * * @return string */ private function getJsonError(int $jsonError): string diff --git a/app/Import/Object/ImportAccount.php b/app/Import/Object/ImportAccount.php index ee7d2257ad..638cdfd83a 100644 --- a/app/Import/Object/ImportAccount.php +++ b/app/Import/Object/ImportAccount.php @@ -27,7 +27,6 @@ use FireflyIII\Models\Account; use FireflyIII\Models\AccountType; use FireflyIII\Repositories\Account\AccountRepositoryInterface; use FireflyIII\User; -use Illuminate\Support\Collection; use Log; /** @@ -70,18 +69,18 @@ class ImportAccount public function __construct() { $this->expectedType = AccountType::ASSET; - $this->account = new Account; $this->repository = app(AccountRepositoryInterface::class); Log::debug('Created ImportAccount.'); } /** * @return Account + * * @throws FireflyException */ public function getAccount(): Account { - if (null === $this->account->id) { + if (null === $this->account) { $this->store(); } @@ -90,6 +89,7 @@ class ImportAccount /** * @codeCoverageIgnore + * * @return string */ public function getExpectedType(): string @@ -187,43 +187,26 @@ class ImportAccount } /** + * Find account by IBAN and type. + * + * @param AccountType $type + * * @return Account|null - * @SuppressWarnings(PHPMD.CyclomaticComplexity) */ - private function findExistingObject(): ?Account + private function findByIBAN(AccountType $type): ?Account { - Log::debug('In findExistingObject() for Account'); - // 0: determin account type: - /** @var AccountType $accountType */ - $accountType = $this->repository->getAccountType($this->expectedType); - - // 1: find by ID, iban or name (and type) - if (3 === count($this->accountId)) { - Log::debug(sprintf('Finding account of type %d and ID %d', $accountType->id, $this->accountId['value'])); - /** @var Account $account */ - - $account = $this->user->accounts()->where('id', '!=', $this->forbiddenAccountId)->where('account_type_id', $accountType->id)->where( - 'id', - $this->accountId['value'] - )->first(); - if (null !== $account) { - Log::debug(sprintf('Found unmapped %s account by ID (#%d): %s', $this->expectedType, $account->id, $account->name)); - - return $account; - } - Log::debug('Found nothing.'); - } - /** @var Collection $accounts */ - $accounts = $this->repository->getAccountsByType([$accountType->type]); - // Two: find by IBAN (and type): if (3 === count($this->accountIban)) { - $iban = $this->accountIban['value']; - Log::debug(sprintf('Finding account of type %d and IBAN %s', $accountType->id, $iban)); + $accounts = $this->repository->getAccountsByType([$type->type]); + $iban = $this->accountIban['value']; + Log::debug(sprintf('Finding account of type %d and IBAN %s', $type->id, $iban)); $filtered = $accounts->filter( function (Account $account) use ($iban) { if ($account->iban === $iban && $account->id !== $this->forbiddenAccountId) { Log::debug( - sprintf('Found unmapped %s account by IBAN (#%d): %s (%s)', $this->expectedType, $account->id, $account->name, $account->iban) + sprintf( + 'Found unmapped %s account by IBAN (#%d): %s (%s)', + $this->expectedType, $account->id, $account->name, $account->iban + ) ); return $account; @@ -238,10 +221,52 @@ class ImportAccount Log::debug('Found nothing.'); } + return null; + } + + /** + * Find account of type X by its ID. + * + * @param AccountType $type + * + * @return Account|null + */ + private function findById(AccountType $type): ?Account + { + if (3 === count($this->accountId)) { + Log::debug(sprintf('Finding account of type %d and ID %d', $type->id, $this->accountId['value'])); + /** @var Account $account */ + $account = $this->user->accounts() + ->where('id', '!=', $this->forbiddenAccountId) + ->where('account_type_id', $type->id) + ->where('id', $this->accountId['value']) + ->first(); + + if (null !== $account) { + Log::debug(sprintf('Found unmapped %s account by ID (#%d): %s', $this->expectedType, $account->id, $account->name)); + + return $account; + } + Log::debug('Found nothing.'); + } + + return null; + } + + /** + * Find account by account type and name. + * + * @param AccountType $type + * + * @return Account|null + */ + private function findByName(AccountType $type): ?Account + { // Three: find by name (and type): if (3 === count($this->accountName)) { - $name = $this->accountName['value']; - Log::debug(sprintf('Finding account of type %d and name %s', $accountType->id, $name)); + $accounts = $this->repository->getAccountsByType([$type->type]); + $name = $this->accountName['value']; + Log::debug(sprintf('Finding account of type %d and name %s', $type->id, $name)); $filtered = $accounts->filter( function (Account $account) use ($name) { if ($account->name === $name && $account->id !== $this->forbiddenAccountId) { @@ -260,7 +285,36 @@ class ImportAccount Log::debug('Found nothing.'); } - // 4: do not search by account number. + return null; + } + + /** + * Determin account type to find, then use fields in object to try and find it. + * + * @return Account|null + */ + private function findExistingObject(): ?Account + { + Log::debug('In findExistingObject() for Account'); + /** @var AccountType $accountType */ + $accountType = $this->repository->getAccountType($this->expectedType); + $result = $this->findById($accountType); + if (!is_null($result)) { + return $result; + } + + $result = $this->findByIBAN($accountType); + + if (!is_null($result)) { + return $result; + } + + $result = $this->findByName($accountType); + + if (!is_null($result)) { + return $result; + } + Log::debug('Found NO existing accounts.'); return null; @@ -342,6 +396,7 @@ class ImportAccount /** * @return bool + * * @throws FireflyException */ private function store(): bool @@ -349,7 +404,7 @@ class ImportAccount if (is_null($this->user)) { throw new FireflyException('ImportAccount cannot continue without user.'); } - if ((is_null($this->defaultAccountId) || intval($this->defaultAccountId) === 0) && AccountType::ASSET === $this->expectedType) { + if ((is_null($this->defaultAccountId) || 0 === intval($this->defaultAccountId)) && AccountType::ASSET === $this->expectedType) { throw new FireflyException('ImportAccount cannot continue without a default account to fall back on.'); } // 1: find mapped object: diff --git a/app/Import/Object/ImportBill.php b/app/Import/Object/ImportBill.php index f27eaaf070..32061504a2 100644 --- a/app/Import/Object/ImportBill.php +++ b/app/Import/Object/ImportBill.php @@ -25,7 +25,6 @@ namespace FireflyIII\Import\Object; use FireflyIII\Models\Bill; use FireflyIII\Repositories\Bill\BillRepositoryInterface; use FireflyIII\User; -use Illuminate\Support\Collection; use Log; use Steam; @@ -52,7 +51,6 @@ class ImportBill */ public function __construct() { - $this->bill = new Bill; $this->repository = app(BillRepositoryInterface::class); Log::debug('Created ImportBill.'); } @@ -62,7 +60,7 @@ class ImportBill */ public function getBill(): Bill { - if (null === $this->bill->id) { + if (null === $this->bill) { $this->store(); } @@ -103,13 +101,10 @@ class ImportBill } /** - * @return Bill + * @return Bill|null */ - private function findExistingObject(): Bill + private function findById(): ?Bill { - Log::debug('In findExistingObject() for Bill'); - // 1: find by ID, or name - if (3 === count($this->id)) { Log::debug(sprintf('Finding bill with ID #%d', $this->id['value'])); /** @var Bill $bill */ @@ -121,9 +116,16 @@ class ImportBill } Log::debug('Found nothing.'); } - // 2: find by name + + return null; + } + + /** + * @return Bill|null + */ + private function findByName(): ?Bill + { if (3 === count($this->name)) { - /** @var Collection $bills */ $bills = $this->repository->getBills(); $name = $this->name['value']; Log::debug(sprintf('Finding bill with name %s', $name)); @@ -145,16 +147,33 @@ class ImportBill Log::debug('Found nothing.'); } - // 4: do not search by account number. - Log::debug('Found NO existing bills.'); - - return new Bill; + return null; } /** - * @return Bill + * @return Bill|null */ - private function findMappedObject(): Bill + private function findExistingObject(): ?Bill + { + Log::debug('In findExistingObject() for Bill'); + $result = $this->findById(); + if (!is_null($result)) { + return $result; + } + $result = $this->findByName(); + if (!is_null($result)) { + return $result; + } + + Log::debug('Found NO existing bills.'); + + return null; + } + + /** + * @return Bill|null + */ + private function findMappedObject(): ?Bill { Log::debug('In findMappedObject() for Bill'); $fields = ['id', 'name']; @@ -163,7 +182,7 @@ class ImportBill Log::debug(sprintf('Find mapped bill 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 bill #%d!', $mapped->id)); return $mapped; @@ -171,7 +190,7 @@ class ImportBill } Log::debug('Found no bill on mapped data or no map present.'); - return new Bill; + return null; } /** @@ -179,19 +198,19 @@ class ImportBill * * @return Bill */ - private function getMappedObject(array $array): Bill + private function getMappedObject(array $array): ?Bill { Log::debug('In getMappedObject() for Bill'); if (0 === count($array)) { Log::debug('Array is empty, nothing will come of this.'); - return new Bill; + return null; } if (array_key_exists('mapped', $array) && null === $array['mapped']) { Log::debug(sprintf('No map present for value "%s". Return NULL.', $array['value'])); - return new Bill; + return null; } Log::debug('Finding a mapped bill based on', $array); @@ -202,7 +221,7 @@ class ImportBill if (null === $bill) { Log::error(sprintf('There is no bill with id #%d. Invalid mapping will be ignored!', $search)); - return new Bill; + return null; } Log::debug(sprintf('Found bill! #%d ("%s"). Return it', $bill->id, $bill->name)); @@ -217,14 +236,14 @@ class ImportBill { // 1: find mapped object: $mapped = $this->findMappedObject(); - if (null !== $mapped->id) { + if (null !== $mapped) { $this->bill = $mapped; return true; } // 2: find existing by given values: $found = $this->findExistingObject(); - if (null !== $found->id) { + if (null !== $found) { $this->bill = $found; return true; diff --git a/app/Import/Object/ImportBudget.php b/app/Import/Object/ImportBudget.php index a45de9d5d6..d1bc96aabd 100644 --- a/app/Import/Object/ImportBudget.php +++ b/app/Import/Object/ImportBudget.php @@ -25,7 +25,6 @@ namespace FireflyIII\Import\Object; use FireflyIII\Models\Budget; use FireflyIII\Repositories\Budget\BudgetRepositoryInterface; use FireflyIII\User; -use Illuminate\Support\Collection; use Log; /** @@ -49,7 +48,6 @@ class ImportBudget */ public function __construct() { - $this->budget = new Budget; $this->repository = app(BudgetRepositoryInterface::class); Log::debug('Created ImportBudget.'); } @@ -59,7 +57,7 @@ class ImportBudget */ public function getBudget(): Budget { - if (null === $this->budget->id) { + if (null === $this->budget) { $this->store(); } @@ -92,27 +90,31 @@ class ImportBudget } /** - * @return Budget + * @return Budget|null */ - private function findExistingObject(): Budget + private function findById(): ?Budget { - Log::debug('In findExistingObject() for Budget'); - // 1: find by ID, or name - if (3 === count($this->id)) { Log::debug(sprintf('Finding budget with ID #%d', $this->id['value'])); /** @var Budget $budget */ - $budget = $this->repository->find(intval($this->id['value'])); - if (null !== $budget->id) { + $budget = $this->repository->findNull(intval($this->id['value'])); + if (null !== $budget) { Log::debug(sprintf('Found unmapped budget by ID (#%d): %s', $budget->id, $budget->name)); return $budget; } Log::debug('Found nothing.'); } - // 2: find by name + + return null; + } + + /** + * @return Budget|null + */ + private function findByName(): ?Budget + { if (3 === count($this->name)) { - /** @var Collection $budgets */ $budgets = $this->repository->getBudgets(); $name = $this->name['value']; Log::debug(sprintf('Finding budget with name %s', $name)); @@ -134,16 +136,33 @@ class ImportBudget Log::debug('Found nothing.'); } - // 4: do not search by account number. - Log::debug('Found NO existing budgets.'); - - return new Budget; + return null; } /** * @return Budget */ - private function findMappedObject(): Budget + private function findExistingObject(): ?Budget + { + Log::debug('In findExistingObject() for Budget'); + $result = $this->findById(); + if (!is_null($result)) { + return $result; + } + $result = $this->findByName(); + if (!is_null($result)) { + return $result; + } + + Log::debug('Found NO existing budgets.'); + + return null; + } + + /** + * @return Budget + */ + private function findMappedObject(): ?Budget { Log::debug('In findMappedObject() for Budget'); $fields = ['id', 'name']; @@ -160,7 +179,7 @@ class ImportBudget } Log::debug('Found no budget on mapped data or no map present.'); - return new Budget; + return null; } /** @@ -168,19 +187,19 @@ class ImportBudget * * @return Budget */ - private function getMappedObject(array $array): Budget + private function getMappedObject(array $array): ?Budget { Log::debug('In getMappedObject() for Budget'); if (0 === count($array)) { Log::debug('Array is empty, nothing will come of this.'); - return new Budget; + return null; } if (array_key_exists('mapped', $array) && null === $array['mapped']) { Log::debug(sprintf('No map present for value "%s". Return NULL.', $array['value'])); - return new Budget; + return null; } Log::debug('Finding a mapped budget based on', $array); @@ -191,7 +210,7 @@ class ImportBudget if (null === $budget->id) { Log::error(sprintf('There is no budget with id #%d. Invalid mapping will be ignored!', $search)); - return new Budget; + return null; } Log::debug(sprintf('Found budget! #%d ("%s"). Return it', $budget->id, $budget->name)); @@ -206,14 +225,14 @@ class ImportBudget { // 1: find mapped object: $mapped = $this->findMappedObject(); - if (null !== $mapped->id) { + if (null !== $mapped) { $this->budget = $mapped; return true; } // 2: find existing by given values: $found = $this->findExistingObject(); - if (null !== $found->id) { + if (null !== $found) { $this->budget = $found; return true; diff --git a/app/Import/Object/ImportCategory.php b/app/Import/Object/ImportCategory.php index 3c899f1664..7845fd878b 100644 --- a/app/Import/Object/ImportCategory.php +++ b/app/Import/Object/ImportCategory.php @@ -25,7 +25,6 @@ namespace FireflyIII\Import\Object; use FireflyIII\Models\Category; use FireflyIII\Repositories\Category\CategoryRepositoryInterface; use FireflyIII\User; -use Illuminate\Support\Collection; use Log; /** @@ -49,7 +48,6 @@ class ImportCategory */ public function __construct() { - $this->category = new Category(); $this->repository = app(CategoryRepositoryInterface::class); Log::debug('Created ImportCategory.'); } @@ -59,7 +57,7 @@ class ImportCategory */ public function getCategory(): Category { - if (null === $this->category->id) { + if (null === $this->category) { $this->store(); } @@ -92,27 +90,35 @@ class ImportCategory } /** - * @return Category + * Find category by ID. + * + * @return Category|null */ - private function findExistingObject(): Category + private function findById(): ?Category { - Log::debug('In findExistingObject() for Category'); - // 1: find by ID, or name - if (3 === count($this->id)) { Log::debug(sprintf('Finding category with ID #%d', $this->id['value'])); /** @var Category $category */ - $category = $this->repository->find(intval($this->id['value'])); - if (null !== $category->id) { + $category = $this->repository->findNull(intval($this->id['value'])); + if (null !== $category) { Log::debug(sprintf('Found unmapped category by ID (#%d): %s', $category->id, $category->name)); return $category; } Log::debug('Found nothing.'); } - // 2: find by name + + return null; + } + + /** + * Find category by name. + * + * @return Category|null + */ + private function findByName(): ?Category + { if (3 === count($this->name)) { - /** @var Collection $categories */ $categories = $this->repository->getCategories(); $name = $this->name['value']; Log::debug(sprintf('Finding category with name %s', $name)); @@ -134,16 +140,34 @@ class ImportCategory Log::debug('Found nothing.'); } - // 4: do not search by account number. - Log::debug('Found NO existing categories.'); - - return new Category; + return null; } /** * @return Category */ - private function findMappedObject(): Category + private function findExistingObject(): ?Category + { + Log::debug('In findExistingObject() for Category'); + $result = $this->findById(); + if (!is_null($result)) { + return $result; + } + + $result = $this->findByName(); + if (!is_null($result)) { + return $result; + } + + Log::debug('Found NO existing categories.'); + + return null; + } + + /** + * @return Category + */ + private function findMappedObject(): ?Category { Log::debug('In findMappedObject() for Category'); $fields = ['id', 'name']; @@ -152,7 +176,7 @@ class ImportCategory Log::debug(sprintf('Find mapped category 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 category #%d!', $mapped->id)); return $mapped; @@ -160,7 +184,7 @@ class ImportCategory } Log::debug('Found no category on mapped data or no map present.'); - return new Category; + return null; } /** @@ -174,24 +198,24 @@ class ImportCategory if (0 === count($array)) { Log::debug('Array is empty, nothing will come of this.'); - return new Category; + return null; } if (array_key_exists('mapped', $array) && null === $array['mapped']) { Log::debug(sprintf('No map present for value "%s". Return NULL.', $array['value'])); - return new Category; + return null; } Log::debug('Finding a mapped category based on', $array); $search = intval($array['mapped']); - $category = $this->repository->find($search); + $category = $this->repository->findNull($search); - if (null === $category->id) { + if (null === $category) { Log::error(sprintf('There is no category with id #%d. Invalid mapping will be ignored!', $search)); - return new Category; + return null; } Log::debug(sprintf('Found category! #%d ("%s"). Return it', $category->id, $category->name)); @@ -206,14 +230,14 @@ class ImportCategory { // 1: find mapped object: $mapped = $this->findMappedObject(); - if (null !== $mapped->id) { + if (null !== $mapped) { $this->category = $mapped; return true; } // 2: find existing by given values: $found = $this->findExistingObject(); - if (null !== $found->id) { + if (null !== $found) { $this->category = $found; return true; @@ -226,11 +250,7 @@ class ImportCategory Log::debug('Found no category so must create one ourselves.'); - $data = [ - 'name' => $name, - ]; - - $this->category = $this->repository->store($data); + $this->category = $this->repository->store(['name' => $name]); Log::debug(sprintf('Successfully stored new category #%d: %s', $this->category->id, $this->category->name)); return true; diff --git a/app/Import/Object/ImportCurrency.php b/app/Import/Object/ImportCurrency.php index 9a459d4c24..d375048024 100644 --- a/app/Import/Object/ImportCurrency.php +++ b/app/Import/Object/ImportCurrency.php @@ -52,23 +52,22 @@ class ImportCurrency */ public function __construct() { - $this->currency = new TransactionCurrency; $this->repository = app(CurrencyRepositoryInterface::class); } /** * @return TransactionCurrency */ - public function getTransactionCurrency(): TransactionCurrency + public function getTransactionCurrency(): ?TransactionCurrency { - if (null !== $this->currency->id) { + if (null !== $this->currency) { return $this->currency; } Log::debug('In createCurrency()'); // check if any of them is mapped: $mapped = $this->findMappedObject(); - if (null !== $mapped->id) { + if (null !== $mapped) { Log::debug('Mapped existing currency.', ['new' => $mapped->toArray()]); $this->currency = $mapped; @@ -76,7 +75,7 @@ class ImportCurrency } $searched = $this->findExistingObject(); - if (null !== $searched->id) { + if (null !== $searched) { Log::debug('Found existing currency.', ['found' => $searched->toArray()]); $this->currency = $searched; @@ -91,7 +90,7 @@ class ImportCurrency if (null === $data['code']) { Log::debug('Need at least a code to create currency, return nothing.'); - return new TransactionCurrency(); + return null; } Log::debug('Search for maps resulted in nothing, create new one based on', $data); @@ -147,33 +146,34 @@ class ImportCurrency /** * @return TransactionCurrency */ - private function findExistingObject(): TransactionCurrency + private function findExistingObject(): ?TransactionCurrency { $search = [ - 'id' => 'find', - 'code' => 'findByCode', - 'symbol' => 'findBySymbol', - 'name' => 'findByName', + 'id' => 'findNull', + 'code' => 'findByCodeNull', + 'symbol' => 'findBySymbolNull', + 'name' => 'findByNameNull', ]; foreach ($search as $field => $function) { $value = $this->$field['value'] ?? null; if (null !== $value) { Log::debug(sprintf('Searching for %s using function %s and value %s', $field, $function, $value)); + /** @var TransactionCurrency|null $currency */ $currency = $this->repository->$function($value); - if (null !== $currency->id) { + if (null !== $currency) { return $currency; } } } - return new TransactionCurrency(); + return null; } /** * @return TransactionCurrency */ - private function findMappedObject(): TransactionCurrency + private function findMappedObject(): ?TransactionCurrency { Log::debug('In findMappedObject()'); $fields = ['id', 'code', 'name', 'symbol']; @@ -182,7 +182,7 @@ class ImportCurrency Log::debug(sprintf('Find mapped currency 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 currency #%d!', $mapped->id)); return $mapped; @@ -190,7 +190,7 @@ class ImportCurrency } Log::debug('Found no currency on mapped data or no map present.'); - return new TransactionCurrency; + return null; } /** @@ -198,30 +198,30 @@ class ImportCurrency * * @return TransactionCurrency */ - private function getMappedObject(array $array): TransactionCurrency + private function getMappedObject(array $array): ?TransactionCurrency { Log::debug('In getMappedObject()'); if (0 === count($array)) { Log::debug('Array is empty, nothing will come of this.'); - return new TransactionCurrency; + return null; } if (array_key_exists('mapped', $array) && null === $array['mapped']) { Log::debug(sprintf('No map present for value "%s". Return NULL.', $array['value'])); - return new TransactionCurrency; + return null; } Log::debug('Finding a mapped object based on', $array); $search = intval($array['mapped']); - $currency = $this->repository->find($search); + $currency = $this->repository->findNull($search); - if (null === $currency->id) { + if (null === $currency) { Log::error(sprintf('There is no currency with id #%d. Invalid mapping will be ignored!', $search)); - return new TransactionCurrency; + return null; } Log::debug(sprintf('Found currency! #%d ("%s"). Return it', $currency->id, $currency->name)); diff --git a/app/Import/Object/ImportJournal.php b/app/Import/Object/ImportJournal.php index cd7c9e11c7..4a7e607b2e 100644 --- a/app/Import/Object/ImportJournal.php +++ b/app/Import/Object/ImportJournal.php @@ -339,7 +339,7 @@ class ImportJournal throw new FireflyException('No amount information for this row.'); } $class = $info['class'] ?? ''; - if (strlen($class) === 0) { + if (0 === strlen($class)) { throw new FireflyException('No amount information (conversion class) for this row.'); } diff --git a/app/Import/Prerequisites/BunqPrerequisites.php b/app/Import/Prerequisites/BunqPrerequisites.php index bb5e1e3599..7c982c9bec 100644 --- a/app/Import/Prerequisites/BunqPrerequisites.php +++ b/app/Import/Prerequisites/BunqPrerequisites.php @@ -114,5 +114,4 @@ class BunqPrerequisites implements PrerequisitesInterface return new MessageBag; } - } diff --git a/app/Import/Prerequisites/FilePrerequisites.php b/app/Import/Prerequisites/FilePrerequisites.php index e0a9219218..72ed8fab76 100644 --- a/app/Import/Prerequisites/FilePrerequisites.php +++ b/app/Import/Prerequisites/FilePrerequisites.php @@ -63,6 +63,7 @@ class FilePrerequisites implements PrerequisitesInterface * True if prerequisites. False if not. * * @return bool + * * @throws FireflyException */ public function hasPrerequisites(): bool diff --git a/app/Import/Routine/BunqRoutine.php b/app/Import/Routine/BunqRoutine.php index 7036268ce5..1921d31cbb 100644 --- a/app/Import/Routine/BunqRoutine.php +++ b/app/Import/Routine/BunqRoutine.php @@ -71,8 +71,6 @@ use Requests; * Map accounts to existing accounts * * Stage 'do-import'? - * - * */ class BunqRoutine implements RoutineInterface { @@ -122,8 +120,8 @@ class BunqRoutine implements RoutineInterface } /** - * * @return bool + * * @throws FireflyException */ public function run(): bool @@ -153,7 +151,7 @@ class BunqRoutine implements RoutineInterface protected function continueJob() { // if in "configuring" - if ($this->getStatus() === 'configuring') { + if ('configuring' === $this->getStatus()) { Log::debug('Job is in configuring stage, will do nothing.'); return; @@ -203,7 +201,6 @@ class BunqRoutine implements RoutineInterface } /** - * * @throws FireflyException */ protected function runStageInitial() @@ -372,6 +369,7 @@ class BunqRoutine implements RoutineInterface * Get the installation token, either from the users preferences or from Bunq. * * @return InstallationToken + * * @throws FireflyException */ private function getInstallationToken(): InstallationToken @@ -477,6 +475,7 @@ class BunqRoutine implements RoutineInterface * Get the public key of the server, necessary to verify server signature. * * @return ServerPublicKey + * * @throws FireflyException */ private function getServerPublicKey(): ServerPublicKey @@ -573,16 +572,16 @@ class BunqRoutine implements RoutineInterface $config = $this->getConfig(); $user = new UserPerson($config['user_person']); $mapping = $config['accounts-mapped']; - $token = new SessionToken($config['session_token']); + $token = new SessionToken($config['session_token']); $count = 0; - if ($user->getId() === 0) { + if (0 === $user->getId()) { $user = new UserCompany($config['user_company']); } foreach ($config['accounts'] as $accountData) { $account = new MonetaryAccountBank($accountData); $importId = $account->getId(); - if ($mapping[$importId] === 1) { + if (1 === $mapping[$importId]) { // grab all transactions $request = new ListPaymentRequest(); $request->setPrivateKey($this->getPrivateKey()); @@ -610,7 +609,6 @@ class BunqRoutine implements RoutineInterface } /** - * * @throws FireflyException */ private function runStageLoggedIn(): void @@ -619,7 +617,7 @@ class BunqRoutine implements RoutineInterface $config = $this->getConfig(); $token = new SessionToken($config['session_token']); $user = new UserPerson($config['user_person']); - if ($user->getId() === 0) { + if (0 === $user->getId()) { $user = new UserCompany($config['user_company']); } diff --git a/app/Import/Routine/FileRoutine.php b/app/Import/Routine/FileRoutine.php index 121651660f..33f12dd0f5 100644 --- a/app/Import/Routine/FileRoutine.php +++ b/app/Import/Routine/FileRoutine.php @@ -113,7 +113,6 @@ class FileRoutine implements RoutineInterface $this->addStep(); Log::debug('Back in run()'); - Log::debug('Updated job...'); Log::debug(sprintf('%d journals in $storage->journals', $storage->journals->count())); $this->journals = $storage->journals; diff --git a/app/Import/Routine/SpectreRoutine.php b/app/Import/Routine/SpectreRoutine.php index 58d0b28c58..347031b924 100644 --- a/app/Import/Routine/SpectreRoutine.php +++ b/app/Import/Routine/SpectreRoutine.php @@ -118,6 +118,7 @@ class SpectreRoutine implements RoutineInterface * * * @return bool + * * @throws FireflyException * @throws SpectreException * @throws \Illuminate\Container\EntryNotFoundException @@ -166,6 +167,7 @@ class SpectreRoutine implements RoutineInterface /** * @return Customer + * * @throws \FireflyIII\Exceptions\FireflyException * @throws \FireflyIII\Services\Spectre\Exception\SpectreException * @throws \Illuminate\Container\EntryNotFoundException @@ -187,22 +189,21 @@ class SpectreRoutine implements RoutineInterface $customers = $getCustomerRequest->getCustomers(); /** @var Customer $current */ foreach ($customers as $current) { - if ($current->getIdentifier() === 'default_ff3_customer') { + if ('default_ff3_customer' === $current->getIdentifier()) { $customer = $current; break; } } } - Preferences::setForUser($this->job->user, 'spectre_customer', $customer->toArray()); return $customer; - } /** * @return Customer + * * @throws FireflyException * @throws SpectreException * @throws \Illuminate\Container\EntryNotFoundException @@ -232,6 +233,7 @@ class SpectreRoutine implements RoutineInterface * @param string $returnUri * * @return Token + * * @throws \FireflyIII\Exceptions\FireflyException * @throws \FireflyIII\Services\Spectre\Exception\SpectreException * @throws \Illuminate\Container\EntryNotFoundException @@ -245,7 +247,6 @@ class SpectreRoutine implements RoutineInterface Log::debug('Call to get token is finished'); return $request->getToken(); - } /** @@ -460,12 +461,10 @@ class SpectreRoutine implements RoutineInterface // date: $importJournal->setValue(['role' => 'date-transaction', 'value' => $transaction->getMadeOn()->toIso8601String()]); - // amount $importJournal->setValue(['role' => 'amount', 'value' => $transaction->getAmount()]); $importJournal->setValue(['role' => 'currency-code', 'value' => $transaction->getCurrencyCode()]); - // various meta fields: $importJournal->setValue(['role' => 'category-name', 'value' => $transaction->getCategory()]); $importJournal->setValue(['role' => 'note', 'value' => $notes]); @@ -540,7 +539,7 @@ class SpectreRoutine implements RoutineInterface foreach ($accounts as $accountArray) { $account = new Account($accountArray); $importId = intval($config['accounts-mapped'][$account->getid()] ?? 0); - $doImport = $importId !== 0 ? true : false; + $doImport = 0 !== $importId ? true : false; if (!$doImport) { Log::debug(sprintf('Will NOT import from Spectre account #%d ("%s")', $account->getId(), $account->getName())); continue; @@ -560,7 +559,6 @@ class SpectreRoutine implements RoutineInterface Log::debug(sprintf('Total number of transactions: %d', $count)); $this->addStep(); - $this->importTransactions($all); } diff --git a/app/Import/Storage/ImportStorage.php b/app/Import/Storage/ImportStorage.php index f62e20c4fa..4759eee90e 100644 --- a/app/Import/Storage/ImportStorage.php +++ b/app/Import/Storage/ImportStorage.php @@ -132,8 +132,6 @@ class ImportStorage } Log::debug(sprintf('Value of apply rules is %s', var_export($this->applyRules, true))); Log::debug(sprintf('Value of match bills is %s', var_export($this->matchBills, true))); - - } /** diff --git a/app/Import/Storage/ImportSupport.php b/app/Import/Storage/ImportSupport.php index 97f85a54c6..ed92fbb82c 100644 --- a/app/Import/Storage/ImportSupport.php +++ b/app/Import/Storage/ImportSupport.php @@ -127,7 +127,7 @@ trait ImportSupport $transaction->transaction_journal_id = intval($parameters['id']); $transaction->transaction_currency_id = intval($parameters['currency']); $transaction->amount = $parameters['amount']; - $transaction->foreign_currency_id = intval($parameters['foreign_currency']) === 0 ? null : intval($parameters['foreign_currency']); + $transaction->foreign_currency_id = 0 === intval($parameters['foreign_currency']) ? null : intval($parameters['foreign_currency']); $transaction->foreign_amount = null === $transaction->foreign_currency_id ? null : $parameters['foreign_amount']; $transaction->save(); if (null === $transaction->id) { @@ -158,6 +158,7 @@ trait ImportSupport * @param ImportJournal $importJournal * * @return int + * * @throws FireflyException */ private function getCurrencyId(ImportJournal $importJournal): int @@ -171,7 +172,7 @@ trait ImportSupport // use given currency $currency = $importJournal->currency->getTransactionCurrency(); - if (null !== $currency->id) { + if (null !== $currency) { return $currency->id; } @@ -196,7 +197,7 @@ trait ImportSupport { // use given currency by import journal. $currency = $importJournal->currency->getTransactionCurrency(); - if (null !== $currency->id && intval($currency->id) !== intval($currencyId)) { + if (null !== $currency && intval($currency->id) !== intval($currencyId)) { return $currency->id; } @@ -220,6 +221,7 @@ trait ImportSupport * @see ImportSupport::getTransactionType * * @return Account + * * @throws FireflyException */ private function getOpposingAccount(ImportAccount $account, int $forbiddenAccount, string $amount): Account diff --git a/app/Repositories/Category/CategoryRepository.php b/app/Repositories/Category/CategoryRepository.php index c591141a8b..53328074fd 100644 --- a/app/Repositories/Category/CategoryRepository.php +++ b/app/Repositories/Category/CategoryRepository.php @@ -23,6 +23,7 @@ declare(strict_types=1); namespace FireflyIII\Repositories\Category; use Carbon\Carbon; +use FireflyIII\Factory\CategoryFactory; use FireflyIII\Helpers\Collector\JournalCollectorInterface; use FireflyIII\Models\Category; use FireflyIII\Models\Transaction; @@ -41,6 +42,8 @@ class CategoryRepository implements CategoryRepositoryInterface private $user; /** + * TODO move to delete service + * * @param Category $category * * @return bool @@ -444,18 +447,17 @@ class CategoryRepository implements CategoryRepositoryInterface */ public function store(array $data): Category { - $newCategory = Category::firstOrCreateEncrypted( - [ - 'user_id' => $this->user->id, - 'name' => $data['name'], - ] - ); - $newCategory->save(); + /** @var CategoryFactory $factory */ + $factory = app(CategoryFactory::class); + $factory->setUser($this->user); + $category = $factory->findOrCreate(null, $data['name']); - return $newCategory; + return $category; } /** + * TODO move to update service + * * @param Category $category * @param array $data * diff --git a/app/Repositories/Currency/CurrencyRepository.php b/app/Repositories/Currency/CurrencyRepository.php index 37af7d2f9b..adb2a6caf5 100644 --- a/app/Repositories/Currency/CurrencyRepository.php +++ b/app/Repositories/Currency/CurrencyRepository.php @@ -89,6 +89,8 @@ class CurrencyRepository implements CurrencyRepositoryInterface } /** + * TODO use service + * * @param TransactionCurrency $currency * * @return bool @@ -107,6 +109,8 @@ class CurrencyRepository implements CurrencyRepositoryInterface * * @param int $currencyId * + * @deprecated + * * @return TransactionCurrency */ public function find(int $currencyId): TransactionCurrency @@ -122,6 +126,8 @@ class CurrencyRepository implements CurrencyRepositoryInterface /** * Find by currency code. * + * @deprecated + * * @param string $currencyCode * * @return TransactionCurrency @@ -138,6 +144,7 @@ class CurrencyRepository implements CurrencyRepositoryInterface /** * Find by currency code, return NULL if unfound. + * Used in Import Currency! * * @param string $currencyCode * @@ -153,6 +160,8 @@ class CurrencyRepository implements CurrencyRepositoryInterface * * @param string $currencyName * + * @deprecated + * * @return TransactionCurrency */ public function findByName(string $currencyName): TransactionCurrency @@ -165,9 +174,24 @@ class CurrencyRepository implements CurrencyRepositoryInterface return $preferred; } + /** + * Find by currency name or return null. + * Used in Import Currency! + * + * @param string $currencyName + * + * @return TransactionCurrency + */ + public function findByNameNull(string $currencyName): ?TransactionCurrency + { + return TransactionCurrency::whereName($currencyName)->first(); + } + /** * Find by currency symbol. * + * @deprecated + * * @param string $currencySymbol * * @return TransactionCurrency @@ -182,8 +206,22 @@ class CurrencyRepository implements CurrencyRepositoryInterface return $currency; } + /** + * Find by currency symbol or return NULL + * Used in Import Currency! + * + * @param string $currencySymbol + * + * @return TransactionCurrency + */ + public function findBySymbolNull(string $currencySymbol): ?TransactionCurrency + { + return TransactionCurrency::whereSymbol($currencySymbol)->first(); + } + /** * Find by ID, return NULL if not found. + * Used in Import Currency! * * @param int $currencyId * @@ -269,6 +307,8 @@ class CurrencyRepository implements CurrencyRepositoryInterface } /** + * TODO use factory + * * @param array $data * * @return TransactionCurrency @@ -289,6 +329,8 @@ class CurrencyRepository implements CurrencyRepositoryInterface } /** + * TODO use factory + * * @param TransactionCurrency $currency * @param array $data * diff --git a/app/Repositories/Currency/CurrencyRepositoryInterface.php b/app/Repositories/Currency/CurrencyRepositoryInterface.php index 60df5cf396..39d3f78a47 100644 --- a/app/Repositories/Currency/CurrencyRepositoryInterface.php +++ b/app/Repositories/Currency/CurrencyRepositoryInterface.php @@ -87,13 +87,31 @@ interface CurrencyRepositoryInterface /** * Find by currency name. - * + * @deprecated * @param string $currencyName * * @return TransactionCurrency */ public function findByName(string $currencyName): TransactionCurrency; + /** + * Find by currency name. + * + * @param string $currencyName + * + * @return TransactionCurrency + */ + public function findByNameNull(string $currencyName): ?TransactionCurrency; + + /** + * Find by currency symbol. + * @deprecated + * @param string $currencySymbol + * + * @return TransactionCurrency + */ + public function findBySymbol(string $currencySymbol): TransactionCurrency; + /** * Find by currency symbol. * @@ -101,7 +119,7 @@ interface CurrencyRepositoryInterface * * @return TransactionCurrency */ - public function findBySymbol(string $currencySymbol): TransactionCurrency; + public function findBySymbolNull(string $currencySymbol): ?TransactionCurrency; /** * Find by ID, return NULL if not found. diff --git a/tests/Feature/Controllers/BudgetControllerTest.php b/tests/Feature/Controllers/BudgetControllerTest.php index d4249dc101..e5befa980b 100644 --- a/tests/Feature/Controllers/BudgetControllerTest.php +++ b/tests/Feature/Controllers/BudgetControllerTest.php @@ -65,6 +65,8 @@ class BudgetControllerTest extends TestCase $journalRepos->shouldReceive('first')->once()->andReturn(new TransactionJournal); $repository->shouldReceive('updateLimitAmount')->andReturn(new BudgetLimit); $repository->shouldReceive('spentInPeriod')->andReturn('0'); + $repository->shouldReceive('budgetedPerDay')->andReturn('10'); + $data = ['amount' => 200, 'start' => '2017-01-01', 'end' => '2017-01-31']; $this->be($this->user()); @@ -83,6 +85,7 @@ class BudgetControllerTest extends TestCase $journalRepos->shouldReceive('first')->once()->andReturn(new TransactionJournal); $repository->shouldReceive('updateLimitAmount')->andReturn(new BudgetLimit); $repository->shouldReceive('spentInPeriod')->andReturn('0'); + $repository->shouldReceive('budgetedPerDay')->andReturn('10'); $data = ['amount' => 0, 'start' => '2017-01-01', 'end' => '2017-01-31']; $this->be($this->user()); diff --git a/tests/Unit/Import/Object/ImportAccountTest.php b/tests/Unit/Import/Object/ImportAccountTest.php index c134122dc6..56a57b5a34 100644 --- a/tests/Unit/Import/Object/ImportAccountTest.php +++ b/tests/Unit/Import/Object/ImportAccountTest.php @@ -52,7 +52,7 @@ class ImportAccountTest extends TestCase // mock calls: $repository->shouldReceive('setUser')->once()->withArgs([Mockery::any()]); $repository->shouldReceive('getAccountType')->twice()->withArgs([AccountType::ASSET])->andReturn($accountType); - $repository->shouldReceive('getAccountsByType')->twice()->withArgs([[AccountType::ASSET]])->andReturn(new Collection()); + //$repository->shouldReceive('getAccountsByType')->twice()->withArgs([[AccountType::ASSET]])->andReturn(new Collection()); $repository->shouldReceive('findNull')->once()->withArgs([1])->andReturn($account); // create import account.