mirror of
				https://github.com/firefly-iii/firefly-iii.git
				synced 2025-10-31 02:36:28 +00:00 
			
		
		
		
	Fix #7189
This commit is contained in:
		| @@ -70,7 +70,7 @@ trait DepositValidation | ||||
|             } | ||||
|             if (null !== $search) { | ||||
|                 Log::debug(sprintf('findExistingAccount() returned #%d ("%s"), so the result is true.', $search->id, $search->name)); | ||||
|                 $this->destination = $search; | ||||
|                 $this->setDestination($search); | ||||
|                 $result            = true; | ||||
|             } | ||||
|         } | ||||
| @@ -107,7 +107,7 @@ trait DepositValidation | ||||
|         $accountNumber = array_key_exists('number', $array) ? $array['number'] : null; | ||||
|         Log::debug('Now in validateDepositSource', $array); | ||||
|  | ||||
|         // null = we found nothing at all or didnt even search | ||||
|         // null = we found nothing at all or didn't even search | ||||
|         // false = invalid results | ||||
|         $result = null; | ||||
|  | ||||
| @@ -129,6 +129,11 @@ trait DepositValidation | ||||
|                 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 accepts ID #%d as valid account data.', $accountId)); | ||||
|             } | ||||
|             if (null !== $search && in_array($search->accountType->type, $validTypes, true)) { | ||||
|                 Log::debug('ID result is not null and seems valid, save as source account.'); | ||||
|                 $this->setSource($search); | ||||
|                 $result       = true; | ||||
|             } | ||||
|         } | ||||
|  | ||||
|         // if user submits an IBAN: | ||||
| @@ -138,10 +143,15 @@ trait DepositValidation | ||||
|                 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)) { | ||||
|                 Log::debug('IBAN result is not null and seems valid, save as source account.'); | ||||
|                 $this->setSource($search); | ||||
|                 $result       = true; | ||||
|             } | ||||
|         } | ||||
|  | ||||
|         // if user submits a number: | ||||
|         if (null !== $accountNumber) { | ||||
|         if (null !== $accountNumber && '' !== $accountNumber) { | ||||
|             $search = $this->accountRepository->findByAccountNumber($accountNumber, $validTypes); | ||||
|             if (null !== $search && !in_array($search->accountType->type, $validTypes, true)) { | ||||
|                 Log::debug( | ||||
| @@ -149,6 +159,11 @@ trait DepositValidation | ||||
|                 ); | ||||
|                 $result = false; | ||||
|             } | ||||
|             if (null !== $search && in_array($search->accountType->type, $validTypes, true)) { | ||||
|                 Log::debug('Number result is not null and seems valid, save as source account.'); | ||||
|                 $this->setSource($search); | ||||
|                 $result       = true; | ||||
|             } | ||||
|         } | ||||
|  | ||||
|         // if the account can be created anyway we don't need to search. | ||||
| @@ -159,7 +174,7 @@ trait DepositValidation | ||||
|             $account              = new Account(); | ||||
|             $accountType          = AccountType::whereType(AccountType::REVENUE)->first(); | ||||
|             $account->accountType = $accountType; | ||||
|             $this->source         = $account; | ||||
|             $this->setSource($account); | ||||
|         } | ||||
|  | ||||
|         return $result ?? false; | ||||
|   | ||||
| @@ -91,7 +91,7 @@ trait LiabilityValidation | ||||
|                 return false; | ||||
|             } | ||||
|             Log::debug(sprintf('Return true, found #%d ("%s")', $result->id, $result->name)); | ||||
|             $this->source = $result; | ||||
|             $this->setSource($result); | ||||
|             return true; | ||||
|         } | ||||
|  | ||||
| @@ -109,7 +109,7 @@ trait LiabilityValidation | ||||
|             $account              = new Account(); | ||||
|             $accountType          = AccountType::whereType(AccountType::LIABILITY_CREDIT)->first(); | ||||
|             $account->accountType = $accountType; | ||||
|             $this->source         = $account; | ||||
|             $this->setSource($account); | ||||
|         } | ||||
|  | ||||
|         return $result; | ||||
|   | ||||
| @@ -70,7 +70,7 @@ trait OBValidation | ||||
|             } | ||||
|             if (null !== $search) { | ||||
|                 Log::debug(sprintf('findExistingAccount() returned #%d ("%s"), so the result is true.', $search->id, $search->name)); | ||||
|                 $this->destination = $search; | ||||
|                 $this->setDestination($search); | ||||
|                 $result            = true; | ||||
|             } | ||||
|         } | ||||
| @@ -127,7 +127,7 @@ trait OBValidation | ||||
|             // the source resulted in an account, AND it's of a valid type. | ||||
|             if (null !== $search && in_array($search->accountType->type, $validTypes, true)) { | ||||
|                 Log::debug(sprintf('Found account of correct type: #%d, "%s"', $search->id, $search->name)); | ||||
|                 $this->source = $search; | ||||
|                 $this->setSource($search); | ||||
|                 $result       = true; | ||||
|             } | ||||
|         } | ||||
| @@ -141,7 +141,7 @@ trait OBValidation | ||||
|             $account              = new Account(); | ||||
|             $accountType          = AccountType::whereType(AccountType::INITIAL_BALANCE)->first(); | ||||
|             $account->accountType = $accountType; | ||||
|             $this->source         = $account; | ||||
|             $this->setSource($account); | ||||
|         } | ||||
|  | ||||
|         return $result ?? false; | ||||
|   | ||||
| @@ -64,7 +64,7 @@ trait ReconciliationValidation | ||||
|  | ||||
|             return false; | ||||
|         } | ||||
|         $this->source = $search; | ||||
|         $this->setSource($search); | ||||
|         Log::debug('Valid source account!'); | ||||
|  | ||||
|         return true; | ||||
| @@ -85,7 +85,7 @@ trait ReconciliationValidation | ||||
|         // source to the asset account that is the destination. | ||||
|         if (null === $accountId && null === $accountName) { | ||||
|             Log::debug('The source is valid because ID and name are NULL.'); | ||||
|             $this->source = new Account(); | ||||
|             $this->setSource(new Account()); | ||||
|             return true; | ||||
|         } | ||||
|  | ||||
| @@ -101,7 +101,7 @@ trait ReconciliationValidation | ||||
|  | ||||
|             return false; | ||||
|         } | ||||
|         $this->source = $search; | ||||
|         $this->setSource($search); | ||||
|         Log::debug('Valid source account!'); | ||||
|  | ||||
|         return true; | ||||
|   | ||||
| @@ -59,7 +59,7 @@ trait TransferValidation | ||||
|  | ||||
|             return false; | ||||
|         } | ||||
|         $this->destination = $search; | ||||
|         $this->setDestination($search); | ||||
|  | ||||
|         // must not be the same as the source account | ||||
|         if (null !== $this->source && $this->source->id === $this->destination->id) { | ||||
| @@ -116,7 +116,7 @@ trait TransferValidation | ||||
|  | ||||
|             return false; | ||||
|         } | ||||
|         $this->source = $search; | ||||
|         $this->setSource($search); | ||||
|         Log::debug('Valid source!'); | ||||
|  | ||||
|         return true; | ||||
|   | ||||
| @@ -61,7 +61,7 @@ trait WithdrawalValidation | ||||
|  | ||||
|             return false; | ||||
|         } | ||||
|         $this->source = $search; | ||||
|         $this->setSource($search); | ||||
|         Log::debug('Valid source account!'); | ||||
|  | ||||
|         return true; | ||||
| @@ -108,7 +108,7 @@ trait WithdrawalValidation | ||||
|             if (null !== $found) { | ||||
|                 $type = $found->accountType->type; | ||||
|                 if (in_array($type, $validTypes, true)) { | ||||
|                     $this->destination = $found; | ||||
|                     $this->setDestination($found); | ||||
|                     return true; | ||||
|                 } | ||||
|                 $this->destError = (string)trans('validation.withdrawal_dest_bad_data', ['id' => $accountId, 'name' => $accountName]); | ||||
| @@ -151,7 +151,7 @@ trait WithdrawalValidation | ||||
|  | ||||
|             return false; | ||||
|         } | ||||
|         $this->source = $search; | ||||
|         $this->setSource($search); | ||||
|         Log::debug('Valid source account!'); | ||||
|  | ||||
|         return true; | ||||
|   | ||||
| @@ -264,4 +264,34 @@ class AccountValidator | ||||
|  | ||||
|         return null; | ||||
|     } | ||||
|  | ||||
|     /** | ||||
|      * @param  Account|null  $account | ||||
|      */ | ||||
|     public function setDestination(?Account $account): void | ||||
|     { | ||||
|         if(null === $account) { | ||||
|             Log::debug('AccountValidator destination is set to NULL'); | ||||
|         } | ||||
|         if(null !== $account) { | ||||
|             Log::debug(sprintf('AccountValidator destination is set to #%d: "%s" (%s)', $account->id, $account->name, $account?->accountType->type)); | ||||
|         } | ||||
|         $this->destination = $account; | ||||
|     } | ||||
|  | ||||
|     /** | ||||
|      * @param  Account|null  $account | ||||
|      */ | ||||
|     public function setSource(?Account $account): void | ||||
|     { | ||||
|         if(null === $account) { | ||||
|             Log::debug('AccountValidator source is set to NULL'); | ||||
|         } | ||||
|         if(null !== $account) { | ||||
|             Log::debug(sprintf('AccountValidator source is set to #%d: "%s" (%s)', $account->id, $account->name, $account?->accountType->type)); | ||||
|         } | ||||
|         $this->source = $account; | ||||
|     } | ||||
|  | ||||
|  | ||||
| } | ||||
|   | ||||
| @@ -24,10 +24,12 @@ declare(strict_types=1); | ||||
| namespace FireflyIII\Validation; | ||||
|  | ||||
| use FireflyIII\Models\Account; | ||||
| use FireflyIII\Models\AccountType; | ||||
| use FireflyIII\Models\Transaction; | ||||
| use FireflyIII\Models\TransactionGroup; | ||||
| use FireflyIII\Models\TransactionJournal; | ||||
| use FireflyIII\Models\TransactionType; | ||||
| use FireflyIII\Repositories\Account\AccountRepositoryInterface; | ||||
| use Illuminate\Validation\Validator; | ||||
| use Log; | ||||
|  | ||||
| @@ -143,25 +145,133 @@ trait TransactionValidation | ||||
|         // sanity check for reconciliation accounts. They can't both be null. | ||||
|         $this->sanityCheckReconciliation($validator, $transactionType, $index, $source, $destination); | ||||
|  | ||||
|         // deposit and the source is a liability or an asset account with a different | ||||
|         // currency than submitted? then the foreign currency info must be present as well (and filled in). | ||||
|         if (0 === $validator->errors()->count() && null !== $accountValidator->source && TransactionType::DEPOSIT === ucfirst($transactionType)) { | ||||
|             $accountType = $accountValidator?->source?->accountType?->type; | ||||
|             if (in_array($accountType, config('firefly.valid_currency_account_types'), true) && !$this->hasForeignCurrencyInfo($transaction)) { | ||||
|                 $validator->errors()->add(sprintf('transactions.%d.foreign_amount', $index), (string)trans('validation.require_foreign_currency')); | ||||
|             } | ||||
|         // sanity check for currency information. | ||||
|         $this->sanityCheckForeignCurrency($validator, $accountValidator, $transaction, $transactionType, $index); | ||||
|     } | ||||
|  | ||||
|     /** | ||||
|      * TODO describe this method. | ||||
|      * @param  Validator  $validator | ||||
|      * @param  AccountValidator  $accountValidator | ||||
|      * @param  array  $transaction | ||||
|      * @param  string  $transactionType | ||||
|      * @param  int  $index | ||||
|      * @return void | ||||
|      */ | ||||
|     private function sanityCheckForeignCurrency( | ||||
|         Validator $validator, | ||||
|         AccountValidator $accountValidator, | ||||
|         array $transaction, | ||||
|         string $transactionType, | ||||
|         int $index | ||||
|     ): void { | ||||
|         Log::debug('Now in sanityCheckForeignCurrency()'); | ||||
|         if (0 !== $validator->errors()->count()) { | ||||
|             Log::debug('Already have errors, return'); | ||||
|             return; | ||||
|         } | ||||
|         if (null === $accountValidator->source) { | ||||
|             Log::debug('No source, return'); | ||||
|             return; | ||||
|         } | ||||
|         if (null === $accountValidator->destination) { | ||||
|             Log::debug('No destination, return'); | ||||
|             return; | ||||
|         } | ||||
|         $source      = $accountValidator->source; | ||||
|         $destination = $accountValidator->destination; | ||||
|  | ||||
|         Log::debug(sprintf('Source: #%d "%s (%s)"', $source->id, $source->name, $source->accountType->type)); | ||||
|         Log::debug(sprintf('Destination: #%d "%s" (%s)', $destination->id, $destination->name, $source->accountType->type)); | ||||
|  | ||||
|         if (!$this->isLiabilityOrAsset($source) || !$this->isLiabilityOrAsset($destination)) { | ||||
|             Log::debug('Any account must be liability or asset account to continue.'); | ||||
|             return; | ||||
|         } | ||||
|  | ||||
|         // withdrawal or transfer and the destination is a liability or an asset account with a different | ||||
|         // currency than submitted? then the foreign currency info must be present as well (and filled in). | ||||
|         if (0 === $validator->errors()->count() && null !== $accountValidator->destination && | ||||
|             (TransactionType::WITHDRAWAL === ucfirst($transactionType) || (TransactionType::TRANSFER === ucfirst($transactionType)))) { | ||||
|             $accountType = $accountValidator?->destination?->accountType?->type; | ||||
|             if (in_array($accountType, config('firefly.valid_currency_account_types'), true) && !$this->hasForeignCurrencyInfo($transaction)) { | ||||
|  | ||||
|         /** @var AccountRepositoryInterface $accountRepository */ | ||||
|         $accountRepository   = app(AccountRepositoryInterface::class); | ||||
|         $defaultCurrency     = app('amount')->getDefaultCurrency(); | ||||
|         $sourceCurrency      = $accountRepository->getAccountCurrency($source) ?? $defaultCurrency; | ||||
|         $destinationCurrency = $accountRepository->getAccountCurrency($destination) ?? $defaultCurrency; | ||||
|         // if both accounts have the same currency, continue. | ||||
|         if ($sourceCurrency->code === $destinationCurrency->code) { | ||||
|             Log::debug('Both accounts have the same currency, continue.'); | ||||
|             return; | ||||
|         } | ||||
|  | ||||
|         if (TransactionType::DEPOSIT === ucfirst($transactionType)) { | ||||
|             // use case: deposit from liability account to an asset account | ||||
|             // the foreign amount must be in the currency of the source | ||||
|             // the amount must be in the currency of the destination | ||||
|  | ||||
|             // no foreign currency information is present: | ||||
|             if (!$this->hasForeignCurrencyInfo($transaction)) { | ||||
|                 $validator->errors()->add(sprintf('transactions.%d.foreign_amount', $index), (string)trans('validation.require_foreign_currency')); | ||||
|                 return; | ||||
|             } | ||||
|  | ||||
|             // wrong currency information is present | ||||
|             $foreignCurrencyCode = $transaction['foreign_currency_code'] ?? false; | ||||
|             if ($foreignCurrencyCode !== $sourceCurrency->code) { | ||||
|                 $validator->errors()->add(sprintf('transactions.%d.foreign_currency_code', $index), (string)trans('validation.require_foreign_src')); | ||||
|                 return; | ||||
|             } | ||||
|         } | ||||
|         // account validator has a valid source and a valid destination | ||||
|         if (TransactionType::TRANSFER === ucfirst($transactionType) || TransactionType::WITHDRAWAL === ucfirst($transactionType)) { | ||||
|             // use case: withdrawal from asset account to a liability account. | ||||
|             // the foreign amount must be in the currency of the destination | ||||
|             // the amount must be in the currency of the source | ||||
|  | ||||
|             // use case: transfer between accounts with different currencies. | ||||
|             // the foreign amount must be in the currency of the destination | ||||
|             // the amount must be in the currency of the source | ||||
|  | ||||
|             // no foreign currency information is present: | ||||
|             if (!$this->hasForeignCurrencyInfo($transaction)) { | ||||
|                 $validator->errors()->add(sprintf('transactions.%d.foreign_amount', $index), (string)trans('validation.require_foreign_currency')); | ||||
|                 return; | ||||
|             } | ||||
|  | ||||
|             // wrong currency information is present | ||||
|             $foreignCurrencyCode = $transaction['foreign_currency_code'] ?? false; | ||||
|             if ($foreignCurrencyCode !== $destinationCurrency->code) { | ||||
|                 $validator->errors()->add(sprintf('transactions.%d.foreign_currency_code', $index), (string)trans('validation.require_foreign_dest')); | ||||
|             } | ||||
|         } | ||||
|     } | ||||
|  | ||||
|     /** | ||||
|      * @param  Account  $account | ||||
|      * @return bool | ||||
|      */ | ||||
|     private function isLiabilityOrAsset(Account $account): bool | ||||
|     { | ||||
|         return $this->isLiability($account) || $this->isAsset($account); | ||||
|     } | ||||
|  | ||||
|     /** | ||||
|      * @param  Account  $account | ||||
|      * @return bool | ||||
|      */ | ||||
|     private function isLiability(Account $account): bool | ||||
|     { | ||||
|         $type = $account->accountType?->type; | ||||
|         if (in_array($type, config('firefly.valid_liabilities'), true)) { | ||||
|             return true; | ||||
|         } | ||||
|         return false; | ||||
|     } | ||||
|  | ||||
|     /** | ||||
|      * @param  Account  $account | ||||
|      * @return bool | ||||
|      */ | ||||
|     private function isAsset(Account $account): bool | ||||
|     { | ||||
|         $type = $account->accountType?->type; | ||||
|         return $type === AccountType::ASSET; | ||||
|     } | ||||
|  | ||||
|     /** | ||||
|   | ||||
		Reference in New Issue
	
	Block a user