From 81d70bd8116d88b1a35b8d650bb99933e1d44727 Mon Sep 17 00:00:00 2001 From: James Cole Date: Thu, 5 Jul 2018 18:02:02 +0200 Subject: [PATCH] Clean up API code. --- app/Api/V1/Controllers/AccountController.php | 50 +-- .../V1/Controllers/AttachmentController.php | 1 + .../Controllers/AvailableBudgetController.php | 1 + app/Api/V1/Controllers/BudgetController.php | 1 + .../V1/Controllers/BudgetLimitController.php | 42 +- app/Api/V1/Controllers/CategoryController.php | 1 + .../Controllers/ConfigurationController.php | 18 +- app/Api/V1/Controllers/Controller.php | 12 +- app/Api/V1/Controllers/CurrencyController.php | 1 + .../CurrencyExchangeRateController.php | 15 +- .../V1/Controllers/JournalLinkController.php | 5 + app/Api/V1/Controllers/LinkTypeController.php | 1 + .../V1/Controllers/PiggyBankController.php | 1 + .../V1/Controllers/PreferenceController.php | 1 + .../V1/Controllers/RecurrenceController.php | 3 - .../V1/Controllers/TransactionController.php | 92 +--- app/Api/V1/Controllers/UserController.php | 1 + app/Api/V1/Requests/RecurrenceRequest.php | 405 +++-------------- app/Api/V1/Requests/Request.php | 4 + app/Api/V1/Requests/TransactionRequest.php | 418 ++---------------- app/Console/Commands/DecryptAttachment.php | 3 - app/Console/Commands/UpgradeDatabase.php | 12 - app/Console/Commands/VerifyDatabase.php | 2 - app/Repositories/Budget/BudgetRepository.php | 8 +- .../Budget/BudgetRepositoryInterface.php | 6 +- app/Validation/FireflyValidator.php | 13 + app/Validation/RecurrenceValidation.php | 194 ++++++++ app/Validation/TransactionValidation.php | 366 +++++++++++++++ 28 files changed, 755 insertions(+), 922 deletions(-) create mode 100644 app/Validation/RecurrenceValidation.php create mode 100644 app/Validation/TransactionValidation.php diff --git a/app/Api/V1/Controllers/AccountController.php b/app/Api/V1/Controllers/AccountController.php index f7bd0580f2..b7472c452b 100644 --- a/app/Api/V1/Controllers/AccountController.php +++ b/app/Api/V1/Controllers/AccountController.php @@ -42,6 +42,7 @@ use League\Fractal\Serializer\JsonApiSerializer; /** * Class AccountController + * @SuppressWarnings(PHPMD.CouplingBetweenObjects) */ class AccountController extends Controller { @@ -204,45 +205,16 @@ class AccountController extends Controller private function mapTypes(string $type): array { $types = [ - 'all' => [ - AccountType::DEFAULT, - AccountType::CASH, - AccountType::ASSET, - AccountType::EXPENSE, - AccountType::REVENUE, - AccountType::INITIAL_BALANCE, - AccountType::BENEFICIARY, - AccountType::IMPORT, - AccountType::RECONCILIATION, - AccountType::LOAN, - ], - 'asset' => [ - AccountType::DEFAULT, - AccountType::ASSET, - ], - 'cash' => [ - AccountType::CASH, - ], - 'expense' => [ - AccountType::EXPENSE, - AccountType::BENEFICIARY, - ], - 'revenue' => [ - AccountType::REVENUE, - ], - 'special' => [ - AccountType::CASH, - AccountType::INITIAL_BALANCE, - AccountType::IMPORT, - AccountType::RECONCILIATION, - AccountType::LOAN, - ], - 'hidden' => [ - AccountType::INITIAL_BALANCE, - AccountType::IMPORT, - AccountType::RECONCILIATION, - AccountType::LOAN, - ], + 'all' => [AccountType::DEFAULT, AccountType::CASH, AccountType::ASSET, AccountType::EXPENSE, AccountType::REVENUE, + AccountType::INITIAL_BALANCE, AccountType::BENEFICIARY, AccountType::IMPORT, AccountType::RECONCILIATION, + AccountType::LOAN,], + 'asset' => [AccountType::DEFAULT, AccountType::ASSET,], + 'cash' => [AccountType::CASH,], + 'expense' => [AccountType::EXPENSE, AccountType::BENEFICIARY,], + 'revenue' => [AccountType::REVENUE,], + 'special' => [AccountType::CASH, AccountType::INITIAL_BALANCE, AccountType::IMPORT, AccountType::RECONCILIATION, + AccountType::LOAN,], + 'hidden' => [AccountType::INITIAL_BALANCE, AccountType::IMPORT, AccountType::RECONCILIATION, AccountType::LOAN,], AccountType::DEFAULT => [AccountType::DEFAULT], AccountType::CASH => [AccountType::CASH], AccountType::ASSET => [AccountType::ASSET], diff --git a/app/Api/V1/Controllers/AttachmentController.php b/app/Api/V1/Controllers/AttachmentController.php index 8dae717b38..e2196d148e 100644 --- a/app/Api/V1/Controllers/AttachmentController.php +++ b/app/Api/V1/Controllers/AttachmentController.php @@ -42,6 +42,7 @@ use League\Fractal\Serializer\JsonApiSerializer; /** * Class AttachmentController + * @SuppressWarnings(PHPMD.CouplingBetweenObjects) */ class AttachmentController extends Controller { diff --git a/app/Api/V1/Controllers/AvailableBudgetController.php b/app/Api/V1/Controllers/AvailableBudgetController.php index 2e918cba8f..ace43d0daf 100644 --- a/app/Api/V1/Controllers/AvailableBudgetController.php +++ b/app/Api/V1/Controllers/AvailableBudgetController.php @@ -41,6 +41,7 @@ use League\Fractal\Serializer\JsonApiSerializer; /** * Class AvailableBudgetController + * @SuppressWarnings(PHPMD.CouplingBetweenObjects) */ class AvailableBudgetController extends Controller { diff --git a/app/Api/V1/Controllers/BudgetController.php b/app/Api/V1/Controllers/BudgetController.php index 3408ed4018..4fbb38a04e 100644 --- a/app/Api/V1/Controllers/BudgetController.php +++ b/app/Api/V1/Controllers/BudgetController.php @@ -40,6 +40,7 @@ use League\Fractal\Serializer\JsonApiSerializer; /** * Class BudgetController + * @SuppressWarnings(PHPMD.CouplingBetweenObjects) */ class BudgetController extends Controller { diff --git a/app/Api/V1/Controllers/BudgetLimitController.php b/app/Api/V1/Controllers/BudgetLimitController.php index e144158001..62e172fd3e 100644 --- a/app/Api/V1/Controllers/BudgetLimitController.php +++ b/app/Api/V1/Controllers/BudgetLimitController.php @@ -34,21 +34,18 @@ use Illuminate\Http\JsonResponse; use Illuminate\Http\Request; use Illuminate\Pagination\LengthAwarePaginator; use Illuminate\Support\Collection; -use InvalidArgumentException; use League\Fractal\Manager; use League\Fractal\Pagination\IlluminatePaginatorAdapter; use League\Fractal\Resource\Collection as FractalCollection; use League\Fractal\Resource\Item; use League\Fractal\Serializer\JsonApiSerializer; -use Log; /** * Class BudgetLimitController + * @SuppressWarnings(PHPMD.CouplingBetweenObjects) */ class BudgetLimitController extends Controller { - ///** @var CurrencyRepositoryInterface */ - //private $currencyRepository; /** @var BudgetRepositoryInterface */ private $repository; @@ -63,7 +60,6 @@ class BudgetLimitController extends Controller /** @var User $user */ $user = auth()->user(); $this->repository = app(BudgetRepositoryInterface::class); - //$this->currencyRepository = app(CurrencyRepositoryInterface::class); $this->repository->setUser($user); return $next($request); @@ -94,40 +90,21 @@ class BudgetLimitController extends Controller */ public function index(Request $request): JsonResponse { - // create some objects: $manager = new Manager; $baseUrl = $request->getSchemeAndHttpHost() . '/api/v1'; - // read budget from request $budgetId = (int)($request->get('budget_id') ?? 0); - $budget = null; - if ($budgetId > 0) { - $budget = $this->repository->findNull($budgetId); - } - // read start date from request - $start = null; - try { - $start = Carbon::createFromFormat('Y-m-d', $request->get('start')); - $this->parameters->set('start', $start->format('Y-m-d')); - } catch (InvalidArgumentException $e) { - Log::debug(sprintf('Could not parse start date "%s": %s', $request->get('start'), $e->getMessage())); - - } - - // read end date from request - $end = null; - try { - $end = Carbon::createFromFormat('Y-m-d', $request->get('end')); - $this->parameters->set('end', $end->format('Y-m-d')); - } catch (InvalidArgumentException $e) { - Log::debug(sprintf('Could not parse end date "%s": %s', $request->get('end'), $e->getMessage())); - } + $budget = $this->repository->findNull($budgetId); $this->parameters->set('budget_id', $budgetId); - // types to get, page size: + $start = Carbon::createFromFormat('Y-m-d', $request->get('start')); + $this->parameters->set('start', $start->format('Y-m-d')); + + $end = Carbon::createFromFormat('Y-m-d', $request->get('end')); + $this->parameters->set('end', $end->format('Y-m-d')); + $pageSize = (int)app('preferences')->getForUser(auth()->user(), 'listPageSize', 50)->data; - // get list of budget limits. Count it and split it. $collection = new Collection; if (null === $budget) { $collection = $this->repository->getAllBudgetLimits($start, $end); @@ -138,12 +115,9 @@ class BudgetLimitController extends Controller $count = $collection->count(); $budgetLimits = $collection->slice(($this->parameters->get('page') - 1) * $pageSize, $pageSize); - - // make paginator: $paginator = new LengthAwarePaginator($budgetLimits, $count, $pageSize, $this->parameters->get('page')); $paginator->setPath(route('api.v1.budget_limits.index') . $this->buildParams()); - // present to user. $manager->setSerializer(new JsonApiSerializer($baseUrl)); $resource = new FractalCollection($budgetLimits, new BudgetLimitTransformer($this->parameters), 'budget_limits'); $resource->setPaginator(new IlluminatePaginatorAdapter($paginator)); diff --git a/app/Api/V1/Controllers/CategoryController.php b/app/Api/V1/Controllers/CategoryController.php index 2b27c53d9d..b41cc34434 100644 --- a/app/Api/V1/Controllers/CategoryController.php +++ b/app/Api/V1/Controllers/CategoryController.php @@ -40,6 +40,7 @@ use League\Fractal\Serializer\JsonApiSerializer; /** * Class CategoryController + * @SuppressWarnings(PHPMD.CouplingBetweenObjects) */ class CategoryController extends Controller { diff --git a/app/Api/V1/Controllers/ConfigurationController.php b/app/Api/V1/Controllers/ConfigurationController.php index 1f63904db8..ab13f31ac8 100644 --- a/app/Api/V1/Controllers/ConfigurationController.php +++ b/app/Api/V1/Controllers/ConfigurationController.php @@ -49,6 +49,12 @@ class ConfigurationController extends Controller $this->middleware( function ($request, $next) { $this->repository = app(UserRepositoryInterface::class); + /** @var User $admin */ + $admin = auth()->user(); + + if (!$this->repository->hasRole($admin, 'owner')) { + throw new FireflyException('No access to method.'); // @codeCoverageIgnore + } return $next($request); } @@ -60,11 +66,6 @@ class ConfigurationController extends Controller */ public function index() { - /** @var User $admin */ - $admin = auth()->user(); - if (!$this->repository->hasRole($admin, 'owner')) { - throw new FireflyException('No access to method.'); // @codeCoverageIgnore - } $configData = $this->getConfigData(); return response()->json(['data' => $configData], 200)->header('Content-Type', 'application/vnd.api+json'); @@ -75,14 +76,10 @@ class ConfigurationController extends Controller * * @return JsonResponse * @throws FireflyException + * @SuppressWarnings(PHPMD.CyclomaticComplexity) */ public function update(Request $request): JsonResponse { - /** @var User $admin */ - $admin = auth()->user(); - if (!$this->repository->hasRole($admin, 'owner')) { - throw new FireflyException('No access to method.'); // @codeCoverageIgnore - } $name = $request->get('name'); $value = $request->get('value'); $valid = ['is_demo_site', 'permission_update_check', 'single_user_mode']; @@ -107,6 +104,7 @@ class ConfigurationController extends Controller /** * @return array + * @SuppressWarnings(PHPMD.CyclomaticComplexity) */ private function getConfigData(): array { diff --git a/app/Api/V1/Controllers/Controller.php b/app/Api/V1/Controllers/Controller.php index 3b5b3d23b0..1db59ff954 100644 --- a/app/Api/V1/Controllers/Controller.php +++ b/app/Api/V1/Controllers/Controller.php @@ -37,6 +37,7 @@ use Symfony\Component\HttpFoundation\ParameterBag; * Class Controller. * * @codeCoverageIgnore + * @SuppressWarnings(PHPMD.NumberOfChildren) */ class Controller extends BaseController { @@ -57,6 +58,8 @@ class Controller extends BaseController /** * @return string + * + * @SuppressWarnings(PHPMD.CyclomaticComplexity) */ protected function buildParams(): string { @@ -68,21 +71,18 @@ class Controller extends BaseController } if ($value instanceof Carbon) { $params[$key] = $value->format('Y-m-d'); + continue; } - if (!$value instanceof Carbon) { - $params[$key] = $value; - } + $params[$key] = $value; } $return .= http_build_query($params); - if (\strlen($return) === 1) { - return ''; - } return $return; } /** * @return ParameterBag + * @SuppressWarnings(PHPMD.CyclomaticComplexity) */ private function getParameters(): ParameterBag { diff --git a/app/Api/V1/Controllers/CurrencyController.php b/app/Api/V1/Controllers/CurrencyController.php index 8f276f85d7..e50324290a 100644 --- a/app/Api/V1/Controllers/CurrencyController.php +++ b/app/Api/V1/Controllers/CurrencyController.php @@ -42,6 +42,7 @@ use League\Fractal\Serializer\JsonApiSerializer; /** * Class CurrencyController + * @SuppressWarnings(PHPMD.CouplingBetweenObjects) */ class CurrencyController extends Controller { diff --git a/app/Api/V1/Controllers/CurrencyExchangeRateController.php b/app/Api/V1/Controllers/CurrencyExchangeRateController.php index 50e091a6f1..cc5a31fdce 100644 --- a/app/Api/V1/Controllers/CurrencyExchangeRateController.php +++ b/app/Api/V1/Controllers/CurrencyExchangeRateController.php @@ -31,11 +31,9 @@ use FireflyIII\Transformers\CurrencyExchangeRateTransformer; use FireflyIII\User; use Illuminate\Http\JsonResponse; use Illuminate\Http\Request; -use InvalidArgumentException; use League\Fractal\Manager; use League\Fractal\Resource\Item; use League\Fractal\Serializer\JsonApiSerializer; -use Log; /** * @@ -79,7 +77,6 @@ class CurrencyExchangeRateController extends Controller $baseUrl = $request->getSchemeAndHttpHost() . '/api/v1'; $manager->setSerializer(new JsonApiSerializer($baseUrl)); - // currencies $fromCurrency = $this->repository->findByCodeNull($request->get('from') ?? 'EUR'); $toCurrency = $this->repository->findByCodeNull($request->get('to') ?? 'USD'); @@ -90,19 +87,11 @@ class CurrencyExchangeRateController extends Controller throw new FireflyException('Unknown destination currency.'); } - $dateObj = new Carbon; - try { - $dateObj = Carbon::createFromFormat('Y-m-d', $request->get('date') ?? date('Y-m-d')); - } catch (InvalidArgumentException $e) { - Log::debug($e->getMessage()); - } - - + $dateObj = Carbon::createFromFormat('Y-m-d', $request->get('date') ?? date('Y-m-d')); $this->parameters->set('from', $fromCurrency->code); $this->parameters->set('to', $toCurrency->code); $this->parameters->set('date', $dateObj->format('Y-m-d')); - // get the exchange rate. $rate = $this->repository->getExchangeRate($fromCurrency, $toCurrency, $dateObj); if (null === $rate) { /** @var User $admin */ @@ -111,8 +100,6 @@ class CurrencyExchangeRateController extends Controller /** @var ExchangeRateInterface $service */ $service = app(ExchangeRateInterface::class); $service->setUser($admin); - - // get rate: $rate = $service->getRate($fromCurrency, $toCurrency, $dateObj); } diff --git a/app/Api/V1/Controllers/JournalLinkController.php b/app/Api/V1/Controllers/JournalLinkController.php index 0dde9742be..3c658c5cc3 100644 --- a/app/Api/V1/Controllers/JournalLinkController.php +++ b/app/Api/V1/Controllers/JournalLinkController.php @@ -39,6 +39,11 @@ use League\Fractal\Resource\Collection as FractalCollection; use League\Fractal\Resource\Item; use League\Fractal\Serializer\JsonApiSerializer; +/** + * + * Class JournalLinkController + * @SuppressWarnings(PHPMD.CouplingBetweenObjects) + */ class JournalLinkController extends Controller { /** @var JournalRepositoryInterface */ diff --git a/app/Api/V1/Controllers/LinkTypeController.php b/app/Api/V1/Controllers/LinkTypeController.php index 45bdab656d..d7808810ed 100644 --- a/app/Api/V1/Controllers/LinkTypeController.php +++ b/app/Api/V1/Controllers/LinkTypeController.php @@ -42,6 +42,7 @@ use League\Fractal\Serializer\JsonApiSerializer; /** * * Class LinkTypeController + * @SuppressWarnings(PHPMD.CouplingBetweenObjects) */ class LinkTypeController extends Controller { diff --git a/app/Api/V1/Controllers/PiggyBankController.php b/app/Api/V1/Controllers/PiggyBankController.php index b310ddaeac..b64a2128b8 100644 --- a/app/Api/V1/Controllers/PiggyBankController.php +++ b/app/Api/V1/Controllers/PiggyBankController.php @@ -41,6 +41,7 @@ use League\Fractal\Serializer\JsonApiSerializer; /** * TODO order up and down. * Class PiggyBankController + * @SuppressWarnings(PHPMD.CouplingBetweenObjects) */ class PiggyBankController extends Controller { diff --git a/app/Api/V1/Controllers/PreferenceController.php b/app/Api/V1/Controllers/PreferenceController.php index 7f302e3b9f..043fb146ab 100644 --- a/app/Api/V1/Controllers/PreferenceController.php +++ b/app/Api/V1/Controllers/PreferenceController.php @@ -106,6 +106,7 @@ class PreferenceController extends Controller * @param Preference $preference * * @return JsonResponse + * @SuppressWarnings(PHPMD.CyclomaticComplexity) */ public function update(PreferenceRequest $request, Preference $preference): JsonResponse { diff --git a/app/Api/V1/Controllers/RecurrenceController.php b/app/Api/V1/Controllers/RecurrenceController.php index f9bf45ff13..5c23d377e3 100644 --- a/app/Api/V1/Controllers/RecurrenceController.php +++ b/app/Api/V1/Controllers/RecurrenceController.php @@ -163,9 +163,6 @@ class RecurrenceController extends Controller public function update(RecurrenceRequest $request, Recurrence $recurrence): JsonResponse { $data = $request->getAll(); - - // - $category = $this->repository->update($recurrence, $data); $manager = new Manager(); $baseUrl = $request->getSchemeAndHttpHost() . '/api/v1'; diff --git a/app/Api/V1/Controllers/TransactionController.php b/app/Api/V1/Controllers/TransactionController.php index aebec3ad5e..cb36c610a5 100644 --- a/app/Api/V1/Controllers/TransactionController.php +++ b/app/Api/V1/Controllers/TransactionController.php @@ -42,10 +42,10 @@ use League\Fractal\Manager; use League\Fractal\Pagination\IlluminatePaginatorAdapter; use League\Fractal\Resource\Collection as FractalCollection; use League\Fractal\Serializer\JsonApiSerializer; -use Log; /** * Class TransactionController + * @SuppressWarnings(PHPMD.CouplingBetweenObjects) */ class TransactionController extends Controller { @@ -96,19 +96,14 @@ class TransactionController extends Controller public function index(Request $request): JsonResponse { $pageSize = (int)app('preferences')->getForUser(auth()->user(), 'listPageSize', 50)->data; - - // read type from URI - $type = $request->get('type') ?? 'default'; + $type = $request->get('type') ?? 'default'; $this->parameters->set('type', $type); - // types to get, page size: - $types = $this->mapTypes($this->parameters->get('type')); - + $types = $this->mapTypes($this->parameters->get('type')); $manager = new Manager(); $baseUrl = $request->getSchemeAndHttpHost() . '/api/v1'; $manager->setSerializer(new JsonApiSerializer($baseUrl)); - // collect transactions using the journal collector /** @var User $admin */ $admin = auth()->user(); /** @var JournalCollectorInterface $collector */ @@ -117,7 +112,6 @@ class TransactionController extends Controller $collector->withOpposingAccount()->withCategoryInformation()->withBudgetInformation(); $collector->setAllAssetAccounts(); - // remove internal transfer filter: if (\in_array(TransactionType::TRANSFER, $types, true)) { $collector->removeFilter(InternalTransferFilter::class); } @@ -131,7 +125,6 @@ class TransactionController extends Controller $paginator->setPath(route('api.v1.transactions.index') . $this->buildParams()); $transactions = $paginator->getCollection(); - $resource = new FractalCollection($transactions, new TransactionTransformer($this->parameters), 'transactions'); $resource->setPaginator(new IlluminatePaginatorAdapter($paginator)); @@ -235,13 +228,9 @@ class TransactionController extends Controller { $data = $request->getAll(); $data['user'] = auth()->user()->id; - - Log::debug('Inside transaction update'); - - $journal = $repository->update($transaction->transactionJournal, $data); - - $manager = new Manager(); - $baseUrl = $request->getSchemeAndHttpHost() . '/api/v1'; + $journal = $repository->update($transaction->transactionJournal, $data); + $manager = new Manager(); + $baseUrl = $request->getSchemeAndHttpHost() . '/api/v1'; $manager->setSerializer(new JsonApiSerializer($baseUrl)); // add include parameter: @@ -280,59 +269,22 @@ class TransactionController extends Controller private function mapTypes(string $type): array { $types = [ - 'all' => [ - TransactionType::WITHDRAWAL, - TransactionType::DEPOSIT, - TransactionType::TRANSFER, - TransactionType::OPENING_BALANCE, - TransactionType::RECONCILIATION, - ], - 'withdrawal' => [ - TransactionType::WITHDRAWAL, - ], - 'withdrawals' => [ - TransactionType::WITHDRAWAL, - ], - 'expense' => [ - TransactionType::WITHDRAWAL, - ], - 'income' => [ - TransactionType::DEPOSIT, - ], - 'deposit' => [ - TransactionType::DEPOSIT, - ], - 'deposits' => [ - TransactionType::DEPOSIT, - ], - 'transfer' => [ - TransactionType::TRANSFER, - ], - 'transfers' => [ - TransactionType::TRANSFER, - ], - 'opening_balance' => [ - TransactionType::OPENING_BALANCE, - ], - 'reconciliation' => [ - TransactionType::RECONCILIATION, - ], - 'reconciliations' => [ - TransactionType::RECONCILIATION, - ], - 'special' => [ - TransactionType::OPENING_BALANCE, - TransactionType::RECONCILIATION, - ], - 'specials' => [ - TransactionType::OPENING_BALANCE, - TransactionType::RECONCILIATION, - ], - 'default' => [ - TransactionType::WITHDRAWAL, - TransactionType::DEPOSIT, - TransactionType::TRANSFER, - ], + 'all' => [TransactionType::WITHDRAWAL, TransactionType::DEPOSIT, TransactionType::TRANSFER, TransactionType::OPENING_BALANCE, + TransactionType::RECONCILIATION,], + 'withdrawal' => [TransactionType::WITHDRAWAL,], + 'withdrawals' => [TransactionType::WITHDRAWAL,], + 'expense' => [TransactionType::WITHDRAWAL,], + 'income' => [TransactionType::DEPOSIT,], + 'deposit' => [TransactionType::DEPOSIT,], + 'deposits' => [TransactionType::DEPOSIT,], + 'transfer' => [TransactionType::TRANSFER,], + 'transfers' => [TransactionType::TRANSFER,], + 'opening_balance' => [TransactionType::OPENING_BALANCE,], + 'reconciliation' => [TransactionType::RECONCILIATION,], + 'reconciliations' => [TransactionType::RECONCILIATION,], + 'special' => [TransactionType::OPENING_BALANCE, TransactionType::RECONCILIATION,], + 'specials' => [TransactionType::OPENING_BALANCE, TransactionType::RECONCILIATION,], + 'default' => [TransactionType::WITHDRAWAL, TransactionType::DEPOSIT, TransactionType::TRANSFER,], ]; if (isset($types[$type])) { return $types[$type]; diff --git a/app/Api/V1/Controllers/UserController.php b/app/Api/V1/Controllers/UserController.php index 5d3ed4a552..7871401633 100644 --- a/app/Api/V1/Controllers/UserController.php +++ b/app/Api/V1/Controllers/UserController.php @@ -41,6 +41,7 @@ use League\Fractal\Serializer\JsonApiSerializer; /** * Class UserController + * @SuppressWarnings(PHPMD.CouplingBetweenObjects) */ class UserController extends Controller { diff --git a/app/Api/V1/Requests/RecurrenceRequest.php b/app/Api/V1/Requests/RecurrenceRequest.php index 688681021a..032c5d4960 100644 --- a/app/Api/V1/Requests/RecurrenceRequest.php +++ b/app/Api/V1/Requests/RecurrenceRequest.php @@ -24,20 +24,18 @@ declare(strict_types=1); namespace FireflyIII\Api\V1\Requests; use Carbon\Carbon; -use FireflyIII\Models\Account; -use FireflyIII\Models\AccountType; -use FireflyIII\Repositories\Account\AccountRepositoryInterface; use FireflyIII\Rules\BelongsUser; -use FireflyIII\User; +use FireflyIII\Validation\RecurrenceValidation; +use FireflyIII\Validation\TransactionValidation; use Illuminate\Validation\Validator; -use InvalidArgumentException; -use Log; /** * Class RecurrenceRequest */ class RecurrenceRequest extends Request { + use RecurrenceValidation, TransactionValidation; + /** * @return bool */ @@ -68,51 +66,10 @@ class RecurrenceRequest extends Request 'piggy_bank_name' => $this->string('piggy_bank_name'), 'tags' => explode(',', $this->string('tags')), ], - 'transactions' => [], - 'repetitions' => [], + 'transactions' => $this->getTransactionData(), + 'repetitions' => $this->getRepetitionData(), ]; - // repetition data: - /** @var array $repetitions */ - $repetitions = $this->get('repetitions'); - /** @var array $repetition */ - foreach ($repetitions as $repetition) { - $return['repetitions'][] = [ - 'type' => $repetition['type'], - 'moment' => $repetition['moment'], - 'skip' => (int)$repetition['skip'], - 'weekend' => (int)$repetition['weekend'], - ]; - } - // transaction data: - /** @var array $transactions */ - $transactions = $this->get('transactions'); - /** @var array $transaction */ - foreach ($transactions as $transaction) { - $return['transactions'][] = [ - 'amount' => $transaction['amount'], - - 'currency_id' => isset($transaction['currency_id']) ? (int)$transaction['currency_id'] : null, - 'currency_code' => $transaction['currency_code'] ?? null, - - 'foreign_amount' => $transaction['foreign_amount'] ?? null, - 'foreign_currency_id' => isset($transaction['foreign_currency_id']) ? (int)$transaction['foreign_currency_id'] : null, - 'foreign_currency_code' => $transaction['foreign_currency_code'] ?? null, - - 'budget_id' => isset($transaction['budget_id']) ? (int)$transaction['budget_id'] : null, - 'budget_name' => $transaction['budget_name'] ?? null, - 'category_id' => isset($transaction['category_id']) ? (int)$transaction['category_id'] : null, - 'category_name' => $transaction['category_name'] ?? null, - - 'source_id' => isset($transaction['source_id']) ? (int)$transaction['source_id'] : null, - 'source_name' => isset($transaction['source_name']) ? (string)$transaction['source_name'] : null, - 'destination_id' => isset($transaction['destination_id']) ? (int)$transaction['destination_id'] : null, - 'destination_name' => isset($transaction['destination_name']) ? (string)$transaction['destination_name'] : null, - - 'description' => $transaction['description'], - ]; - } - return $return; } @@ -133,18 +90,12 @@ class RecurrenceRequest extends Request 'nr_of_repetitions' => 'numeric|between:1,31', 'apply_rules' => 'required|boolean', 'active' => 'required|boolean', - - // rules for meta values: 'tags' => 'between:1,64000', 'piggy_bank_id' => 'numeric', - - // rules for repetitions. 'repetitions.*.type' => 'required|in:daily,weekly,ndom,monthly,yearly', 'repetitions.*.moment' => 'between:0,10', 'repetitions.*.skip' => 'required|numeric|between:0,31', 'repetitions.*.weekend' => 'required|numeric|min:1|max:4', - - // rules for transactions. 'transactions.*.currency_id' => 'numeric|exists:transaction_currencies,id|required_without:transactions.*.currency_code', 'transactions.*.currency_code' => 'min:3|max:3|exists:transaction_currencies,code|required_without:transactions.*.currency_id', 'transactions.*.foreign_amount' => 'numeric|more:0', @@ -172,316 +123,76 @@ class RecurrenceRequest extends Request { $validator->after( function (Validator $validator) { - $this->atLeastOneTransaction($validator); - $this->atLeastOneRepetition($validator); - $this->validRepeatsUntil($validator); - $this->validRepetitionMoment($validator); - $this->foreignCurrencyInformation($validator); + $this->validateOneTransaction($validator); + $this->validateOneRepetition($validator); + $this->validateRecurrenceRepetition($validator); + $this->validateRepetitionMoment($validator); + $this->validateForeignCurrencyInformation($validator); $this->validateAccountInformation($validator); } ); } + /** - * Throws an error when this asset account is invalid. + * Returns the repetition data as it is found in the submitted data. * - * @noinspection MoreThanThreeArgumentsInspection - * - * @param Validator $validator - * @param int|null $accountId - * @param null|string $accountName - * @param string $idField - * @param string $nameField - * - * @return null|Account + * @return array */ - protected function assetAccountExists(Validator $validator, ?int $accountId, ?string $accountName, string $idField, string $nameField): ?Account + private function getRepetitionData(): array { - /** @var User $admin */ - $admin = auth()->user(); - $accountId = (int)$accountId; - $accountName = (string)$accountName; - // both empty? hard exit. - if ($accountId < 1 && '' === $accountName) { - $validator->errors()->add($idField, trans('validation.filled', ['attribute' => $idField])); - - return null; - } - // ID belongs to user and is asset account: - /** @var AccountRepositoryInterface $repository */ - $repository = app(AccountRepositoryInterface::class); - $repository->setUser($admin); - $set = $repository->getAccountsById([$accountId]); - Log::debug(sprintf('Count of accounts found by ID %d is: %d', $accountId, $set->count())); - if ($set->count() === 1) { - /** @var Account $first */ - $first = $set->first(); - if ($first->accountType->type !== AccountType::ASSET) { - $validator->errors()->add($idField, trans('validation.belongs_user')); - - return null; - } - - // we ignore the account name at this point. - return $first; + $return = []; + // repetition data: + /** @var array $repetitions */ + $repetitions = $this->get('repetitions'); + /** @var array $repetition */ + foreach ($repetitions as $repetition) { + $return[] = [ + 'type' => $repetition['type'], + 'moment' => $repetition['moment'], + 'skip' => (int)$repetition['skip'], + 'weekend' => (int)$repetition['weekend'], + ]; } - $account = $repository->findByName($accountName, [AccountType::ASSET]); - if (null === $account) { - $validator->errors()->add($nameField, trans('validation.belongs_user')); - - return null; - } - - return $account; + return $return; } /** - * Adds an error to the validator when there are no repetitions in the array of data. + * Returns the transaction data as it is found in the submitted data. It's a complex method according to code + * standards but it just has a lot of ??-statements because of the fields that may or may not exist. * - * @param Validator $validator + * @return array + * @SuppressWarnings(PHPMD.NPathComplexity) + * @SuppressWarnings(PHPMD.CyclomaticComplexity) */ - protected function atLeastOneRepetition(Validator $validator): void + private function getTransactionData(): array { - $data = $validator->getData(); - $repetitions = $data['repetitions'] ?? []; - // need at least one transaction - if (\count($repetitions) === 0) { - $validator->errors()->add('description', trans('validation.at_least_one_repetition')); - } - } - - /** - * Adds an error to the validator when there are no transactions in the array of data. - * - * @param Validator $validator - */ - protected function atLeastOneTransaction(Validator $validator): void - { - $data = $validator->getData(); - $transactions = $data['transactions'] ?? []; - // need at least one transaction - if (\count($transactions) === 0) { - $validator->errors()->add('description', trans('validation.at_least_one_transaction')); - } - } - - /** - * TODO can be made a rule? - * If the transactions contain foreign amounts, there must also be foreign currency information. - * - * @param Validator $validator - */ - protected function foreignCurrencyInformation(Validator $validator): void - { - $data = $validator->getData(); - $transactions = $data['transactions'] ?? []; - foreach ($transactions as $index => $transaction) { - // must have currency info. - if (isset($transaction['foreign_amount']) - && !(isset($transaction['foreign_currency_id']) - || isset($transaction['foreign_currency_code']))) { - $validator->errors()->add( - 'transactions.' . $index . '.foreign_amount', - trans('validation.require_currency_info') - ); - } - } - } - - /** - * Throws an error when the given opposing account (of type $type) is invalid. - * Empty data is allowed, system will default to cash. - * - * @noinspection MoreThanThreeArgumentsInspection - * - * @param Validator $validator - * @param string $type - * @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): ?Account - { - /** @var User $admin */ - $admin = auth()->user(); - $accountId = (int)$accountId; - $accountName = (string)$accountName; - // both empty? done! - if ($accountId < 1 && '' === $accountName) { - return null; - } - if ($accountId !== 0) { - // ID belongs to user and is $type account: - /** @var AccountRepositoryInterface $repository */ - $repository = app(AccountRepositoryInterface::class); - $repository->setUser($admin); - $set = $repository->getAccountsById([$accountId]); - if ($set->count() === 1) { - /** @var Account $first */ - $first = $set->first(); - if ($first->accountType->type !== $type) { - $validator->errors()->add($idField, trans('validation.belongs_user')); - - return null; - } - - // we ignore the account name at this point. - return $first; - } + $return = []; + // transaction data: + /** @var array $transactions */ + $transactions = $this->get('transactions'); + /** @var array $transaction */ + foreach ($transactions as $transaction) { + $return[] = [ + 'amount' => $transaction['amount'], + 'currency_id' => isset($transaction['currency_id']) ? (int)$transaction['currency_id'] : null, + 'currency_code' => $transaction['currency_code'] ?? null, + 'foreign_amount' => $transaction['foreign_amount'] ?? null, + 'foreign_currency_id' => isset($transaction['foreign_currency_id']) ? (int)$transaction['foreign_currency_id'] : null, + 'foreign_currency_code' => $transaction['foreign_currency_code'] ?? null, + 'budget_id' => isset($transaction['budget_id']) ? (int)$transaction['budget_id'] : null, + 'budget_name' => $transaction['budget_name'] ?? null, + 'category_id' => isset($transaction['category_id']) ? (int)$transaction['category_id'] : null, + 'category_name' => $transaction['category_name'] ?? null, + 'source_id' => isset($transaction['source_id']) ? (int)$transaction['source_id'] : null, + 'source_name' => isset($transaction['source_name']) ? (string)$transaction['source_name'] : null, + 'destination_id' => isset($transaction['destination_id']) ? (int)$transaction['destination_id'] : null, + 'destination_name' => isset($transaction['destination_name']) ? (string)$transaction['destination_name'] : null, + 'description' => $transaction['description'], + ]; } - // not having an opposing account by this name is NOT a problem. - return null; - } - - /** - * TODO can be a rule? - * - * Validates the given account information. Switches on given transaction type. - * - * @param Validator $validator - */ - protected function validateAccountInformation(Validator $validator): void - { - $data = $validator->getData(); - $transactions = $data['transactions'] ?? []; - $idField = 'description'; - $transactionType = $data['type'] ?? 'false'; - foreach ($transactions as $index => $transaction) { - $sourceId = isset($transaction['source_id']) ? (int)$transaction['source_id'] : null; - $sourceName = $transaction['source_name'] ?? null; - $destinationId = isset($transaction['destination_id']) ? (int)$transaction['destination_id'] : null; - $destinationName = $transaction['destination_name'] ?? null; - $sourceAccount = null; - $destinationAccount = null; - switch ($transactionType) { - case 'withdrawal': - $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'; - $sourceAccount = $this->opposingAccountExists($validator, AccountType::REVENUE, $sourceId, $sourceName, $idField); - - $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'; - $sourceAccount = $this->assetAccountExists($validator, $sourceId, $sourceName, $idField, $nameField); - - $idField = 'transactions.' . $index . '.destination_id'; - $nameField = 'transactions.' . $index . '.destination_name'; - $destinationAccount = $this->assetAccountExists($validator, $destinationId, $destinationName, $idField, $nameField); - break; - default: - $validator->errors()->add($idField, trans('validation.invalid_account_info')); - - return; - - } - // add some errors in case of same account submitted: - if (null !== $sourceAccount && null !== $destinationAccount && $sourceAccount->id === $destinationAccount->id) { - $validator->errors()->add($idField, trans('validation.source_equals_destination')); - } - } - } - - /** - * @param Validator $validator - */ - private function validRepeatsUntil(Validator $validator): void - { - $data = $validator->getData(); - $repetitions = $data['nr_of_repetitions'] ?? null; - $repeatUntil = $data['repeat_until'] ?? null; - if (null !== $repetitions && null !== $repeatUntil) { - // expect a date OR count: - $validator->errors()->add('repeat_until', trans('validation.require_repeat_until')); - $validator->errors()->add('nr_of_repetitions', trans('validation.require_repeat_until')); - - return; - } - } - - /** - * TODO merge this in a rule somehow. - * - * @param Validator $validator - */ - private function validRepetitionMoment(Validator $validator): void - { - $data = $validator->getData(); - $repetitions = $data['repetitions'] ?? []; - /** - * @var int $index - * @var array $repetition - */ - foreach ($repetitions as $index => $repetition) { - switch ($repetition['type']) { - default: - $validator->errors()->add(sprintf('repetitions.%d.type', $index), trans('validation.valid_recurrence_rep_type')); - - return; - case 'daily': - if ('' !== (string)$repetition['moment']) { - $validator->errors()->add(sprintf('repetitions.%d.moment', $index), trans('validation.valid_recurrence_rep_moment')); - } - - return; - case 'monthly': - $dayOfMonth = (int)$repetition['moment']; - if ($dayOfMonth < 1 || $dayOfMonth > 31) { - $validator->errors()->add(sprintf('repetitions.%d.moment', $index), trans('validation.valid_recurrence_rep_moment')); - } - - return; - case 'ndom': - $parameters = explode(',', $repetition['moment']); - if (\count($parameters) !== 2) { - $validator->errors()->add(sprintf('repetitions.%d.moment', $index), trans('validation.valid_recurrence_rep_moment')); - - return; - } - $nthDay = (int)($parameters[0] ?? 0.0); - $dayOfWeek = (int)($parameters[1] ?? 0.0); - if ($nthDay < 1 || $nthDay > 5) { - $validator->errors()->add(sprintf('repetitions.%d.moment', $index), trans('validation.valid_recurrence_rep_moment')); - - return; - } - if ($dayOfWeek < 1 || $dayOfWeek > 7) { - $validator->errors()->add(sprintf('repetitions.%d.moment', $index), trans('validation.valid_recurrence_rep_moment')); - - return; - } - - return; - case 'weekly': - $dayOfWeek = (int)$repetition['moment']; - if ($dayOfWeek < 1 || $dayOfWeek > 7) { - $validator->errors()->add(sprintf('repetitions.%d.moment', $index), trans('validation.valid_recurrence_rep_moment')); - - return; - } - break; - case 'yearly': - try { - Carbon::createFromFormat('Y-m-d', $repetition['moment']); - } catch (InvalidArgumentException $e) { - $validator->errors()->add(sprintf('repetitions.%d.moment', $index), trans('validation.valid_recurrence_rep_moment')); - - return; - } - } - } + return $return; } } \ No newline at end of file diff --git a/app/Api/V1/Requests/Request.php b/app/Api/V1/Requests/Request.php index e2016dba34..416818471c 100644 --- a/app/Api/V1/Requests/Request.php +++ b/app/Api/V1/Requests/Request.php @@ -28,6 +28,10 @@ use FireflyIII\Http\Requests\Request as FireflyIIIRequest; /** * Class Request. + * + * Technically speaking this class does not have to be extended like this but who knows what the future brings. + * + * @SuppressWarnings(PHPMD.NumberOfChildren) */ class Request extends FireflyIIIRequest { diff --git a/app/Api/V1/Requests/TransactionRequest.php b/app/Api/V1/Requests/TransactionRequest.php index 2bd6ed2dbb..57c94b33c3 100644 --- a/app/Api/V1/Requests/TransactionRequest.php +++ b/app/Api/V1/Requests/TransactionRequest.php @@ -24,13 +24,8 @@ declare(strict_types=1); namespace FireflyIII\Api\V1\Requests; -use FireflyIII\Exceptions\FireflyException; -use FireflyIII\Models\Account; -use FireflyIII\Models\AccountType; -use FireflyIII\Models\Transaction; -use FireflyIII\Repositories\Account\AccountRepositoryInterface; use FireflyIII\Rules\BelongsUser; -use FireflyIII\User; +use FireflyIII\Validation\TransactionValidation; use Illuminate\Validation\Validator; @@ -39,6 +34,8 @@ use Illuminate\Validation\Validator; */ class TransactionRequest extends Request { + use TransactionValidation; + /** * @return bool */ @@ -49,12 +46,15 @@ class TransactionRequest extends Request } /** + * Get all data. Is pretty complex because of all the ??-statements. + * + * @SuppressWarnings(PHPMD.CyclomaticComplexity) + * @SuppressWarnings(PHPMD.NPathComplexity) * @return array */ public function getAll(): array { $data = [ - // basic fields for journal: 'type' => $this->string('type'), 'date' => $this->date('date'), 'description' => $this->string('description'), @@ -63,8 +63,6 @@ class TransactionRequest extends Request 'bill_id' => $this->integer('bill_id'), 'bill_name' => $this->string('bill_name'), 'tags' => explode(',', $this->string('tags')), - - // then, custom fields for journal 'interest_date' => $this->date('interest_date'), 'book_date' => $this->date('book_date'), 'process_date' => $this->date('process_date'), @@ -73,39 +71,15 @@ class TransactionRequest extends Request 'invoice_date' => $this->date('invoice_date'), 'internal_reference' => $this->string('internal_reference'), 'notes' => $this->string('notes'), - - // then, transactions (see below). - 'transactions' => [], - + 'transactions' => $this->getTransactionData(), ]; - foreach ($this->get('transactions') as $index => $transaction) { - $array = [ - 'description' => $transaction['description'] ?? null, - 'amount' => $transaction['amount'], - 'currency_id' => isset($transaction['currency_id']) ? (int)$transaction['currency_id'] : null, - 'currency_code' => $transaction['currency_code'] ?? null, - 'foreign_amount' => $transaction['foreign_amount'] ?? null, - 'foreign_currency_id' => isset($transaction['foreign_currency_id']) ? (int)$transaction['foreign_currency_id'] : null, - 'foreign_currency_code' => $transaction['foreign_currency_code'] ?? null, - 'budget_id' => isset($transaction['budget_id']) ? (int)$transaction['budget_id'] : null, - 'budget_name' => $transaction['budget_name'] ?? null, - 'category_id' => isset($transaction['category_id']) ? (int)$transaction['category_id'] : null, - 'category_name' => $transaction['category_name'] ?? null, - 'source_id' => isset($transaction['source_id']) ? (int)$transaction['source_id'] : null, - 'source_name' => isset($transaction['source_name']) ? (string)$transaction['source_name'] : null, - 'destination_id' => isset($transaction['destination_id']) ? (int)$transaction['destination_id'] : null, - 'destination_name' => isset($transaction['destination_name']) ? (string)$transaction['destination_name'] : null, - 'reconciled' => $transaction['reconciled'] ?? false, - 'identifier' => $index, - ]; - $data['transactions'][] = $array; - } return $data; } /** * @return array + * @SuppressWarnings(PHPMD.ExcessiveMethodLength) */ public function rules(): array { @@ -150,13 +124,8 @@ class TransactionRequest extends Request 'transactions.*.destination_name' => 'between:1,255|nullable', ]; - switch ($this->method()) { - default: - break; - case 'PUT': - case 'PATCH': - unset($rules['type'], $rules['piggy_bank_id'], $rules['piggy_bank_name']); - break; + if ($this->method() === 'PUT') { + unset($rules['type'], $rules['piggy_bank_id'], $rules['piggy_bank_name']); } return $rules; @@ -175,11 +144,11 @@ class TransactionRequest extends Request { $validator->after( function (Validator $validator) { - $this->atLeastOneTransaction($validator); - $this->checkValidDescriptions($validator); - $this->equalToJournalDescription($validator); - $this->emptySplitDescriptions($validator); - $this->foreignCurrencyInformation($validator); + $this->validateOneTransaction($validator); + $this->validateDescriptions($validator); + $this->validateJournalDescription($validator); + $this->validateSplitDescriptions($validator); + $this->validateForeignCurrencyInformation($validator); $this->validateAccountInformation($validator); $this->validateSplitAccounts($validator); } @@ -187,338 +156,35 @@ class TransactionRequest extends Request } /** - * Throws an error when this asset account is invalid. - * - * @noinspection MoreThanThreeArgumentsInspection - * - * @param Validator $validator - * @param int|null $accountId - * @param null|string $accountName - * @param string $idField - * @param string $nameField - * - * @return null|Account + * @SuppressWarnings(PHPMD.CyclomaticComplexity) + * @SuppressWarnings(PHPMD.NPathComplexity) + * @return array */ - protected function assetAccountExists(Validator $validator, ?int $accountId, ?string $accountName, string $idField, string $nameField): ?Account + private function getTransactionData(): array { - /** @var User $admin */ - $admin = auth()->user(); - $accountId = (int)$accountId; - $accountName = (string)$accountName; - // both empty? hard exit. - if ($accountId < 1 && '' === $accountName) { - $validator->errors()->add($idField, trans('validation.filled', ['attribute' => $idField])); - - return null; - } - // ID belongs to user and is asset account: - /** @var AccountRepositoryInterface $repository */ - $repository = app(AccountRepositoryInterface::class); - $repository->setUser($admin); - $set = $repository->getAccountsById([$accountId]); - if ($set->count() === 1) { - /** @var Account $first */ - $first = $set->first(); - if ($first->accountType->type !== AccountType::ASSET) { - $validator->errors()->add($idField, trans('validation.belongs_user')); - - return null; - } - - // we ignore the account name at this point. - return $first; + $return = []; + foreach ($this->get('transactions') as $index => $transaction) { + $return[] = [ + 'description' => $transaction['description'] ?? null, + 'amount' => $transaction['amount'], + 'currency_id' => isset($transaction['currency_id']) ? (int)$transaction['currency_id'] : null, + 'currency_code' => $transaction['currency_code'] ?? null, + 'foreign_amount' => $transaction['foreign_amount'] ?? null, + 'foreign_currency_id' => isset($transaction['foreign_currency_id']) ? (int)$transaction['foreign_currency_id'] : null, + 'foreign_currency_code' => $transaction['foreign_currency_code'] ?? null, + 'budget_id' => isset($transaction['budget_id']) ? (int)$transaction['budget_id'] : null, + 'budget_name' => $transaction['budget_name'] ?? null, + 'category_id' => isset($transaction['category_id']) ? (int)$transaction['category_id'] : null, + 'category_name' => $transaction['category_name'] ?? null, + 'source_id' => isset($transaction['source_id']) ? (int)$transaction['source_id'] : null, + 'source_name' => isset($transaction['source_name']) ? (string)$transaction['source_name'] : null, + 'destination_id' => isset($transaction['destination_id']) ? (int)$transaction['destination_id'] : null, + 'destination_name' => isset($transaction['destination_name']) ? (string)$transaction['destination_name'] : null, + 'reconciled' => $transaction['reconciled'] ?? false, + 'identifier' => $index, + ]; } - $account = $repository->findByName($accountName, [AccountType::ASSET]); - if (null === $account) { - $validator->errors()->add($nameField, trans('validation.belongs_user')); - - return null; - } - - return $account; + return $return; } - - /** - * Adds an error to the validator when there are no transactions in the array of data. - * - * @param Validator $validator - */ - protected function atLeastOneTransaction(Validator $validator): void - { - $data = $validator->getData(); - $transactions = $data['transactions'] ?? []; - // need at least one transaction - if (\count($transactions) === 0) { - $validator->errors()->add('description', trans('validation.at_least_one_transaction')); - } - } - - /** - * Adds an error to the "description" field when the user has submitted no descriptions and no - * journal description. - * - * @param Validator $validator - */ - protected function checkValidDescriptions(Validator $validator): void - { - $data = $validator->getData(); - $transactions = $data['transactions'] ?? []; - $journalDescription = (string)($data['description'] ?? ''); - $validDescriptions = 0; - foreach ($transactions as $index => $transaction) { - if (\strlen((string)($transaction['description'] ?? '')) > 0) { - $validDescriptions++; - } - } - - // no valid descriptions and empty journal description? error. - if ($validDescriptions === 0 && '' === $journalDescription) { - $validator->errors()->add('description', trans('validation.filled', ['attribute' => trans('validation.attributes.description')])); - } - - } - - /** - * Adds an error to the validator when the user submits a split transaction (more than 1 transactions) - * but does not give them a description. - * - * @param Validator $validator - */ - protected function emptySplitDescriptions(Validator $validator): void - { - $data = $validator->getData(); - $transactions = $data['transactions'] ?? []; - foreach ($transactions as $index => $transaction) { - $description = (string)($transaction['description'] ?? ''); - // filled description is mandatory for split transactions. - if ('' === $description && \count($transactions) > 1) { - $validator->errors()->add( - 'transactions.' . $index . '.description', - trans('validation.filled', ['attribute' => trans('validation.attributes.transaction_description')]) - ); - } - } - } - - /** - * Adds an error to the validator when any transaction descriptions are equal to the journal description. - * - * @param Validator $validator - */ - protected function equalToJournalDescription(Validator $validator): void - { - $data = $validator->getData(); - $transactions = $data['transactions'] ?? []; - $journalDescription = (string)($data['description'] ?? ''); - foreach ($transactions as $index => $transaction) { - $description = (string)($transaction['description'] ?? ''); - // description cannot be equal to journal description. - if ($description === $journalDescription) { - $validator->errors()->add('transactions.' . $index . '.description', trans('validation.equal_description')); - } - } - } - - /** - * TODO can be made a rule? - * - * If the transactions contain foreign amounts, there must also be foreign currency information. - * - * @param Validator $validator - */ - protected function foreignCurrencyInformation(Validator $validator): void - { - $data = $validator->getData(); - $transactions = $data['transactions'] ?? []; - foreach ($transactions as $index => $transaction) { - // must have currency info. - if (isset($transaction['foreign_amount']) - && !(isset($transaction['foreign_currency_id']) - || isset($transaction['foreign_currency_code']))) { - $validator->errors()->add( - 'transactions.' . $index . '.foreign_amount', - trans('validation.require_currency_info') - ); - } - } - } - - /** - * Throws an error when the given opposing account (of type $type) is invalid. - * Empty data is allowed, system will default to cash. - * - * @noinspection MoreThanThreeArgumentsInspection - * - * @param Validator $validator - * @param string $type - * @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): ?Account - { - /** @var User $admin */ - $admin = auth()->user(); - $accountId = (int)$accountId; - $accountName = (string)$accountName; - // both empty? done! - if ($accountId < 1 && '' === $accountName) { - return null; - } - if ($accountId !== 0) { - // ID belongs to user and is $type account: - /** @var AccountRepositoryInterface $repository */ - $repository = app(AccountRepositoryInterface::class); - $repository->setUser($admin); - $set = $repository->getAccountsById([$accountId]); - if ($set->count() === 1) { - /** @var Account $first */ - $first = $set->first(); - if ($first->accountType->type !== $type) { - $validator->errors()->add($idField, trans('validation.belongs_user')); - - return null; - } - - // we ignore the account name at this point. - return $first; - } - } - - // not having an opposing account by this name is NOT a problem. - return null; - } - - /** - * Validates the given account information. Switches on given transaction type. - * - * @param Validator $validator - * - * @throws FireflyException - */ - protected function validateAccountInformation(Validator $validator): void - { - $data = $validator->getData(); - $transactions = $data['transactions'] ?? []; - if (!isset($data['type'])) { - // the journal may exist in the request: - /** @var Transaction $transaction */ - $transaction = $this->route()->parameter('transaction'); - if (null === $transaction) { - return; // @codeCoverageIgnore - } - $data['type'] = strtolower($transaction->transactionJournal->transactionType->type); - } - foreach ($transactions as $index => $transaction) { - $sourceId = isset($transaction['source_id']) ? (int)$transaction['source_id'] : null; - $sourceName = $transaction['source_name'] ?? null; - $destinationId = isset($transaction['destination_id']) ? (int)$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'; - $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'; - $sourceAccount = $this->opposingAccountExists($validator, AccountType::REVENUE, $sourceId, $sourceName, $idField); - - $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'; - $sourceAccount = $this->assetAccountExists($validator, $sourceId, $sourceName, $idField, $nameField); - - $idField = 'transactions.' . $index . '.destination_id'; - $nameField = 'transactions.' . $index . '.destination_name'; - $destinationAccount = $this->assetAccountExists($validator, $destinationId, $destinationName, $idField, $nameField); - break; - default: - // @codeCoverageIgnoreStart - throw new FireflyException( - sprintf('The validator cannot handle transaction type "%s" in validateAccountInformation().', $data['type']) - ); - // @codeCoverageIgnoreEnd - - } - // add some errors in case of same account submitted: - if (null !== $sourceAccount && null !== $destinationAccount && $sourceAccount->id === $destinationAccount->id) { - $validator->errors()->add($idField, trans('validation.source_equals_destination')); - } - } - } - - /** - * @param Validator $validator - * - * @throws FireflyException - */ - protected 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', trans('validation.all_accounts_equal')); - } - break; - case 'deposit': - if (\count($destinations) > 1) { - $validator->errors()->add('transactions.0.destination_id', trans('validation.all_accounts_equal')); - } - break; - case 'transfer': - if (\count($sources) > 1 || \count($destinations) > 1) { - $validator->errors()->add('transactions.0.source_id', trans('validation.all_accounts_equal')); - $validator->errors()->add('transactions.0.destination_id', trans('validation.all_accounts_equal')); - } - break; - default: - // @codeCoverageIgnoreStart - throw new FireflyException( - sprintf('The validator cannot handle transaction type "%s" in validateSplitAccounts().', $data['type']) - ); - // @codeCoverageIgnoreEnd - } - } - } diff --git a/app/Console/Commands/DecryptAttachment.php b/app/Console/Commands/DecryptAttachment.php index dca003e8b9..ede989fcbd 100644 --- a/app/Console/Commands/DecryptAttachment.php +++ b/app/Console/Commands/DecryptAttachment.php @@ -51,9 +51,6 @@ class DecryptAttachment extends Command /** * Execute the console command. - * - * @SuppressWarnings(PHPMD.CyclomaticComplexity) // it's five its fine. - * @SuppressWarnings(PHPMD.ExcessiveMethodLength) */ public function handle() { diff --git a/app/Console/Commands/UpgradeDatabase.php b/app/Console/Commands/UpgradeDatabase.php index a533aedf2f..ff7a45a9cc 100644 --- a/app/Console/Commands/UpgradeDatabase.php +++ b/app/Console/Commands/UpgradeDatabase.php @@ -55,9 +55,6 @@ use Schema; * Class UpgradeDatabase. * * Upgrade user database. - * - * - * @SuppressWarnings(PHPMD.CouplingBetweenObjects) // it just touches a lot of things. */ class UpgradeDatabase extends Command { @@ -259,9 +256,6 @@ class UpgradeDatabase extends Command /** * Each (asset) account must have a reference to a preferred currency. If the account does not have one, it's forced upon the account. - * - * @SuppressWarnings(PHPMD.CyclomaticComplexity) // it's seven but it can't really be helped. - * @SuppressWarnings(PHPMD.ExcessiveMethodLength) */ public function updateAccountCurrencies(): void { @@ -316,8 +310,6 @@ class UpgradeDatabase extends Command * * Both source and destination must match the respective currency preference of the related asset account. * So FF3 must verify all transactions. - * - * @SuppressWarnings(PHPMD.ExcessiveMethodLength) */ public function updateOtherCurrencies(): void { @@ -551,10 +543,6 @@ class UpgradeDatabase extends Command * * Method is long and complex bit I'm taking it for granted. * - * @SuppressWarnings(PHPMD.ExcessiveMethodLength) - * @SuppressWarnings(PHPMD.NPathComplexity) - * @SuppressWarnings(PHPMD.CyclomaticComplexity) - * * @param Transaction $transaction */ private function updateTransactionCurrency(Transaction $transaction): void diff --git a/app/Console/Commands/VerifyDatabase.php b/app/Console/Commands/VerifyDatabase.php index 9ca4b6569d..981fb1d28a 100644 --- a/app/Console/Commands/VerifyDatabase.php +++ b/app/Console/Commands/VerifyDatabase.php @@ -46,8 +46,6 @@ use stdClass; /** * Class VerifyDatabase. - * - * @SuppressWarnings(PHPMD.CouplingBetweenObjects) */ class VerifyDatabase extends Command { diff --git a/app/Repositories/Budget/BudgetRepository.php b/app/Repositories/Budget/BudgetRepository.php index 443a117405..6cc8bd41eb 100644 --- a/app/Repositories/Budget/BudgetRepository.php +++ b/app/Repositories/Budget/BudgetRepository.php @@ -269,12 +269,16 @@ class BudgetRepository implements BudgetRepositoryInterface /** * Find a budget or return NULL * - * @param int $budgetId + * @param int $budgetId |null * * @return Budget|null */ - public function findNull(int $budgetId): ?Budget + public function findNull(int $budgetId = null): ?Budget { + if (null === $budgetId) { + return null; + } + return $this->user->budgets()->find($budgetId); } diff --git a/app/Repositories/Budget/BudgetRepositoryInterface.php b/app/Repositories/Budget/BudgetRepositoryInterface.php index d8e69f2fef..229bee793a 100644 --- a/app/Repositories/Budget/BudgetRepositoryInterface.php +++ b/app/Repositories/Budget/BudgetRepositoryInterface.php @@ -114,13 +114,11 @@ interface BudgetRepositoryInterface public function findByName(string $name): ?Budget; /** - * Find a budget or return NULL - * - * @param int $budgetId + * @param int|null $budgetId * * @return Budget|null */ - public function findNull(int $budgetId): ?Budget; + public function findNull(int $budgetId = null): ?Budget; /** * This method returns the oldest journal or transaction date known to this budget. diff --git a/app/Validation/FireflyValidator.php b/app/Validation/FireflyValidator.php index 15dcc33618..f0aab84bf0 100644 --- a/app/Validation/FireflyValidator.php +++ b/app/Validation/FireflyValidator.php @@ -432,6 +432,19 @@ class FireflyValidator extends Validator return false; } + /** + * TODO fill me. + * + * @param $attribute + * @param $value + * @param $parameters + * + * @return bool + */ + public function validateRepetitionMoment($attribute, $value, $parameters): bool { + + } + /** * @SuppressWarnings(PHPMD.UnusedFormalParameter) * diff --git a/app/Validation/RecurrenceValidation.php b/app/Validation/RecurrenceValidation.php new file mode 100644 index 0000000000..42f46d3d3e --- /dev/null +++ b/app/Validation/RecurrenceValidation.php @@ -0,0 +1,194 @@ +. + */ + +declare(strict_types=1); + +namespace FireflyIII\Validation; + +use Carbon\Carbon; +use Exception; +use Illuminate\Validation\Validator; +use InvalidArgumentException; + +/** + * Trait RecurrenceValidation + * + * Contains advanced validation rules used in validation of new and existing recurrences. + * + * @package FireflyIII\Validation + */ +trait RecurrenceValidation +{ + /** + * @param Validator $validator + */ + public function validateRepetitionMoment(Validator $validator): void + { + $data = $validator->getData(); + $repetitions = $data['repetitions'] ?? []; + /** + * @var int $index + * @var array $repetition + */ + foreach ($repetitions as $index => $repetition) { + switch ($repetition['type']) { + default: + $validator->errors()->add(sprintf('repetitions.%d.type', $index), trans('validation.valid_recurrence_rep_type')); + + return; + case 'daily': + $this->validateDaily($validator, $index, (string)$repetition['moment']); + break; + case 'monthly': + $this->validateMonthly($validator, $index, (int)$repetition['moment']); + break; + case 'ndom': + $this->validateNdom($validator, $index, (string)$repetition['moment']); + break; + case 'weekly': + $this->validateWeekly($validator, $index, (int)$repetition['moment']); + break; + case 'yearly': + $this->validateYearly($validator, $index, (string)$repetition['moment']); + break; + } + } + } + + /** + * Adds an error to the validator when there are no repetitions in the array of data. + * + * @param Validator $validator + */ + public function validateOneRepetition(Validator $validator): void + { + $data = $validator->getData(); + $repetitions = $data['repetitions'] ?? []; + // need at least one transaction + if (\count($repetitions) === 0) { + $validator->errors()->add('description', trans('validation.at_least_one_repetition')); + } + } + + /** + * Validates that the recurrence has valid repetition information. It either doesn't stop, + * or stops after X times or at X date. Not both of them., + * + * @param Validator $validator + */ + public function validateRecurrenceRepetition(Validator $validator): void + { + $data = $validator->getData(); + $repetitions = $data['nr_of_repetitions'] ?? null; + $repeatUntil = $data['repeat_until'] ?? null; + if (null !== $repetitions && null !== $repeatUntil) { + // expect a date OR count: + $validator->errors()->add('repeat_until', trans('validation.require_repeat_until')); + $validator->errors()->add('nr_of_repetitions', trans('validation.require_repeat_until')); + } + } + + /** + * If the repetition type is daily, the moment should be empty. + * + * @param Validator $validator + * @param int $index + * @param string $moment + */ + protected function validateDaily(Validator $validator, int $index, string $moment): void + { + if ('' !== $moment) { + $validator->errors()->add(sprintf('repetitions.%d.moment', $index), trans('validation.valid_recurrence_rep_moment')); + } + } + + /** + * If the repetition type is monthly, the moment should be a day between 1-31 (inclusive). + * + * @param Validator $validator + * @param int $index + * @param int $dayOfMonth + */ + protected function validateMonthly(Validator $validator, int $index, int $dayOfMonth): void + { + if ($dayOfMonth < 1 || $dayOfMonth > 31) { + $validator->errors()->add(sprintf('repetitions.%d.moment', $index), trans('validation.valid_recurrence_rep_moment')); + } + } + + /** + * If the repetition type is "ndom", the first part must be between 1-5 (inclusive), for the week in the month, + * and the second one must be between 1-7 (inclusive) for the day of the week. + * + * @param Validator $validator + * @param int $index + * @param string $moment + */ + protected function validateNdom(Validator $validator, int $index, string $moment): void + { + $parameters = explode(',', $moment); + if (\count($parameters) !== 2) { + $validator->errors()->add(sprintf('repetitions.%d.moment', $index), trans('validation.valid_recurrence_rep_moment')); + + return; + } + $nthDay = (int)($parameters[0] ?? 0.0); + $dayOfWeek = (int)($parameters[1] ?? 0.0); + if ($nthDay < 1 || $nthDay > 5) { + $validator->errors()->add(sprintf('repetitions.%d.moment', $index), trans('validation.valid_recurrence_rep_moment')); + + return; + } + if ($dayOfWeek < 1 || $dayOfWeek > 7) { + $validator->errors()->add(sprintf('repetitions.%d.moment', $index), trans('validation.valid_recurrence_rep_moment')); + } + } + + /** + * If the repetition type is weekly, the moment should be a day between 1-7 (inclusive). + * + * @param Validator $validator + * @param int $index + * @param int $dayOfWeek + */ + protected function validateWeekly(Validator $validator, int $index, int $dayOfWeek): void + { + if ($dayOfWeek < 1 || $dayOfWeek > 7) { + $validator->errors()->add(sprintf('repetitions.%d.moment', $index), trans('validation.valid_recurrence_rep_moment')); + } + } + + /** + * If the repetition type is yearly, the moment should be a valid date. + * + * @param Validator $validator + * @param int $index + * @param string $moment + */ + protected function validateYearly(Validator $validator, int $index, string $moment): void + { + try { + Carbon::createFromFormat('Y-m-d', $moment); + } catch (InvalidArgumentException|Exception $e) { + $validator->errors()->add(sprintf('repetitions.%d.moment', $index), trans('validation.valid_recurrence_rep_moment')); + } + } +} \ No newline at end of file diff --git a/app/Validation/TransactionValidation.php b/app/Validation/TransactionValidation.php new file mode 100644 index 0000000000..af397a7e0e --- /dev/null +++ b/app/Validation/TransactionValidation.php @@ -0,0 +1,366 @@ +. + */ + +declare(strict_types=1); + +namespace FireflyIII\Validation; + +use FireflyIII\Models\Account; +use FireflyIII\Models\AccountType; +use FireflyIII\Models\Transaction; +use FireflyIII\Repositories\Account\AccountRepositoryInterface; +use FireflyIII\User; +use Illuminate\Validation\Validator; +use Log; + +/** + * Trait TransactionValidation + * + * @package FireflyIII\Validation + */ +trait TransactionValidation +{ + /** + * Validates the given account information. Switches on given transaction type. + * + * @param Validator $validator + */ + public function validateAccountInformation(Validator $validator): void + { + $data = $validator->getData(); + $transactions = $data['transactions'] ?? []; + $idField = 'description'; + $transactionType = $data['type'] ?? 'invalid'; + // get transaction type: + if (!isset($data['type'])) { + // the journal may exist in the request: + /** @var Transaction $transaction */ + $transaction = $this->route()->parameter('transaction'); + if (null !== $transaction) { + $transactionType = strtolower($transaction->transactionJournal->transactionType->type); + } + } + + foreach ($transactions as $index => $transaction) { + $sourceId = isset($transaction['source_id']) ? (int)$transaction['source_id'] : null; + $sourceName = $transaction['source_name'] ?? null; + $destinationId = isset($transaction['destination_id']) ? (int)$transaction['destination_id'] : null; + $destinationName = $transaction['destination_name'] ?? null; + $sourceAccount = null; + $destinationAccount = null; + switch ($transactionType) { + case 'withdrawal': + $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'; + $sourceAccount = $this->opposingAccountExists($validator, AccountType::REVENUE, $sourceId, $sourceName, $idField); + + $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'; + $sourceAccount = $this->assetAccountExists($validator, $sourceId, $sourceName, $idField, $nameField); + + $idField = 'transactions.' . $index . '.destination_id'; + $nameField = 'transactions.' . $index . '.destination_name'; + $destinationAccount = $this->assetAccountExists($validator, $destinationId, $destinationName, $idField, $nameField); + break; + default: + $validator->errors()->add($idField, trans('validation.invalid_account_info')); + + return; + + } + // add some errors in case of same account submitted: + if (null !== $sourceAccount && null !== $destinationAccount && $sourceAccount->id === $destinationAccount->id) { + $validator->errors()->add($idField, trans('validation.source_equals_destination')); + } + } + } + + /** + * Adds an error to the "description" field when the user has submitted no descriptions and no + * journal description. + * + * @param Validator $validator + */ + public function validateDescriptions(Validator $validator): void + { + $data = $validator->getData(); + $transactions = $data['transactions'] ?? []; + $journalDescription = (string)($data['description'] ?? ''); + $validDescriptions = 0; + foreach ($transactions as $index => $transaction) { + if (\strlen((string)($transaction['description'] ?? '')) > 0) { + $validDescriptions++; + } + } + + // no valid descriptions and empty journal description? error. + if ($validDescriptions === 0 && '' === $journalDescription) { + $validator->errors()->add('description', trans('validation.filled', ['attribute' => trans('validation.attributes.description')])); + } + } + + /** + * If the transactions contain foreign amounts, there must also be foreign currency information. + * + * @param Validator $validator + */ + public function validateForeignCurrencyInformation(Validator $validator): void + { + $data = $validator->getData(); + $transactions = $data['transactions'] ?? []; + foreach ($transactions as $index => $transaction) { + // must have currency info. + if (isset($transaction['foreign_amount']) + && !(isset($transaction['foreign_currency_id']) + || isset($transaction['foreign_currency_code']))) { + $validator->errors()->add( + 'transactions.' . $index . '.foreign_amount', + trans('validation.require_currency_info') + ); + } + } + } + + /** + * Adds an error to the validator when any transaction descriptions are equal to the journal description. + * + * @param Validator $validator + */ + public function validateJournalDescription(Validator $validator): void + { + $data = $validator->getData(); + $transactions = $data['transactions'] ?? []; + $journalDescription = (string)($data['description'] ?? ''); + foreach ($transactions as $index => $transaction) { + $description = (string)($transaction['description'] ?? ''); + // description cannot be equal to journal description. + if ($description === $journalDescription) { + $validator->errors()->add('transactions.' . $index . '.description', trans('validation.equal_description')); + } + } + } + + /** + * Adds an error to the validator when there are no transactions in the array of data. + * + * @param Validator $validator + */ + public function validateOneTransaction(Validator $validator): void + { + $data = $validator->getData(); + $transactions = $data['transactions'] ?? []; + // need at least one transaction + if (\count($transactions) === 0) { + $validator->errors()->add('description', trans('validation.at_least_one_transaction')); + } + } + + /** + * 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', trans('validation.all_accounts_equal')); + } + break; + case 'deposit': + if (\count($destinations) > 1) { + $validator->errors()->add('transactions.0.destination_id', trans('validation.all_accounts_equal')); + } + break; + case 'transfer': + if (\count($sources) > 1 || \count($destinations) > 1) { + $validator->errors()->add('transactions.0.source_id', trans('validation.all_accounts_equal')); + $validator->errors()->add('transactions.0.destination_id', trans('validation.all_accounts_equal')); + } + break; + } + } + + /** + * Adds an error to the validator when the user submits a split transaction (more than 1 transactions) + * but does not give them a description. + * + * @param Validator $validator + */ + public function validateSplitDescriptions(Validator $validator): void + { + $data = $validator->getData(); + $transactions = $data['transactions'] ?? []; + foreach ($transactions as $index => $transaction) { + $description = (string)($transaction['description'] ?? ''); + // filled description is mandatory for split transactions. + if ('' === $description && \count($transactions) > 1) { + $validator->errors()->add( + 'transactions.' . $index . '.description', + trans('validation.filled', ['attribute' => trans('validation.attributes.transaction_description')]) + ); + } + } + } + + /** + * Throws an error when this asset account is invalid. + * + * @noinspection MoreThanThreeArgumentsInspection + * + * @param Validator $validator + * @param int|null $accountId + * @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): ?Account + { + /** @var User $admin */ + $admin = auth()->user(); + $accountId = (int)$accountId; + $accountName = (string)$accountName; + // both empty? hard exit. + if ($accountId < 1 && '' === $accountName) { + $validator->errors()->add($idField, trans('validation.filled', ['attribute' => $idField])); + + return null; + } + // ID belongs to user and is asset account: + /** @var AccountRepositoryInterface $repository */ + $repository = app(AccountRepositoryInterface::class); + $repository->setUser($admin); + $set = $repository->getAccountsById([$accountId]); + Log::debug(sprintf('Count of accounts found by ID %d is: %d', $accountId, $set->count())); + if ($set->count() === 1) { + /** @var Account $first */ + $first = $set->first(); + if ($first->accountType->type !== AccountType::ASSET) { + $validator->errors()->add($idField, trans('validation.belongs_user')); + + return null; + } + + // we ignore the account name at this point. + return $first; + } + + $account = $repository->findByName($accountName, [AccountType::ASSET]); + if (null === $account) { + $validator->errors()->add($nameField, trans('validation.belongs_user')); + + return null; + } + + return $account; + } + + /** + * Throws an error when the given opposing account (of type $type) is invalid. + * Empty data is allowed, system will default to cash. + * + * @noinspection MoreThanThreeArgumentsInspection + * + * @param Validator $validator + * @param string $type + * @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): ?Account + { + /** @var User $admin */ + $admin = auth()->user(); + $accountId = (int)$accountId; + $accountName = (string)$accountName; + // both empty? done! + if ($accountId < 1 && '' === $accountName) { + return null; + } + if ($accountId !== 0) { + // ID belongs to user and is $type account: + /** @var AccountRepositoryInterface $repository */ + $repository = app(AccountRepositoryInterface::class); + $repository->setUser($admin); + $set = $repository->getAccountsById([$accountId]); + if ($set->count() === 1) { + /** @var Account $first */ + $first = $set->first(); + if ($first->accountType->type !== $type) { + $validator->errors()->add($idField, trans('validation.belongs_user')); + + return null; + } + + // we ignore the account name at this point. + return $first; + } + } + + // not having an opposing account by this name is NOT a problem. + return null; + } +} \ No newline at end of file