diff --git a/app/Api/V1/Requests/TransactionStoreRequest.php b/app/Api/V1/Requests/TransactionStoreRequest.php index e8a99c8c85..66c8c8d9ef 100644 --- a/app/Api/V1/Requests/TransactionStoreRequest.php +++ b/app/Api/V1/Requests/TransactionStoreRequest.php @@ -167,23 +167,23 @@ class TransactionStoreRequest extends Request // all journals must have a description $this->validateDescriptions($validator); - // all transaction types must be equal: - $this->validateTransactionTypes($validator); + // all transaction types must be equal: + $this->validateTransactionTypes($validator); // validate foreign currency info $this->validateForeignCurrencyInformation($validator); - // validate all account info $this->validateAccountInformation($validator); - // make sure all splits have valid source + dest info - $this->validateSplitAccounts($validator); + // validate source/destination is equal, depending on the transaction journal type. + $this->validateEqualAccounts($validator); // the group must have a description if > 1 journal. $this->validateGroupDescription($validator); // TODO validate that the currency fits the source and/or destination account. + } ); } @@ -282,5 +282,4 @@ class TransactionStoreRequest extends Request return $return; } - } diff --git a/app/Api/V1/Requests/TransactionUpdateRequest.php b/app/Api/V1/Requests/TransactionUpdateRequest.php index 72ec3e6811..28b4465eca 100644 --- a/app/Api/V1/Requests/TransactionUpdateRequest.php +++ b/app/Api/V1/Requests/TransactionUpdateRequest.php @@ -240,6 +240,9 @@ class TransactionUpdateRequest extends Request // if type is set, source + destination info is mandatory. $this->validateAccountPresence($validator); + // validate source/destination is equal, depending on the transaction journal type. + $this->validateEqualAccountsForUpdate($validator, $transactionGroup); + // TODO validate that the currency fits the source and/or destination account. // all journals must have a description diff --git a/app/Validation/TransactionValidation.php b/app/Validation/TransactionValidation.php index de9b344ca7..5904f6ef8b 100644 --- a/app/Validation/TransactionValidation.php +++ b/app/Validation/TransactionValidation.php @@ -23,7 +23,6 @@ declare(strict_types=1); namespace FireflyIII\Validation; -use FireflyIII\Models\Transaction; use FireflyIII\Models\TransactionGroup; use Illuminate\Validation\Validator; @@ -32,23 +31,10 @@ use Illuminate\Validation\Validator; */ trait TransactionValidation { - /** - * If type is set, source + destination info is mandatory. - * - * @param Validator $validator - */ - protected function validateAccountPresence(Validator $validator): void - { - // TODO - } - - /** * Validates the given account information. Switches on given transaction type. * * @param Validator $validator - * - * @throws \FireflyIII\Exceptions\FireflyException */ public function validateAccountInformation(Validator $validator): void { @@ -86,7 +72,6 @@ trait TransactionValidation return; } - } } @@ -165,62 +150,6 @@ trait TransactionValidation } } - /** - * Make sure that all the splits accounts are valid in combination with each other. - * - * @param Validator $validator - */ - public function validateSplitAccounts(Validator $validator): void - { - $data = $validator->getData(); - $count = isset($data['transactions']) ? \count($data['transactions']) : 0; - if ($count < 2) { - return; - } - // this is pretty much impossible: - // @codeCoverageIgnoreStart - if (!isset($data['type'])) { - // the journal may exist in the request: - /** @var Transaction $transaction */ - $transaction = $this->route()->parameter('transaction'); - if (null === $transaction) { - return; - } - $data['type'] = strtolower($transaction->transactionJournal->transactionType->type); - } - // @codeCoverageIgnoreEnd - - // collect all source ID's and destination ID's, if present: - $sources = []; - $destinations = []; - - foreach ($data['transactions'] as $transaction) { - $sources[] = isset($transaction['source_id']) ? (int)$transaction['source_id'] : 0; - $destinations[] = isset($transaction['destination_id']) ? (int)$transaction['destination_id'] : 0; - } - $destinations = array_unique($destinations); - $sources = array_unique($sources); - // switch on type: - switch ($data['type']) { - case 'withdrawal': - if (\count($sources) > 1) { - $validator->errors()->add('transactions.0.source_id', (string)trans('validation.all_accounts_equal')); - } - break; - case 'deposit': - if (\count($destinations) > 1) { - $validator->errors()->add('transactions.0.destination_id', (string)trans('validation.all_accounts_equal')); - } - break; - case 'transfer': - if (\count($sources) > 1 || \count($destinations) > 1) { - $validator->errors()->add('transactions.0.source_id', (string)trans('validation.all_accounts_equal')); - $validator->errors()->add('transactions.0.destination_id', (string)trans('validation.all_accounts_equal')); - } - break; - } - } - /** * All types of splits must be equal. * @@ -267,6 +196,97 @@ trait TransactionValidation } } + /** + * If type is set, source + destination info is mandatory. + * + * @param Validator $validator + */ + protected function validateAccountPresence(Validator $validator): void + { + // TODO + } + + /** + * @param Validator $validator + */ + private function validateEqualAccounts(Validator $validator): void + { + $data = $validator->getData(); + $transactions = $data['transactions'] ?? []; + // needs to be split + if (count($transactions) < 2) { + return; + } + $type = $transactions[0]['type'] ?? 'withdrawal'; + $sources = []; + $dests = []; + foreach ($transactions as $transaction) { + $sources[] = sprintf('%d-%s', $transaction['source_id'] ?? 0, $transaction['source_name'] ?? ''); + $dests[] = sprintf('%d-%s', $transaction['destination_id'] ?? 0, $transaction['destination_name'] ?? ''); + } + $sources = array_unique($sources); + $dests = array_unique($dests); + switch ($type) { + case 'withdrawal': + if (count($sources) > 1) { + $validator->errors()->add('transactions.0.source_id', (string)trans('validation.all_accounts_equal')); + } + break; + case 'deposit': + if (count($dests) > 1) { + $validator->errors()->add('transactions.0.destination_id', (string)trans('validation.all_accounts_equal')); + } + break; + case'transfer': + if (count($sources) > 1 || count($dests) > 1) { + $validator->errors()->add('transactions.0.source_id', (string)trans('validation.all_accounts_equal')); + $validator->errors()->add('transactions.0.destination_id', (string)trans('validation.all_accounts_equal')); + } + break; + } + } + + /** + * @param Validator $validator + * @param TransactionGroup $transactionGroup + */ + private function validateEqualAccountsForUpdate(Validator $validator, TransactionGroup $transactionGroup): void + { + $data = $validator->getData(); + $transactions = $data['transactions'] ?? []; + // needs to be split + if (count($transactions) < 2) { + return; + } + $type = $transactions[0]['type'] ?? strtolower($transactionGroup->transactionJournals()->first()->transactionType->type); + $sources = []; + $dests = []; + foreach ($transactions as $transaction) { + $sources[] = sprintf('%d-%s', $transaction['source_id'] ?? 0, $transaction['source_name'] ?? ''); + $dests[] = sprintf('%d-%s', $transaction['destination_id'] ?? 0, $transaction['destination_name'] ?? ''); + } + $sources = array_unique($sources); + $dests = array_unique($dests); + switch ($type) { + case 'withdrawal': + if (count($sources) > 1) { + $validator->errors()->add('transactions.0.source_id', (string)trans('validation.all_accounts_equal')); + } + break; + case 'deposit': + if (count($dests) > 1) { + $validator->errors()->add('transactions.0.destination_id', (string)trans('validation.all_accounts_equal')); + } + break; + case'transfer': + if (count($sources) > 1 || count($dests) > 1) { + $validator->errors()->add('transactions.0.source_id', (string)trans('validation.all_accounts_equal')); + $validator->errors()->add('transactions.0.destination_id', (string)trans('validation.all_accounts_equal')); + } + break; + } + } + /** * @param Validator $validator * @param TransactionGroup $transactionGroup