From ce10036a2769929fb371a79314800232b2383154 Mon Sep 17 00:00:00 2001 From: James Cole Date: Sat, 10 Mar 2018 09:39:49 +0100 Subject: [PATCH] Fix #1199 in API --- app/Api/V1/Requests/TransactionRequest.php | 77 +++++++++++++--------- 1 file changed, 45 insertions(+), 32 deletions(-) diff --git a/app/Api/V1/Requests/TransactionRequest.php b/app/Api/V1/Requests/TransactionRequest.php index 744ee8fb77..4fdf648e61 100644 --- a/app/Api/V1/Requests/TransactionRequest.php +++ b/app/Api/V1/Requests/TransactionRequest.php @@ -192,8 +192,10 @@ class TransactionRequest extends Request * @param null|string $accountName * @param string $idField * @param string $nameField + * + * @return null|Account */ - protected function assetAccountExists(Validator $validator, ?int $accountId, ?string $accountName, string $idField, string $nameField): void + protected function assetAccountExists(Validator $validator, ?int $accountId, ?string $accountName, string $idField, string $nameField): ?Account { $accountId = intval($accountId); @@ -202,7 +204,7 @@ class TransactionRequest extends Request if ($accountId < 1 && strlen($accountName) === 0) { $validator->errors()->add($idField, trans('validation.filled', ['attribute' => $idField])); - return; + return null; } // ID belongs to user and is asset account: /** @var AccountRepositoryInterface $repository */ @@ -215,19 +217,21 @@ class TransactionRequest extends Request if ($first->accountType->type !== AccountType::ASSET) { $validator->errors()->add($idField, trans('validation.belongs_user')); - return; + return null; } // we ignore the account name at this point. - return; + return $first; } $account = $repository->findByName($accountName, [AccountType::ASSET]); if (is_null($account)) { $validator->errors()->add($nameField, trans('validation.belongs_user')); + + return null; } - return; + return $account; } /** @@ -342,13 +346,16 @@ class TransactionRequest extends Request * @param int|null $accountId * @param null|string $accountName * @param string $idField + * + * @return null|Account */ - protected function opposingAccountExists(Validator $validator, string $type, ?int $accountId, ?string $accountName, string $idField): void { + protected function opposingAccountExists(Validator $validator, string $type, ?int $accountId, ?string $accountName, string $idField): ?Account + { $accountId = intval($accountId); $accountName = strval($accountName); // both empty? done! if ($accountId < 1 && strlen($accountName) === 0) { - return; + return null; } if ($accountId !== 0) { // ID belongs to user and is $type account: @@ -362,16 +369,16 @@ class TransactionRequest extends Request if ($first->accountType->type !== $type) { $validator->errors()->add($idField, trans('validation.belongs_user')); - return; + return null; } // we ignore the account name at this point. - return; + return $first; } } // not having an opposing account by this name is NOT a problem. - return; + return null; } /** @@ -395,41 +402,47 @@ class TransactionRequest extends Request $data['type'] = strtolower($transaction->transactionJournal->transactionType->type); } foreach ($transactions as $index => $transaction) { - $sourceId = isset($transaction['source_id']) ? intval($transaction['source_id']) : null; - $sourceName = $transaction['source_name'] ?? null; - $destinationId = isset($transaction['destination_id']) ? intval($transaction['destination_id']) : null; - $destinationName = $transaction['destination_name'] ?? null; - + $sourceId = isset($transaction['source_id']) ? intval($transaction['source_id']) : null; + $sourceName = $transaction['source_name'] ?? null; + $destinationId = isset($transaction['destination_id']) ? intval($transaction['destination_id']) : null; + $destinationName = $transaction['destination_name'] ?? null; + $sourceAccount = null; + $destinationAccount = null; switch ($data['type']) { case 'withdrawal': - $idField = 'transactions.' . $index . '.source_id'; - $nameField = 'transactions.' . $index . '.source_name'; - $this->assetAccountExists($validator, $sourceId, $sourceName, $idField, $nameField); - - $idField = 'transactions.' . $index . '.destination_id'; - $this->opposingAccountExists($validator, AccountType::EXPENSE, $destinationId, $destinationName, $idField); + $idField = 'transactions.' . $index . '.source_id'; + $nameField = 'transactions.' . $index . '.source_name'; + $sourceAccount = $this->assetAccountExists($validator, $sourceId, $sourceName, $idField, $nameField); + $idField = 'transactions.' . $index . '.destination_id'; + $destinationAccount = $this->opposingAccountExists($validator, AccountType::EXPENSE, $destinationId, $destinationName, $idField); break; case 'deposit': - $idField = 'transactions.' . $index . '.source_id'; - $this->opposingAccountExists($validator, AccountType::REVENUE, $sourceId, $sourceName, $idField); + $idField = 'transactions.' . $index . '.source_id'; + $sourceAccount = $this->opposingAccountExists($validator, AccountType::REVENUE, $sourceId, $sourceName, $idField); - $idField = 'transactions.' . $index . '.destination_id'; - $nameField = 'transactions.' . $index . '.destination_name'; - $this->assetAccountExists($validator, $destinationId, $destinationName, $idField, $nameField); + $idField = 'transactions.' . $index . '.destination_id'; + $nameField = 'transactions.' . $index . '.destination_name'; + $destinationAccount = $this->assetAccountExists($validator, $destinationId, $destinationName, $idField, $nameField); break; case 'transfer': - $idField = 'transactions.' . $index . '.source_id'; - $nameField = 'transactions.' . $index . '.source_name'; - $this->assetAccountExists($validator, $sourceId, $sourceName, $idField, $nameField); + $idField = 'transactions.' . $index . '.source_id'; + $nameField = 'transactions.' . $index . '.source_name'; + $sourceAccount = $this->assetAccountExists($validator, $sourceId, $sourceName, $idField, $nameField); - $idField = 'transactions.' . $index . '.destination_id'; - $nameField = 'transactions.' . $index . '.destination_name'; - $this->assetAccountExists($validator, $destinationId, $destinationName, $idField, $nameField); + $idField = 'transactions.' . $index . '.destination_id'; + $nameField = 'transactions.' . $index . '.destination_name'; + $destinationAccount = $this->assetAccountExists($validator, $destinationId, $destinationName, $idField, $nameField); break; default: throw new FireflyException(sprintf('The validator cannot handle transaction type "%s" in validateAccountInformation().', $data['type'])); } + // add some errors in case of same account submitted: + if (!is_null($sourceAccount) && !is_null($destinationAccount)) { + if ($sourceAccount->id === $destinationAccount->id) { + $validator->errors()->add($idField, trans('validation.source_equals_destination')); + } + } } }