From 9e4b7f8bb40e4ab9732e155df618a5aaadc1fbb7 Mon Sep 17 00:00:00 2001 From: James Cole Date: Tue, 31 Mar 2020 07:04:00 +0200 Subject: [PATCH] Add IBAN to account validator. --- app/Factory/TransactionJournalFactory.php | 6 ++--- .../Transaction/ConvertController.php | 6 ++--- app/Http/Requests/RecurrenceFormRequest.php | 4 ++-- app/Rules/IsTransferAccount.php | 4 ++-- .../Support/RecurringTransactionTrait.php | 4 ++-- .../Internal/Update/JournalUpdateService.php | 4 ++-- app/Validation/AccountValidator.php | 24 ++++++++++--------- app/Validation/RecurrenceValidation.php | 4 ++-- app/Validation/TransactionValidation.php | 14 ++++++----- 9 files changed, 37 insertions(+), 33 deletions(-) diff --git a/app/Factory/TransactionJournalFactory.php b/app/Factory/TransactionJournalFactory.php index 98034be969..0292bdb9ad 100644 --- a/app/Factory/TransactionJournalFactory.php +++ b/app/Factory/TransactionJournalFactory.php @@ -599,7 +599,7 @@ class TransactionJournalFactory // validate source account. $sourceId = isset($data['source_id']) ? (int) $data['source_id'] : null; $sourceName = $data['source_name'] ?? null; - $validSource = $this->accountValidator->validateSource($sourceId, $sourceName); + $validSource = $this->accountValidator->validateSource($sourceId, $sourceName, null); // do something with result: if (false === $validSource) { @@ -608,8 +608,8 @@ class TransactionJournalFactory Log::debug('Source seems valid.'); // validate destination account $destinationId = isset($data['destination_id']) ? (int) $data['destination_id'] : null; - $destinationName = $data['destination_name'] ?? null; - $validDestination = $this->accountValidator->validateDestination($destinationId, $destinationName); + $destinationName = (string)($data['destination_name'] ?? null); + $validDestination = $this->accountValidator->validateDestination($destinationId, $destinationName, null); // do something with result: if (false === $validDestination) { throw new FireflyException(sprintf('Destination: %s', $this->accountValidator->destError)); // @codeCoverageIgnore diff --git a/app/Http/Controllers/Transaction/ConvertController.php b/app/Http/Controllers/Transaction/ConvertController.php index 3befc15d74..b5254a3769 100644 --- a/app/Http/Controllers/Transaction/ConvertController.php +++ b/app/Http/Controllers/Transaction/ConvertController.php @@ -208,9 +208,9 @@ class ConvertController extends Controller $sourceId = '' === $sourceId || null === $sourceId ? null : (int) $sourceId; $sourceName = '' === $sourceName ? null : $sourceName; $destinationId = '' === $destinationId || null === $destinationId ? null : (int) $destinationId; - $destinationName = '' === $destinationName ? null : $destinationName; - $validSource = $validator->validateSource($sourceId, $sourceName); - $validDestination = $validator->validateDestination($destinationId, $destinationName); + $destinationName = (string)('' === $destinationName ? null : $destinationName); + $validSource = $validator->validateSource($sourceId, $sourceName, null); + $validDestination = $validator->validateDestination($destinationId, $destinationName, null); if (false === $validSource) { throw new FireflyException(sprintf(trans('firefly.convert_invalid_source'), $journal->id)); diff --git a/app/Http/Requests/RecurrenceFormRequest.php b/app/Http/Requests/RecurrenceFormRequest.php index b3345ddefb..857b18010c 100644 --- a/app/Http/Requests/RecurrenceFormRequest.php +++ b/app/Http/Requests/RecurrenceFormRequest.php @@ -265,7 +265,7 @@ class RecurrenceFormRequest extends Request // validate source account. - $validSource = $accountValidator->validateSource($sourceId, null); + $validSource = $accountValidator->validateSource($sourceId, null, null); // do something with result: if (false === $validSource) { @@ -277,7 +277,7 @@ class RecurrenceFormRequest extends Request } // validate destination account - $validDestination = $accountValidator->validateDestination($destinationId, null); + $validDestination = $accountValidator->validateDestination($destinationId, null, null); // do something with result: if (false === $validDestination) { $message = (string) trans('validation.generic_invalid_destination'); diff --git a/app/Rules/IsTransferAccount.php b/app/Rules/IsTransferAccount.php index aee892cae3..b47c261d6b 100644 --- a/app/Rules/IsTransferAccount.php +++ b/app/Rules/IsTransferAccount.php @@ -58,14 +58,14 @@ class IsTransferAccount implements Rule $validator->setTransactionType(TransactionType::TRANSFER); $validator->setUser(auth()->user()); - $validAccount = $validator->validateSource(null, (string)$value); + $validAccount = $validator->validateSource(null, (string)$value, null); if (true === $validAccount) { Log::debug('Found account based on name. Return true.'); // found by name, use repos to return. return true; } - $validAccount = $validator->validateSource((int)$value, null); + $validAccount = $validator->validateSource((int)$value, null, null); Log::debug(sprintf('Search by id (%d), result is %s.', (int)$value, var_export($validAccount, true))); return !(false === $validAccount); diff --git a/app/Services/Internal/Support/RecurringTransactionTrait.php b/app/Services/Internal/Support/RecurringTransactionTrait.php index cd6ccffca3..f1f15f8522 100644 --- a/app/Services/Internal/Support/RecurringTransactionTrait.php +++ b/app/Services/Internal/Support/RecurringTransactionTrait.php @@ -98,11 +98,11 @@ trait RecurringTransactionTrait $validator = app(AccountValidator::class); $validator->setUser($recurrence->user); $validator->setTransactionType($recurrence->transactionType->type); - if (!$validator->validateSource($source->id, null)) { + if (!$validator->validateSource($source->id, null, null)) { throw new FireflyException(sprintf('Source invalid: %s', $validator->sourceError)); // @codeCoverageIgnore } - if (!$validator->validateDestination($destination->id, null)) { + if (!$validator->validateDestination($destination->id, null, null)) { throw new FireflyException(sprintf('Destination invalid: %s', $validator->destError)); // @codeCoverageIgnore } diff --git a/app/Services/Internal/Update/JournalUpdateService.php b/app/Services/Internal/Update/JournalUpdateService.php index dee9a22552..d2343c50c1 100644 --- a/app/Services/Internal/Update/JournalUpdateService.php +++ b/app/Services/Internal/Update/JournalUpdateService.php @@ -359,7 +359,7 @@ class JournalUpdateService $validator->source = $this->getValidSourceAccount(); - $result = $validator->validateDestination($destId, $destName); + $result = $validator->validateDestination($destId, $destName, null); Log::debug(sprintf('hasValidDestinationAccount(%d, "%s") will return %s', $destId, $destName, var_export($result, true))); // validate submitted info: @@ -391,7 +391,7 @@ class JournalUpdateService $validator->setTransactionType($expectedType); $validator->setUser($this->transactionJournal->user); - $result = $validator->validateSource($sourceId, $sourceName); + $result = $validator->validateSource($sourceId, $sourceName, null); Log::debug(sprintf('hasValidSourceAccount(%d, "%s") will return %s', $sourceId, $sourceName, var_export($result, true))); // validate submitted info: diff --git a/app/Validation/AccountValidator.php b/app/Validation/AccountValidator.php index 9683921adb..f9b94e3b92 100644 --- a/app/Validation/AccountValidator.php +++ b/app/Validation/AccountValidator.php @@ -80,14 +80,15 @@ class AccountValidator } /** - * @param int|null $destinationId - * @param $destinationName + * @param int|null $accountId + * @param string|null $accountName + * @param string|null $accountIban * * @return bool */ - public function validateDestination(?int $destinationId, $destinationName): bool + public function validateDestination(?int $accountId, ?string $accountName, ?string $accountIban): bool { - Log::debug(sprintf('Now in AccountValidator::validateDestination(%d, "%s")', $destinationId, $destinationName)); + Log::debug(sprintf('Now in AccountValidator::validateDestination(%d, "%s", "%s")', $accountId, $accountName, $accountIban)); if (null === $this->source) { 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.'; @@ -103,19 +104,19 @@ class AccountValidator break; case TransactionType::WITHDRAWAL: - $result = $this->validateWithdrawalDestination($destinationId, $destinationName); + $result = $this->validateWithdrawalDestination($accountId, $accountName); break; case TransactionType::DEPOSIT: - $result = $this->validateDepositDestination($destinationId, $destinationName); + $result = $this->validateDepositDestination($accountId, $accountName); break; case TransactionType::TRANSFER: - $result = $this->validateTransferDestination($destinationId, $destinationName); + $result = $this->validateTransferDestination($accountId, $accountName); break; case TransactionType::OPENING_BALANCE: - $result = $this->validateOBDestination($destinationId, $destinationName); + $result = $this->validateOBDestination($accountId, $accountName); break; case TransactionType::RECONCILIATION: - $result = $this->validateReconciliationDestination($destinationId); + $result = $this->validateReconciliationDestination($accountId); break; } @@ -125,12 +126,13 @@ class AccountValidator /** * @param int|null $accountId * @param string|null $accountName + * @param string|null $accountIban * * @return bool */ - public function validateSource(?int $accountId, ?string $accountName): bool + public function validateSource(?int $accountId, ?string $accountName, ?string $accountIban): bool { - Log::debug(sprintf('Now in AccountValidator::validateSource(%d, "%s")', $accountId, $accountName)); + Log::debug(sprintf('Now in AccountValidator::validateSource(%d, "%s", "%s")', $accountId, $accountName, $accountIban)); switch ($this->transactionType) { default: $result = false; diff --git a/app/Validation/RecurrenceValidation.php b/app/Validation/RecurrenceValidation.php index ce63ccc5b0..f24da81b15 100644 --- a/app/Validation/RecurrenceValidation.php +++ b/app/Validation/RecurrenceValidation.php @@ -65,7 +65,7 @@ trait RecurrenceValidation // validate source account. $sourceId = isset($transaction['source_id']) ? (int)$transaction['source_id'] : null; $sourceName = $transaction['source_name'] ?? null; - $validSource = $accountValidator->validateSource($sourceId, $sourceName); + $validSource = $accountValidator->validateSource($sourceId, $sourceName, null); // do something with result: if (false === $validSource) { @@ -77,7 +77,7 @@ trait RecurrenceValidation // validate destination account $destinationId = isset($transaction['destination_id']) ? (int)$transaction['destination_id'] : null; $destinationName = $transaction['destination_name'] ?? null; - $validDestination = $accountValidator->validateDestination($destinationId, $destinationName); + $validDestination = $accountValidator->validateDestination($destinationId, $destinationName, null); // do something with result: if (false === $validDestination) { $validator->errors()->add(sprintf('transactions.%d.destination_id', $index), $accountValidator->destError); diff --git a/app/Validation/TransactionValidation.php b/app/Validation/TransactionValidation.php index db67a951c6..eaca941da1 100644 --- a/app/Validation/TransactionValidation.php +++ b/app/Validation/TransactionValidation.php @@ -72,9 +72,10 @@ trait TransactionValidation $accountValidator->setTransactionType($transactionType); // validate source account. - $sourceId = isset($transaction['source_id']) ? (int)$transaction['source_id'] : null; + $sourceId = isset($transaction['source_id']) ? (int) $transaction['source_id'] : null; $sourceName = $transaction['source_name'] ?? null; - $validSource = $accountValidator->validateSource($sourceId, $sourceName); + $sourceIban = $transaction['source_iban'] ?? null; + $validSource = $accountValidator->validateSource($sourceId, $sourceName, $sourceIban); // do something with result: if (false === $validSource) { @@ -84,9 +85,10 @@ trait TransactionValidation return; } // validate destination account - $destinationId = isset($transaction['destination_id']) ? (int)$transaction['destination_id'] : null; + $destinationId = isset($transaction['destination_id']) ? (int) $transaction['destination_id'] : null; $destinationName = $transaction['destination_name'] ?? null; - $validDestination = $accountValidator->validateDestination($destinationId, $destinationName); + $destinationIban = $transaction['destination_iban'] ?? null; + $validDestination = $accountValidator->validateDestination($destinationId, $destinationName, $destinationIban); // do something with result: if (false === $validDestination) { $validator->errors()->add(sprintf('transactions.%d.destination_id', $index), $accountValidator->destError); @@ -140,7 +142,7 @@ trait TransactionValidation // validate source account. $sourceId = isset($transaction['source_id']) ? (int)$transaction['source_id'] : $originalData['source_id']; $sourceName = $transaction['source_name'] ?? $originalData['source_name']; - $validSource = $accountValidator->validateSource($sourceId, $sourceName); + $validSource = $accountValidator->validateSource($sourceId, $sourceName, null); // do something with result: if (false === $validSource) { @@ -152,7 +154,7 @@ trait TransactionValidation // validate destination account $destinationId = isset($transaction['destination_id']) ? (int)$transaction['destination_id'] : $originalData['destination_id']; $destinationName = $transaction['destination_name'] ?? $originalData['destination_name']; - $validDestination = $accountValidator->validateDestination($destinationId, $destinationName); + $validDestination = $accountValidator->validateDestination($destinationId, $destinationName, null); // do something with result: if (false === $validDestination) { $validator->errors()->add(sprintf('transactions.%d.destination_id', $index), $accountValidator->destError);