From 3491fbb99dc131736c92a0a3d7376049224261ff Mon Sep 17 00:00:00 2001 From: James Cole Date: Wed, 17 Sep 2025 07:09:40 +0200 Subject: [PATCH] Force account search to validate it did not just find the source account. #10920 --- app/Factory/TransactionJournalFactory.php | 2 +- .../Account/AccountRepository.php | 13 ++-- .../Internal/Support/JournalServiceTrait.php | 74 ++++++++++++------- app/Validation/Account/DepositValidation.php | 34 +++++---- app/Validation/Account/OBValidation.php | 23 +++--- .../Account/WithdrawalValidation.php | 25 ++++--- app/Validation/AccountValidator.php | 43 +++++------ resources/lang/en_US/validation.php | 12 +-- 8 files changed, 126 insertions(+), 100 deletions(-) diff --git a/app/Factory/TransactionJournalFactory.php b/app/Factory/TransactionJournalFactory.php index 7206947be2..dd2d9e6830 100644 --- a/app/Factory/TransactionJournalFactory.php +++ b/app/Factory/TransactionJournalFactory.php @@ -222,7 +222,7 @@ class TransactionJournalFactory Log::debug('Source info:', $sourceInfo); Log::debug('Destination info:', $destInfo); $sourceAccount = $this->getAccount($type->type, 'source', $sourceInfo); - $destinationAccount = $this->getAccount($type->type, 'destination', $destInfo); + $destinationAccount = $this->getAccount($type->type, 'destination', $destInfo, $sourceAccount); Log::debug('Done with getAccount(2x)'); diff --git a/app/Repositories/Account/AccountRepository.php b/app/Repositories/Account/AccountRepository.php index c9039b7c0f..c2a14d8d81 100644 --- a/app/Repositories/Account/AccountRepository.php +++ b/app/Repositories/Account/AccountRepository.php @@ -45,6 +45,7 @@ use FireflyIII\Support\Repositories\UserGroup\UserGroupTrait; use Illuminate\Database\Eloquent\Builder as EloquentBuilder; use Illuminate\Database\Eloquent\Relations\HasMany; use Illuminate\Support\Collection; +use Illuminate\Support\Facades\Log; use Illuminate\Support\Facades\Storage; use Override; @@ -150,18 +151,18 @@ class AccountRepository implements AccountRepositoryInterface, UserGroupInterfac $query->leftJoin('account_types', 'accounts.account_type_id', '=', 'account_types.id'); $query->whereIn('account_types.type', $types); } - app('log')->debug(sprintf('Searching for account named "%s" (of user #%d) of the following type(s)', $name, $this->user->id), ['types' => $types]); + Log::debug(sprintf('Searching for account named "%s" (of user #%d) of the following type(s)', $name, $this->user->id), ['types' => $types]); $query->where('accounts.name', $name); /** @var null|Account $account */ $account = $query->first(['accounts.*']); if (null === $account) { - app('log')->debug(sprintf('There is no account with name "%s" of types', $name), $types); + Log::debug(sprintf('There is no account with name "%s" of types', $name), $types); return null; } - app('log')->debug(sprintf('Found #%d (%s) with type id %d', $account->id, $account->name, $account->account_type_id)); + Log::debug(sprintf('Found #%d (%s) with type id %d', $account->id, $account->name, $account->account_type_id)); return $account; } @@ -465,14 +466,14 @@ class AccountRepository implements AccountRepositoryInterface, UserGroupInterfac ]; if (array_key_exists(ucfirst($type), $sets)) { $order = (int) $this->getAccountsByType($sets[ucfirst($type)])->max('order'); - app('log')->debug(sprintf('Return max order of "%s" set: %d', $type, $order)); + Log::debug(sprintf('Return max order of "%s" set: %d', $type, $order)); return $order; } $specials = [AccountTypeEnum::CASH->value, AccountTypeEnum::INITIAL_BALANCE->value, AccountTypeEnum::IMPORT->value, AccountTypeEnum::RECONCILIATION->value]; $order = (int) $this->getAccountsByType($specials)->max('order'); - app('log')->debug(sprintf('Return max order of "%s" set (specials!): %d', $type, $order)); + Log::debug(sprintf('Return max order of "%s" set (specials!): %d', $type, $order)); return $order; } @@ -599,7 +600,7 @@ class AccountRepository implements AccountRepositoryInterface, UserGroupInterfac continue; } if ($index !== (int) $account->order) { - app('log')->debug(sprintf('Account #%d ("%s"): order should %d be but is %d.', $account->id, $account->name, $index, $account->order)); + Log::debug(sprintf('Account #%d ("%s"): order should %d be but is %d.', $account->id, $account->name, $index, $account->order)); $account->order = $index; $account->save(); } diff --git a/app/Services/Internal/Support/JournalServiceTrait.php b/app/Services/Internal/Support/JournalServiceTrait.php index d0de5db9df..9d7cedad90 100644 --- a/app/Services/Internal/Support/JournalServiceTrait.php +++ b/app/Services/Internal/Support/JournalServiceTrait.php @@ -54,7 +54,7 @@ trait JournalServiceTrait /** * @throws FireflyException */ - protected function getAccount(string $transactionType, string $direction, array $data): ?Account + protected function getAccount(string $transactionType, string $direction, array $data, ?Account $opposite = null): ?Account { // some debug logging: Log::debug(sprintf('Now in getAccount(%s)', $direction), $data); @@ -69,12 +69,12 @@ trait JournalServiceTrait $message = 'Transaction = %s, %s account should be in: %s. Direction is %s.'; Log::debug(sprintf($message, $transactionType, $direction, implode(', ', $expectedTypes[$transactionType] ?? ['UNKNOWN']), $direction)); - $result = $this->findAccountById($data, $expectedTypes[$transactionType]); - $result = $this->findAccountByIban($result, $data, $expectedTypes[$transactionType]); + $result = $this->findAccountById($data, $expectedTypes[$transactionType], $opposite); + $result = $this->findAccountByIban($result, $data, $expectedTypes[$transactionType], $opposite); $ibanResult = $result; - $result = $this->findAccountByNumber($result, $data, $expectedTypes[$transactionType]); + $result = $this->findAccountByNumber($result, $data, $expectedTypes[$transactionType], $opposite); $numberResult = $result; - $result = $this->findAccountByName($result, $data, $expectedTypes[$transactionType]); + $result = $this->findAccountByName($result, $data, $expectedTypes[$transactionType], $opposite); $nameResult = $result; // if $result (find by name) is NULL, but IBAN is set, any result of the search by NAME can't overrule @@ -82,7 +82,7 @@ trait JournalServiceTrait if (null !== $nameResult && null === $numberResult && null === $ibanResult && '' !== (string) $data['iban'] && '' !== (string) $nameResult->iban) { $data['name'] = sprintf('%s (%s)', $data['name'], $data['iban']); Log::debug(sprintf('Search again using the new name, "%s".', $data['name'])); - $result = $this->findAccountByName(null, $data, $expectedTypes[$transactionType]); + $result = $this->findAccountByName(null, $data, $expectedTypes[$transactionType], $opposite); } // the account that Firefly III creates must be "creatable", aka select the one we can create from the list just in case @@ -115,15 +115,18 @@ trait JournalServiceTrait return $result; } - private function findAccountById(array $data, array $types): ?Account + private function findAccountById(array $data, array $types, ?Account $opposite = null): ?Account { // first attempt, find by ID. if (null !== $data['id']) { $search = $this->accountRepository->find((int) $data['id']); if (null !== $search && in_array($search->accountType->type, $types, true)) { - Log::debug( - sprintf('Found "account_id" object: #%d, "%s" of type %s (1)', $search->id, $search->name, $search->accountType->type) - ); + Log::debug(sprintf('Found "account_id" object: #%d, "%s" of type %s (1)', $search->id, $search->name, $search->accountType->type)); + + if($opposite?->id === $search->id) { + Log::debug(sprintf('Account #%d is the same as opposite account #%d, returning NULL.', $search->id, $opposite->id)); + return null; + } return $search; } @@ -140,7 +143,7 @@ trait JournalServiceTrait return null; } - private function findAccountByIban(?Account $account, array $data, array $types): ?Account + private function findAccountByIban(?Account $account, array $data, array $types, ?Account $opposite = null): ?Account { if ($account instanceof Account) { Log::debug(sprintf('Already have account #%d ("%s"), return that.', $account->id, $account->name)); @@ -153,21 +156,26 @@ trait JournalServiceTrait return null; } // find by preferred type. - $source = $this->accountRepository->findByIbanNull($data['iban'], [$types[0]]); + $result = $this->accountRepository->findByIbanNull($data['iban'], [$types[0]]); // or any expected type. - $source ??= $this->accountRepository->findByIbanNull($data['iban'], $types); + $result ??= $this->accountRepository->findByIbanNull($data['iban'], $types); - if (null !== $source) { - Log::debug(sprintf('Found "account_iban" object: #%d, %s', $source->id, $source->name)); + if (null !== $result) { + Log::debug(sprintf('Found "account_iban" object: #%d, %s', $result->id, $result->name)); - return $source; + if($opposite?->id === $result->id) { + Log::debug(sprintf('Account #%d is the same as opposite account #%d, returning NULL.', $result->id, $opposite->id)); + return null; + } + + return $result; } Log::debug(sprintf('Found no account with IBAN "%s" of expected types', $data['iban']), $types); return null; } - private function findAccountByNumber(?Account $account, array $data, array $types): ?Account + private function findAccountByNumber(?Account $account, array $data, array $types, ?Account $opposite = null): ?Account { if ($account instanceof Account) { Log::debug(sprintf('Already have account #%d ("%s"), return that.', $account->id, $account->name)); @@ -180,15 +188,20 @@ trait JournalServiceTrait return null; } // find by preferred type. - $source = $this->accountRepository->findByAccountNumber((string) $data['number'], [$types[0]]); + $result = $this->accountRepository->findByAccountNumber((string) $data['number'], [$types[0]]); // or any expected type. - $source ??= $this->accountRepository->findByAccountNumber((string) $data['number'], $types); + $result ??= $this->accountRepository->findByAccountNumber((string) $data['number'], $types); - if (null !== $source) { - Log::debug(sprintf('Found account: #%d, %s', $source->id, $source->name)); + if (null !== $result) { + Log::debug(sprintf('Found account: #%d, %s', $result->id, $result->name)); - return $source; + if($opposite?->id === $result->id) { + Log::debug(sprintf('Account #%d is the same as opposite account #%d, returning NULL.', $result->id, $opposite->id)); + return null; + } + + return $result; } Log::debug(sprintf('Found no account with account number "%s" of expected types', $data['number']), $types); @@ -196,7 +209,7 @@ trait JournalServiceTrait return null; } - private function findAccountByName(?Account $account, array $data, array $types): ?Account + private function findAccountByName(?Account $account, array $data, array $types, ?Account $opposite = null): ?Account { if ($account instanceof Account) { Log::debug(sprintf('Already have account #%d ("%s"), return that.', $account->id, $account->name)); @@ -210,15 +223,20 @@ trait JournalServiceTrait } // find by preferred type. - $source = $this->accountRepository->findByName($data['name'], [$types[0]]); + $result = $this->accountRepository->findByName($data['name'], [$types[0]]); // or any expected type. - $source ??= $this->accountRepository->findByName($data['name'], $types); + $result ??= $this->accountRepository->findByName($data['name'], $types); - if (null !== $source) { - Log::debug(sprintf('Found "account_name" object: #%d, %s', $source->id, $source->name)); + if (null !== $result) { + Log::debug(sprintf('Found "account_name" object: #%d, %s', $result->id, $result->name)); - return $source; + if($opposite?->id === $result->id) { + Log::debug(sprintf('Account #%d is the same as opposite account #%d, returning NULL.', $result->id, $opposite->id)); + return null; + } + + return $result; } Log::debug(sprintf('Found no account with account name "%s" of expected types', $data['name']), $types); diff --git a/app/Validation/Account/DepositValidation.php b/app/Validation/Account/DepositValidation.php index 87e2e69f43..42a38447b6 100644 --- a/app/Validation/Account/DepositValidation.php +++ b/app/Validation/Account/DepositValidation.php @@ -27,6 +27,7 @@ namespace FireflyIII\Validation\Account; use FireflyIII\Enums\AccountTypeEnum; use FireflyIII\Models\Account; use FireflyIII\Models\AccountType; +use Illuminate\Support\Facades\Log; /** * Trait DepositValidation @@ -40,7 +41,7 @@ trait DepositValidation $accountName = array_key_exists('name', $array) ? $array['name'] : null; $accountIban = array_key_exists('iban', $array) ? $array['iban'] : null; - app('log')->debug('Now in validateDepositDestination', $array); + Log::debug('Now in validateDepositDestination', $array); // source can be any of the following types. $validTypes = $this->combinations[$this->transactionType][$this->source->accountType->type] ?? []; @@ -48,12 +49,12 @@ trait DepositValidation // if both values are NULL we return false, // because the destination of a deposit can't be created. $this->destError = (string) trans('validation.deposit_dest_need_data'); - app('log')->error('Both values are NULL, cant create deposit destination.'); + Log::error('Both values are NULL, cant create deposit destination.'); $result = false; } // if the account can be created anyway we don't need to search. if (null === $result && true === $this->canCreateTypes($validTypes)) { - app('log')->debug('Can create some of these types, so return true.'); + Log::debug('Can create some of these types, so return true.'); $result = true; } @@ -61,17 +62,17 @@ trait DepositValidation // otherwise try to find the account: $search = $this->findExistingAccount($validTypes, $array); if (null === $search) { - app('log')->debug('findExistingAccount() returned NULL, so the result is false.'); + Log::debug('findExistingAccount() returned NULL, so the result is false.'); $this->destError = (string) trans('validation.deposit_dest_bad_data', ['id' => $accountId, 'name' => $accountName]); $result = false; } if (null !== $search) { - app('log')->debug(sprintf('findExistingAccount() returned #%d ("%s"), so the result is true.', $search->id, $search->name)); + Log::debug(sprintf('findExistingAccount() returned #%d ("%s"), so the result is true.', $search->id, $search->name)); $this->setDestination($search); $result = true; } } - app('log')->debug(sprintf('validateDepositDestination will return %s', var_export($result, true))); + Log::debug(sprintf('validateDepositDestination will return %s', var_export($result, true))); return $result; } @@ -92,7 +93,7 @@ trait DepositValidation $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; - app('log')->debug('Now in validateDepositSource', $array); + Log::debug('Now in validateDepositSource', $array); // null = we found nothing at all or didn't even search // false = invalid results @@ -114,7 +115,7 @@ trait DepositValidation // 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 another account with this IBAN'); + Log::debug('Check if there is not already another account with this IBAN'); $existing = $this->findExistingAccount($validTypes, ['iban' => $accountIban], true); if (null !== $existing) { $this->sourceError = (string) trans('validation.deposit_src_iban_exists'); @@ -128,11 +129,14 @@ trait DepositValidation if (null !== $accountId) { $search = $this->accountRepository->find($accountId); if (null !== $search && !in_array($search->accountType->type, $validTypes, true)) { - app('log')->debug(sprintf('User submitted an ID (#%d), which is a "%s", so this is not a valid source.', $accountId, $search->accountType->type)); - app('log')->debug(sprintf('Firefly III accepts ID #%d as valid account data.', $accountId)); + Log::debug(sprintf('User submitted an ID (#%d), which is a "%s", so this is not a valid source.', $accountId, $search->accountType->type)); + Log::debug(sprintf('Firefly III does not accept ID #%d as valid account data.', $accountId)); + // #10921 Set result false + $this->sourceError = (string) trans('validation.withdrawal_source_bad_data', ['id' => $accountId, 'name' => $accountName]); + $result = false; } if (null !== $search && in_array($search->accountType->type, $validTypes, true)) { - app('log')->debug('ID result is not null and seems valid, save as source account.'); + Log::debug('ID result is not null and seems valid, save as source account.'); $this->setSource($search); $result = true; } @@ -142,11 +146,11 @@ trait DepositValidation if (null !== $accountIban) { $search = $this->accountRepository->findByIbanNull($accountIban, $validTypes); if (null !== $search && !in_array($search->accountType->type, $validTypes, true)) { - app('log')->debug(sprintf('User submitted IBAN ("%s"), which is a "%s", so this is not a valid source.', $accountIban, $search->accountType->type)); + Log::debug(sprintf('User submitted IBAN ("%s"), which is a "%s", so this is not a valid source.', $accountIban, $search->accountType->type)); $result = false; } if (null !== $search && in_array($search->accountType->type, $validTypes, true)) { - app('log')->debug('IBAN result is not null and seems valid, save as source account.'); + Log::debug('IBAN result is not null and seems valid, save as source account.'); $this->setSource($search); $result = true; } @@ -156,13 +160,13 @@ trait DepositValidation if (null !== $accountNumber && '' !== $accountNumber) { $search = $this->accountRepository->findByAccountNumber($accountNumber, $validTypes); if (null !== $search && !in_array($search->accountType->type, $validTypes, true)) { - app('log')->debug( + Log::debug( sprintf('User submitted number ("%s"), which is a "%s", so this is not a valid source.', $accountNumber, $search->accountType->type) ); $result = false; } if (null !== $search && in_array($search->accountType->type, $validTypes, true)) { - app('log')->debug('Number result is not null and seems valid, save as source account.'); + Log::debug('Number result is not null and seems valid, save as source account.'); $this->setSource($search); $result = true; } diff --git a/app/Validation/Account/OBValidation.php b/app/Validation/Account/OBValidation.php index a3f32afeb0..b0633c63f9 100644 --- a/app/Validation/Account/OBValidation.php +++ b/app/Validation/Account/OBValidation.php @@ -27,6 +27,7 @@ namespace FireflyIII\Validation\Account; use FireflyIII\Enums\AccountTypeEnum; use FireflyIII\Models\Account; use FireflyIII\Models\AccountType; +use Illuminate\Support\Facades\Log; /** * Trait OBValidation @@ -38,7 +39,7 @@ trait OBValidation $result = null; $accountId = array_key_exists('id', $array) ? $array['id'] : null; $accountName = array_key_exists('name', $array) ? $array['name'] : null; - app('log')->debug('Now in validateOBDestination', $array); + Log::debug('Now in validateOBDestination', $array); // source can be any of the following types. $validTypes = $this->combinations[$this->transactionType][$this->source?->accountType->type] ?? []; @@ -46,12 +47,12 @@ trait OBValidation // if both values are NULL we return false, // because the destination of a deposit can't be created. $this->destError = (string) trans('validation.ob_dest_need_data'); - app('log')->error('Both values are NULL, cant create OB destination.'); + Log::error('Both values are NULL, cant create OB destination.'); $result = false; } // if the account can be created anyway we don't need to search. if (null === $result && true === $this->canCreateTypes($validTypes)) { - app('log')->debug('Can create some of these types, so return true.'); + Log::debug('Can create some of these types, so return true.'); $result = true; } @@ -59,17 +60,17 @@ trait OBValidation // otherwise try to find the account: $search = $this->findExistingAccount($validTypes, $array); if (null === $search) { - app('log')->debug('findExistingAccount() returned NULL, so the result is false.', $validTypes); + Log::debug('findExistingAccount() returned NULL, so the result is false.', $validTypes); $this->destError = (string) trans('validation.ob_dest_bad_data', ['id' => $accountId, 'name' => $accountName]); $result = false; } if (null !== $search) { - app('log')->debug(sprintf('findExistingAccount() returned #%d ("%s"), so the result is true.', $search->id, $search->name)); + Log::debug(sprintf('findExistingAccount() returned #%d ("%s"), so the result is true.', $search->id, $search->name)); $this->setDestination($search); $result = true; } } - app('log')->debug(sprintf('validateOBDestination(%d, "%s") will return %s', $accountId, $accountName, var_export($result, true))); + Log::debug(sprintf('validateOBDestination(%d, "%s") will return %s', $accountId, $accountName, var_export($result, true))); return $result; } @@ -84,7 +85,7 @@ trait OBValidation { $accountId = array_key_exists('id', $array) ? $array['id'] : null; $accountName = array_key_exists('name', $array) ? $array['name'] : null; - app('log')->debug('Now in validateOBSource', $array); + Log::debug('Now in validateOBSource', $array); $result = null; // source can be any of the following types. $validTypes = array_keys($this->combinations[$this->transactionType]); @@ -100,19 +101,19 @@ trait OBValidation // if the user submits an ID only but that ID is not of the correct type, // return false. if (null !== $accountId && null === $accountName) { - app('log')->debug('Source ID is not null, but name is null.'); + Log::debug('Source ID is not null, but name is null.'); $search = $this->accountRepository->find($accountId); // the source resulted in an account, but it's not of a valid type. if (null !== $search && !in_array($search->accountType->type, $validTypes, true)) { $message = sprintf('User submitted only an ID (#%d), which is a "%s", so this is not a valid source.', $accountId, $search->accountType->type); - app('log')->debug($message); + Log::debug($message); $this->sourceError = $message; $result = false; } // the source resulted in an account, AND it's of a valid type. if (null !== $search && in_array($search->accountType->type, $validTypes, true)) { - app('log')->debug(sprintf('Found account of correct type: #%d, "%s"', $search->id, $search->name)); + Log::debug(sprintf('Found account of correct type: #%d, "%s"', $search->id, $search->name)); $this->setSource($search); $result = true; } @@ -120,7 +121,7 @@ trait OBValidation // if the account can be created anyway we don't need to search. if (null === $result && true === $this->canCreateTypes($validTypes)) { - app('log')->debug('Result is still null.'); + Log::debug('Result is still null.'); $result = true; // set the source to be a (dummy) initial balance account. diff --git a/app/Validation/Account/WithdrawalValidation.php b/app/Validation/Account/WithdrawalValidation.php index 9456e4ecf9..ac2a06d825 100644 --- a/app/Validation/Account/WithdrawalValidation.php +++ b/app/Validation/Account/WithdrawalValidation.php @@ -26,6 +26,7 @@ namespace FireflyIII\Validation\Account; use FireflyIII\Enums\AccountTypeEnum; use FireflyIII\Models\Account; +use Illuminate\Support\Facades\Log; /** * Trait WithdrawalValidation @@ -37,14 +38,14 @@ trait WithdrawalValidation $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; - app('log')->debug('Now in validateGenericSource', $array); + Log::debug('Now in validateGenericSource', $array); // source can be any of the following types. $validTypes = [AccountTypeEnum::ASSET->value, AccountTypeEnum::REVENUE->value, AccountTypeEnum::LOAN->value, AccountTypeEnum::DEBT->value, AccountTypeEnum::MORTGAGE->value]; if (null === $accountId && null === $accountName && null === $accountIban && false === $this->canCreateTypes($validTypes)) { // if both values are NULL we return TRUE // because we assume the user doesn't want to submit / change anything. $this->sourceError = (string) trans('validation.withdrawal_source_need_data'); - app('log')->warning('[a] Not a valid source. Need more data.'); + Log::warning('[a] Not a valid source. Need more data.'); return false; } @@ -53,12 +54,12 @@ trait WithdrawalValidation $search = $this->findExistingAccount($validTypes, $array); if (null === $search) { $this->sourceError = (string) trans('validation.withdrawal_source_bad_data', ['id' => $accountId, 'name' => $accountName]); - app('log')->warning('Not a valid source. Cant find it.', $validTypes); + Log::warning('Not a valid source. Cant find it.', $validTypes); return false; } $this->setSource($search); - app('log')->debug('Valid source account!'); + Log::debug('Valid source account!'); return true; } @@ -73,10 +74,10 @@ trait WithdrawalValidation $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; - app('log')->debug('Now in validateWithdrawalDestination()', $array); + Log::debug('Now in validateWithdrawalDestination()', $array); // source can be any of the following types. $validTypes = $this->combinations[$this->transactionType][$this->source->accountType->type] ?? []; - app('log')->debug('Source type can be: ', $validTypes); + 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. @@ -86,7 +87,7 @@ trait WithdrawalValidation } // if there's an ID it must be of the "validTypes". - if (null !== $accountId && 0 !== $accountId) { + if (null !== $accountId && 0 !== $accountId && $accountId !== $this->source->id) { $found = $this->accountRepository->find($accountId); if (null !== $found) { $type = $found->accountType->type; @@ -104,7 +105,7 @@ trait WithdrawalValidation // 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'); + Log::debug('Check if there is not already an account with this IBAN'); // 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) { @@ -125,14 +126,14 @@ trait WithdrawalValidation $accountIban = array_key_exists('iban', $array) ? $array['iban'] : null; $accountNumber = array_key_exists('number', $array) ? $array['number'] : null; - app('log')->debug('Now in validateWithdrawalSource', $array); + Log::debug('Now in validateWithdrawalSource', $array); // source can be any of the following types. $validTypes = array_keys($this->combinations[$this->transactionType]); if (null === $accountId && null === $accountName && null === $accountNumber && null === $accountIban && false === $this->canCreateTypes($validTypes)) { // if both values are NULL we return false, // because the source of a withdrawal can't be created. $this->sourceError = (string) trans('validation.withdrawal_source_need_data'); - app('log')->warning('[b] Not a valid source. Need more data.'); + Log::warning('[b] Not a valid source. Need more data.'); return false; } @@ -141,12 +142,12 @@ trait WithdrawalValidation $search = $this->findExistingAccount($validTypes, $array); if (null === $search) { $this->sourceError = (string) trans('validation.withdrawal_source_bad_data', ['id' => $accountId, 'name' => $accountName]); - app('log')->warning('Not a valid source. Cant find it.', $validTypes); + Log::warning('Not a valid source. Cant find it.', $validTypes); return false; } $this->setSource($search); - app('log')->debug('Valid source account!'); + Log::debug('Valid source account!'); return true; } diff --git a/app/Validation/AccountValidator.php b/app/Validation/AccountValidator.php index 44c9ad9d7b..0605b82bf0 100644 --- a/app/Validation/AccountValidator.php +++ b/app/Validation/AccountValidator.php @@ -36,6 +36,7 @@ use FireflyIII\Validation\Account\OBValidation; use FireflyIII\Validation\Account\ReconciliationValidation; use FireflyIII\Validation\Account\TransferValidation; use FireflyIII\Validation\Account\WithdrawalValidation; +use Illuminate\Support\Facades\Log; /** * Class AccountValidator @@ -80,10 +81,10 @@ class AccountValidator public function setSource(?Account $account): void { if (!$account instanceof Account) { - app('log')->debug('AccountValidator source is set to NULL'); + Log::debug('AccountValidator source is set to NULL'); } if ($account instanceof Account) { - app('log')->debug(sprintf('AccountValidator source is set to #%d: "%s" (%s)', $account->id, $account->name, $account->accountType?->type)); + Log::debug(sprintf('AccountValidator source is set to #%d: "%s" (%s)', $account->id, $account->name, $account->accountType?->type)); } $this->source = $account; } @@ -91,17 +92,17 @@ class AccountValidator public function setDestination(?Account $account): void { if (!$account instanceof Account) { - app('log')->debug('AccountValidator destination is set to NULL'); + Log::debug('AccountValidator destination is set to NULL'); } if ($account instanceof Account) { - app('log')->debug(sprintf('AccountValidator destination is set to #%d: "%s" (%s)', $account->id, $account->name, $account->accountType->type)); + Log::debug(sprintf('AccountValidator destination is set to #%d: "%s" (%s)', $account->id, $account->name, $account->accountType->type)); } $this->destination = $account; } public function setTransactionType(string $transactionType): void { - app('log')->debug(sprintf('Transaction type for validator is now "%s".', ucfirst($transactionType))); + Log::debug(sprintf('Transaction type for validator is now "%s".', ucfirst($transactionType))); $this->transactionType = ucfirst($transactionType); } @@ -117,9 +118,9 @@ class AccountValidator public function validateDestination(array $array): bool { - app('log')->debug('Now in AccountValidator::validateDestination()', $array); + Log::debug('Now in AccountValidator::validateDestination()', $array); if (!$this->source instanceof Account) { - app('log')->error('Source is NULL, always FALSE.'); + Log::error('Source is NULL, always FALSE.'); $this->destError = 'No source account validation has taken place yet. Please do this first or overrule the object.'; return false; @@ -128,7 +129,7 @@ class AccountValidator switch ($this->transactionType) { default: $this->destError = sprintf('AccountValidator::validateDestination cannot handle "%s", so it will always return false.', $this->transactionType); - app('log')->error(sprintf('AccountValidator::validateDestination cannot handle "%s", so it will always return false.', $this->transactionType)); + Log::error(sprintf('AccountValidator::validateDestination cannot handle "%s", so it will always return false.', $this->transactionType)); $result = false; @@ -170,11 +171,11 @@ class AccountValidator public function validateSource(array $array): bool { - app('log')->debug('Now in AccountValidator::validateSource()', $array); + Log::debug('Now in AccountValidator::validateSource()', $array); switch ($this->transactionType) { default: - app('log')->error(sprintf('AccountValidator::validateSource cannot handle "%s", so it will do a generic check.', $this->transactionType)); + Log::error(sprintf('AccountValidator::validateSource cannot handle "%s", so it will do a generic check.', $this->transactionType)); $result = $this->validateGenericSource($array); break; @@ -205,7 +206,7 @@ class AccountValidator break; case TransactionTypeEnum::RECONCILIATION->value: - app('log')->debug('Calling validateReconciliationSource'); + Log::debug('Calling validateReconciliationSource'); $result = $this->validateReconciliationSource($array); break; @@ -216,17 +217,17 @@ class AccountValidator protected function canCreateTypes(array $accountTypes): bool { - app('log')->debug('Can we create any of these types?', $accountTypes); + Log::debug('Can we create any of these types?', $accountTypes); /** @var string $accountType */ foreach ($accountTypes as $accountType) { if ($this->canCreateType($accountType)) { - app('log')->debug(sprintf('YES, we can create a %s', $accountType)); + Log::debug(sprintf('YES, we can create a %s', $accountType)); return true; } } - app('log')->debug('NO, we cant create any of those.'); + Log::debug('NO, we cant create any of those.'); return false; } @@ -250,8 +251,8 @@ class AccountValidator */ protected function findExistingAccount(array $validTypes, array $data, bool $inverse = false): ?Account { - app('log')->debug('Now in findExistingAccount', [$validTypes, $data]); - app('log')->debug('The search will be reversed!'); + Log::debug('Now in findExistingAccount', [$validTypes, $data]); + 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 +265,7 @@ class AccountValidator $check = in_array($accountType, $validTypes, true); $check = $inverse ? !$check : $check; // reverse the validation check if necessary. if (($first instanceof Account) && $check) { - app('log')->debug(sprintf('ID: Found %s account #%d ("%s", IBAN "%s")', $first->accountType->type, $first->id, $first->name, $first->iban ?? 'no iban')); + Log::debug(sprintf('ID: Found %s account #%d ("%s", IBAN "%s")', $first->accountType->type, $first->id, $first->name, $first->iban ?? 'no iban')); return $first; } @@ -277,7 +278,7 @@ class AccountValidator $check = in_array($accountType, $validTypes, true); $check = $inverse ? !$check : $check; // reverse the validation check if necessary. if (($first instanceof Account) && $check) { - app('log')->debug(sprintf('Iban: Found %s account #%d ("%s", IBAN "%s")', $first->accountType->type, $first->id, $first->name, $first->iban ?? 'no iban')); + Log::debug(sprintf('Iban: Found %s account #%d ("%s", IBAN "%s")', $first->accountType->type, $first->id, $first->name, $first->iban ?? 'no iban')); return $first; } @@ -290,7 +291,7 @@ class AccountValidator $check = in_array($accountType, $validTypes, true); $check = $inverse ? !$check : $check; // reverse the validation check if necessary. if (($first instanceof Account) && $check) { - app('log')->debug(sprintf('Number: Found %s account #%d ("%s", IBAN "%s")', $first->accountType->type, $first->id, $first->name, $first->iban ?? 'no iban')); + Log::debug(sprintf('Number: Found %s account #%d ("%s", IBAN "%s")', $first->accountType->type, $first->id, $first->name, $first->iban ?? 'no iban')); return $first; } @@ -300,12 +301,12 @@ class AccountValidator if ('' !== (string) $accountName) { $first = $this->accountRepository->findByName($accountName, $validTypes); if ($first instanceof Account) { - app('log')->debug(sprintf('Name: Found %s account #%d ("%s", IBAN "%s")', $first->accountType->type, $first->id, $first->name, $first->iban ?? 'no iban')); + Log::debug(sprintf('Name: Found %s account #%d ("%s", IBAN "%s")', $first->accountType->type, $first->id, $first->name, $first->iban ?? 'no iban')); return $first; } } - app('log')->debug('Found nothing in findExistingAccount()'); + Log::debug('Found nothing in findExistingAccount()'); return null; } diff --git a/resources/lang/en_US/validation.php b/resources/lang/en_US/validation.php index 7c2134f5a2..f7b27c0da6 100644 --- a/resources/lang/en_US/validation.php +++ b/resources/lang/en_US/validation.php @@ -24,10 +24,10 @@ declare(strict_types=1); return [ - 'limit_exists' => 'There is already a budget limit (amount) for this budget and currency in the given period.', + 'limit_exists' => 'There is already a budget limit (amount) for this budget and currency in the given period.', 'invalid_sort_instruction' => 'The sort instruction is invalid for an object of type ":object".', - 'invalid_sort_instruction_index' => 'The sort instruction at index #:index is invalid for an object of type ":object".', - 'no_sort_instructions' => 'There are no sort instructions defined for an object of type ":object".', + 'invalid_sort_instruction_index' => 'The sort instruction at index #:index is invalid for an object of type ":object".', + 'no_sort_instructions' => 'There are no sort instructions defined for an object of type ":object".', 'webhook_budget_info' => 'Cannot deliver budget information for transaction related webhooks.', 'webhook_account_info' => 'Cannot deliver account information for budget related webhooks.', 'webhook_transaction_info' => 'Cannot deliver transaction information for budget related webhooks.', @@ -40,8 +40,8 @@ return [ 'nog_logged_in' => 'You are not logged in.', 'prohibited' => 'You must not submit anything in field.', 'bad_webhook_combination' => 'Webhook trigger ":trigger" cannot be combined with webhook response ":response".', - 'unknown_webhook_trigger' => 'Unknown webhook trigger ":trigger".', - 'only_any_trigger' => 'If you select the "Any event"-trigger, you may not select any other triggers.', + 'unknown_webhook_trigger' => 'Unknown webhook trigger ":trigger".', + 'only_any_trigger' => 'If you select the "Any event"-trigger, you may not select any other triggers.', 'bad_type_source' => 'Firefly III can\'t determine the transaction type based on this source account.', 'bad_type_destination' => 'Firefly III can\'t determine the transaction type based on this destination account.', 'missing_where' => 'Array is missing "where"-clause', @@ -123,7 +123,7 @@ return [ 'between.file' => 'The :attribute must be between :min and :max kilobytes.', 'between.string' => 'The :attribute must be between :min and :max characters.', 'between.array' => 'The :attribute must have between :min and :max items.', - 'between_date' => 'The date must be between the given start and end date.', + 'between_date' => 'The date must be between the given start and end date.', 'boolean' => 'The :attribute field must be true or false.', 'confirmed' => 'The :attribute confirmation does not match.', 'date' => 'The :attribute is not a valid date.',