From 8e104a62aefa25f3f561ae217f28e5703af0272d Mon Sep 17 00:00:00 2001 From: James Cole Date: Tue, 10 Aug 2021 18:43:21 +0200 Subject: [PATCH] Better endpoint to move transactions. --- .../Data/Bulk/AccountController.php | 9 +- .../Data/Bulk/TransactionController.php | 75 +++++++++++++++ .../Requests/Data/Bulk/TransactionRequest.php | 64 +++++++++++++ app/Enums/ClauseType.php | 13 +++ app/Rules/IsValidBulkClause.php | 94 +++++++++++++++++++ .../Bulk/ValidatesBulkTransactionQuery.php | 53 +++++++++++ config/bulk.php | 14 +++ resources/lang/en_US/validation.php | 9 +- routes/api.php | 14 ++- 9 files changed, 339 insertions(+), 6 deletions(-) create mode 100644 app/Api/V1/Controllers/Data/Bulk/TransactionController.php create mode 100644 app/Api/V1/Requests/Data/Bulk/TransactionRequest.php create mode 100644 app/Enums/ClauseType.php create mode 100644 app/Rules/IsValidBulkClause.php create mode 100644 app/Validation/Api/Data/Bulk/ValidatesBulkTransactionQuery.php create mode 100644 config/bulk.php diff --git a/app/Api/V1/Controllers/Data/Bulk/AccountController.php b/app/Api/V1/Controllers/Data/Bulk/AccountController.php index 704a6b62aa..09719a0b3e 100644 --- a/app/Api/V1/Controllers/Data/Bulk/AccountController.php +++ b/app/Api/V1/Controllers/Data/Bulk/AccountController.php @@ -12,11 +12,16 @@ use Illuminate\Http\JsonResponse; /** * Class AccountController + * + * @deprecated */ class AccountController extends Controller { private AccountRepositoryInterface $repository; + /** + * + */ public function __construct() { parent::__construct(); @@ -37,8 +42,8 @@ class AccountController extends Controller */ public function moveTransactions(MoveTransactionsRequest $request): JsonResponse { - $accountIds = $request->getAll(); - $original = $this->repository->find($accountIds['original_account']); + $accountIds = $request->getAll(); + $original = $this->repository->find($accountIds['original_account']); $destination = $this->repository->find($accountIds['destination_account']); /** @var AccountDestroyService $service */ diff --git a/app/Api/V1/Controllers/Data/Bulk/TransactionController.php b/app/Api/V1/Controllers/Data/Bulk/TransactionController.php new file mode 100644 index 0000000000..7ce0be96e8 --- /dev/null +++ b/app/Api/V1/Controllers/Data/Bulk/TransactionController.php @@ -0,0 +1,75 @@ +middleware( + function ($request, $next) { + $this->repository = app(AccountRepositoryInterface::class); + $this->repository->setUser(auth()->user()); + + return $next($request); + } + ); + } + + /** + * @param TransactionRequest $request + * + * @return JsonResponse + */ + public function update(TransactionRequest $request): JsonResponse + { + $query = $request->getAll(); + $params = $query['query']; + // this deserves better code, but for now a loop of basic if-statements + // to respond to what is in the $query. + // this is OK because only one thing can be in the query at the moment. + if ($this->updatesTransactionAccount($params)) { + $original = $this->repository->find((int)$params['where']['source_account_id']); + $destination = $this->repository->find((int)$params['update']['destination_account_id']); + + /** @var AccountDestroyService $service */ + $service = app(AccountDestroyService::class); + $service->moveTransactions($original, $destination); + } + + return response()->json([], 204); + } + + /** + * @param array $params + * + * @return bool + */ + private function updatesTransactionAccount(array $params): bool + { + return array_key_exists('source_account_id', $params['where']) && array_key_exists('destination_account_id', $params['update']); + } + +} \ No newline at end of file diff --git a/app/Api/V1/Requests/Data/Bulk/TransactionRequest.php b/app/Api/V1/Requests/Data/Bulk/TransactionRequest.php new file mode 100644 index 0000000000..3231425c13 --- /dev/null +++ b/app/Api/V1/Requests/Data/Bulk/TransactionRequest.php @@ -0,0 +1,64 @@ + json_decode($this->get('query'), true, 8, JSON_THROW_ON_ERROR), + ]; + } catch (JsonException $e) { + // dont really care. the validation should catch invalid json. + Log::error($e->getMessage()); + } + + return $data; + } + + /** + * @return string[] + */ + public function rules(): array + { + return [ + 'query' => ['required', 'min:1', 'max:255', 'json', new IsValidBulkClause(ClauseType::TRANSACTION)], + ]; + } + + /** + * @param Validator $validator + * + * @return void + */ + public function withValidator(Validator $validator): void + { + $validator->after( + function (Validator $validator) { + // validate transaction query data. + $this->validateTransactionQuery($validator); + } + ); + } +} \ No newline at end of file diff --git a/app/Enums/ClauseType.php b/app/Enums/ClauseType.php new file mode 100644 index 0000000000..fc13218603 --- /dev/null +++ b/app/Enums/ClauseType.php @@ -0,0 +1,13 @@ +rules = config(sprintf('bulk.%s', $type)); + $this->error = (string)trans('firefly.belongs_user'); + } + + /** + * @param string $attribute + * @param mixed $value + * + * @return bool + */ + public function passes($attribute, $value): bool + { + $result = $this->basicValidation((string)$value); + if (false === $result) { + return false; + } + return true; + } + + /** + * @return string + */ + public function message(): string + { + return $this->error; + } + + /** + * Does basic rule based validation. + * + * @return bool + */ + private function basicValidation(string $value): bool + { + try { + $array = json_decode($value, true, 8, JSON_THROW_ON_ERROR); + } catch (JsonException $e) { + $this->error = (string)trans('validation.json'); + + return false; + } + $clauses = ['where', 'update']; + foreach ($clauses as $clause) { + if (!array_key_exists($clause, $array)) { + $this->error = (string)trans(sprintf('validation.missing_%s', $clause)); + + return false; + } + /** + * @var string $arrayKey + * @var mixed $arrayValue + */ + foreach ($array[$clause] as $arrayKey => $arrayValue) { + if (!array_key_exists($arrayKey, $this->rules[$clause])) { + $this->error = (string)trans(sprintf('validation.invalid_%s_key', $clause)); + + return false; + } + // validate! + $validator = Validator::make(['value' => $arrayValue], [ + 'value' => $this->rules[$clause][$arrayKey], + ]); + if ($validator->fails()) { + $this->error = sprintf('%s: %s: %s',$clause, $arrayKey, join(', ', ($validator->errors()->get('value')))); + + return false; + } + } + } + + return true; + } +} \ No newline at end of file diff --git a/app/Validation/Api/Data/Bulk/ValidatesBulkTransactionQuery.php b/app/Validation/Api/Data/Bulk/ValidatesBulkTransactionQuery.php new file mode 100644 index 0000000000..1126569355 --- /dev/null +++ b/app/Validation/Api/Data/Bulk/ValidatesBulkTransactionQuery.php @@ -0,0 +1,53 @@ +getData(); + // assumption is all validation has already taken place + // and the query key exists. + $json = json_decode($data['query'], true, 8); + + if (array_key_exists('source_account_id', $json['where']) + && array_key_exists('destination_account_id', $json['update']) + ) { + // find both accounts + // must be same type. + // already validated: belongs to this user. + $repository = app(AccountRepositoryInterface::class); + $source = $repository->find((int)$json['where']['source_account_id']); + $dest = $repository->find((int)$json['update']['destination_account_id']); + if (null === $source) { + $validator->errors()->add('query', sprintf((string)trans('validation.invalid_query_data'), 'where', 'source_account_id')); + + return; + } + if (null === $dest) { + $validator->errors()->add('query', sprintf((string)trans('validation.invalid_query_data'), 'update', 'destination_account_id')); + + return; + } + if ($source->accountType->type !== $dest->accountType->type) { + $validator->errors()->add('query', (string)trans('validation.invalid_query_account_type')); + return; + } + // must have same currency: + if($repository->getAccountCurrency($source)->id !== $repository->getAccountCurrency($dest)->id) { + $validator->errors()->add('query', (string)trans('validation.invalid_query_currency')); + } + } + } + +} \ No newline at end of file diff --git a/config/bulk.php b/config/bulk.php new file mode 100644 index 0000000000..4539a0c192 --- /dev/null +++ b/config/bulk.php @@ -0,0 +1,14 @@ + [ + ClauseType::WHERE => [ + 'source_account_id' => 'required|numeric|belongsToUser:accounts,id', + ], + ClauseType::UPDATE => [ + 'destination_account_id' => 'required|numeric|belongsToUser:accounts,id', + ], + ], +]; \ No newline at end of file diff --git a/resources/lang/en_US/validation.php b/resources/lang/en_US/validation.php index b7e6143ff5..8a4bf1e955 100644 --- a/resources/lang/en_US/validation.php +++ b/resources/lang/en_US/validation.php @@ -23,6 +23,13 @@ declare(strict_types=1); return [ + 'missing_where' => 'Array is missing "where"-clause', + 'missing_update' => 'Array is missing "update"-clause', + 'invalid_where_key' => 'JSON contains an invalid key for the "where"-clause', + 'invalid_update_key' => 'JSON contains an invalid key for the "update"-clause', + 'invalid_query_data' => 'There is invalid data in the %s:%s field of your query.', + 'invalid_query_account_type' => 'Your query contains accounts of different types, which is not allowed.', + 'invalid_query_currency' => 'Your query contains accounts that have different currency settings, which is not allowed.', 'iban' => 'This is not a valid IBAN.', 'zero_or_more' => 'The value cannot be negative.', 'date_or_time' => 'The value must be a valid date or time value (ISO 8601).', @@ -185,7 +192,7 @@ return [ 'withdrawal_dest_need_data' => 'Need to get a valid destination account ID and/or valid destination account name to continue.', 'withdrawal_dest_bad_data' => 'Could not find a valid destination account when searching for ID ":id" or name ":name".', - 'generic_source_bad_data' => 'Could not find a valid source account when searching for ID ":id" or name ":name".', + 'generic_source_bad_data' => 'Could not find a valid source account when searching for ID ":id" or name ":name".', 'deposit_source_need_data' => 'Need to get a valid source account ID and/or valid source account name to continue.', 'deposit_source_bad_data' => 'Could not find a valid source account when searching for ID ":id" or name ":name".', diff --git a/routes/api.php b/routes/api.php index 7e8570df4e..83b658f039 100644 --- a/routes/api.php +++ b/routes/api.php @@ -90,14 +90,22 @@ Route::group( } ); -// Bulk update Account routes +// Bulk update API routes Route::group( - ['namespace' => 'FireflyIII\Api\V1\Controllers\Data\Bulk', 'prefix' => 'data/bulk/accounts', + ['namespace' => 'FireflyIII\Api\V1\Controllers\Data\Bulk', 'prefix' => 'data/bulk', 'as' => 'api.v1.data.bulk.',], static function () { - Route::post('transactions', ['uses' => 'AccountController@moveTransactions', 'as' => 'accounts.move-transactions']); + Route::post('transactions', ['uses' => 'TransactionController@update', 'as' => 'transactions']); } ); +//Route::group( +// ['namespace' => 'FireflyIII\Api\V1\Controllers\Data\Bulk', 'prefix' => 'data/bulk', +// 'as' => 'api.v1.data.bulk.',], +// static function () { +// Route::post('transactions', ['uses' => 'AccountController@moveTransactions', 'as' => 'accounts.move-transactions']); +// } +//); + /** * INSIGHTS ROUTES