From 19fee6a8fb5722839b6d62b0a0f948e61bc459a9 Mon Sep 17 00:00:00 2001 From: James Cole Date: Tue, 12 Sep 2023 05:58:39 +0200 Subject: [PATCH] Fix https://github.com/orgs/firefly-iii/discussions/7940 --- app/Validation/Account/DepositValidation.php | 4 ++-- app/Validation/Account/WithdrawalValidation.php | 17 +++++++++++------ app/Validation/AccountValidator.php | 16 ++++++++++++---- 3 files changed, 25 insertions(+), 12 deletions(-) diff --git a/app/Validation/Account/DepositValidation.php b/app/Validation/Account/DepositValidation.php index 3fc58764ac..f1bfb62185 100644 --- a/app/Validation/Account/DepositValidation.php +++ b/app/Validation/Account/DepositValidation.php @@ -126,10 +126,10 @@ trait DepositValidation $result = false; } - // if there is an iban, it can only be in use by a revenue account, or we will fail. + // if there is an iban, it can only be in use by a valid source type, or we will fail. if (null !== $accountIban && '' !== $accountIban) { app('log')->debug('Check if there is not already an account with this IBAN'); - $existing = $this->findExistingAccount([AccountType::ASSET, AccountType::LOAN, AccountType::DEBT, AccountType::MORTGAGE], ['iban' => $accountIban]); + $existing = $this->findExistingAccount($validTypes, ['iban' => $accountIban], true); if (null !== $existing) { $this->sourceError = (string)trans('validation.deposit_src_iban_exists'); return false; diff --git a/app/Validation/Account/WithdrawalValidation.php b/app/Validation/Account/WithdrawalValidation.php index c953faf3af..e8343e44c0 100644 --- a/app/Validation/Account/WithdrawalValidation.php +++ b/app/Validation/Account/WithdrawalValidation.php @@ -90,13 +90,15 @@ trait WithdrawalValidation */ protected function validateWithdrawalDestination(array $array): bool { - $accountId = array_key_exists('id', $array) ? $array['id'] : null; - $accountName = array_key_exists('name', $array) ? $array['name'] : null; - $accountIban = array_key_exists('iban', $array) ? $array['iban'] : null; + $accountId = array_key_exists('id', $array) ? $array['id'] : null; + $accountName = array_key_exists('name', $array) ? $array['name'] : null; + $accountIban = array_key_exists('iban', $array) ? $array['iban'] : null; + $accountNumber = array_key_exists('number', $array) ? $array['number'] : null; Log::debug('Now in validateWithdrawalDestination()', $array); // source can be any of the following types. $validTypes = $this->combinations[$this->transactionType][$this->source->accountType->type] ?? []; - if (null === $accountId && null === $accountName && null === $accountIban && false === $this->canCreateTypes($validTypes)) { + app('log')->debug('Source type can be: ', $validTypes); + if (null === $accountId && null === $accountName && null === $accountIban && null === $accountNumber && false === $this->canCreateTypes($validTypes)) { // if both values are NULL return false, // because the destination of a withdrawal can never be created automatically. $this->destError = (string)trans('validation.withdrawal_dest_need_data'); @@ -113,15 +115,18 @@ trait WithdrawalValidation $this->setDestination($found); return true; } + // todo explain error in log message. $this->destError = (string)trans('validation.withdrawal_dest_bad_data', ['id' => $accountId, 'name' => $accountName]); return false; } } - // if there is an iban, it can only be in use by a revenue account or we will fail. + // if there is an iban, it can only be in use by a valid destination type, or we will fail. + // the inverse of $validTypes is if (null !== $accountIban && '' !== $accountIban) { app('log')->debug('Check if there is not already an account with this IBAN'); - $existing = $this->findExistingAccount([AccountType::ASSET, AccountType::LOAN, AccountType::DEBT, AccountType::MORTGAGE], ['iban' => $accountIban]); + // the inverse flag reverses the search, searching for everything that is NOT a valid type. + $existing = $this->findExistingAccount($validTypes, ['iban' => $accountIban], true); if (null !== $existing) { $this->destError = (string)trans('validation.withdrawal_dest_iban_exists'); return false; diff --git a/app/Validation/AccountValidator.php b/app/Validation/AccountValidator.php index 333b617252..c1544f5cfb 100644 --- a/app/Validation/AccountValidator.php +++ b/app/Validation/AccountValidator.php @@ -250,12 +250,14 @@ class AccountValidator /** * @param array $validTypes * @param array $data + * @param bool $inverse * * @return Account|null */ - protected function findExistingAccount(array $validTypes, array $data): ?Account + protected function findExistingAccount(array $validTypes, array $data, bool $inverse = false): ?Account { Log::debug('Now in findExistingAccount', $data); + app('log')->debug('The search will be reversed!'); $accountId = array_key_exists('id', $data) ? $data['id'] : null; $accountIban = array_key_exists('iban', $data) ? $data['iban'] : null; $accountNumber = array_key_exists('number', $data) ? $data['number'] : null; @@ -264,7 +266,9 @@ class AccountValidator // find by ID if (null !== $accountId && $accountId > 0) { $first = $this->accountRepository->find($accountId); - if ((null !== $first) && in_array($first->accountType->type, $validTypes, true)) { + $check = in_array($first->accountType->type, $validTypes, true); + $check = $inverse ? !$check : $check; // reverse the validation check if necessary. + if ((null !== $first) && $check) { app('log')->debug(sprintf('ID: Found %s account #%d ("%s", IBAN "%s")', $first->accountType->type, $first->id, $first->name, $first->iban ?? 'no iban')); return $first; } @@ -273,7 +277,9 @@ class AccountValidator // find by iban if (null !== $accountIban && '' !== (string)$accountIban) { $first = $this->accountRepository->findByIbanNull($accountIban, $validTypes); - if ((null !== $first) && in_array($first->accountType->type, $validTypes, true)) { + $check = in_array($first->accountType->type, $validTypes, true); + $check = $inverse ? !$check : $check; // reverse the validation check if necessary. + if ((null !== $first) && $check) { app('log')->debug(sprintf('Iban: Found %s account #%d ("%s", IBAN "%s")', $first->accountType->type, $first->id, $first->name, $first->iban ?? 'no iban')); return $first; } @@ -282,7 +288,9 @@ class AccountValidator // find by number if (null !== $accountNumber && '' !== (string)$accountNumber) { $first = $this->accountRepository->findByAccountNumber($accountNumber, $validTypes); - if ((null !== $first) && in_array($first->accountType->type, $validTypes, true)) { + $check = in_array($first->accountType->type, $validTypes, true); + $check = $inverse ? !$check : $check; // reverse the validation check if necessary. + if ((null !== $first) && $check) { app('log')->debug(sprintf('Number: Found %s account #%d ("%s", IBAN "%s")', $first->accountType->type, $first->id, $first->name, $first->iban ?? 'no iban')); return $first; }