From c0909aebba2668816baea42d09d1d614435953be Mon Sep 17 00:00:00 2001 From: James Cole Date: Sun, 25 Aug 2019 07:48:46 +0200 Subject: [PATCH] Rule controller can handle empty submission. #2477 The rule repository can't handle this (yet). --- app/Api/V1/Controllers/RuleController.php | 116 ++++++------ .../{RuleRequest.php => RuleStoreRequest.php} | 6 +- app/Api/V1/Requests/RuleUpdateRequest.php | 166 ++++++++++++++++++ app/Http/Requests/Request.php | 11 +- 4 files changed, 238 insertions(+), 61 deletions(-) rename app/Api/V1/Requests/{RuleRequest.php => RuleStoreRequest.php} (98%) create mode 100644 app/Api/V1/Requests/RuleUpdateRequest.php diff --git a/app/Api/V1/Controllers/RuleController.php b/app/Api/V1/Controllers/RuleController.php index cfff4945a8..50493083cc 100644 --- a/app/Api/V1/Controllers/RuleController.php +++ b/app/Api/V1/Controllers/RuleController.php @@ -24,8 +24,10 @@ declare(strict_types=1); namespace FireflyIII\Api\V1\Controllers; use FireflyIII\Api\V1\Requests\RuleRequest; +use FireflyIII\Api\V1\Requests\RuleStoreRequest; use FireflyIII\Api\V1\Requests\RuleTestRequest; use FireflyIII\Api\V1\Requests\RuleTriggerRequest; +use FireflyIII\Api\V1\Requests\RuleUpdateRequest; use FireflyIII\Exceptions\FireflyException; use FireflyIII\Helpers\Collector\GroupCollectorInterface; use FireflyIII\Models\Rule; @@ -59,6 +61,7 @@ class RuleController extends Controller /** * RuleController constructor. + * * @codeCoverageIgnore */ public function __construct() @@ -135,11 +138,59 @@ class RuleController extends Controller } + /** + * @param Request $request + * @param Rule $rule + * + * @return JsonResponse + */ + public function moveDown(Request $request, Rule $rule): JsonResponse + { + $this->ruleRepository->moveDown($rule); + $rule = $this->ruleRepository->find($rule->id); + $manager = new Manager(); + $baseUrl = $request->getSchemeAndHttpHost() . '/api/v1'; + $manager->setSerializer(new JsonApiSerializer($baseUrl)); + + /** @var RuleTransformer $transformer */ + $transformer = app(RuleTransformer::class); + $transformer->setParameters($this->parameters); + + $resource = new Item($rule, $transformer, 'rules'); + + return response()->json($manager->createData($resource)->toArray())->header('Content-Type', 'application/vnd.api+json'); + + } + + /** + * @param Request $request + * @param Rule $rule + * + * @return JsonResponse + */ + public function moveUp(Request $request, Rule $rule): JsonResponse + { + $this->ruleRepository->moveUp($rule); + $rule = $this->ruleRepository->find($rule->id); + $manager = new Manager(); + $baseUrl = $request->getSchemeAndHttpHost() . '/api/v1'; + $manager->setSerializer(new JsonApiSerializer($baseUrl)); + + /** @var RuleTransformer $transformer */ + $transformer = app(RuleTransformer::class); + $transformer->setParameters($this->parameters); + + $resource = new Item($rule, $transformer, 'rules'); + + return response()->json($manager->createData($resource)->toArray())->header('Content-Type', 'application/vnd.api+json'); + + } + /** * List single resource. * * @param Request $request - * @param Rule $rule + * @param Rule $rule * * @return JsonResponse * @codeCoverageIgnore @@ -163,11 +214,11 @@ class RuleController extends Controller /** * Store new object. * - * @param RuleRequest $request + * @param RuleStoreRequest $request * * @return JsonResponse */ - public function store(RuleRequest $request): JsonResponse + public function store(RuleStoreRequest $request): JsonResponse { $rule = $this->ruleRepository->store($request->getAll()); $manager = new Manager(); @@ -185,7 +236,7 @@ class RuleController extends Controller /** * @param RuleTestRequest $request - * @param Rule $rule + * @param Rule $rule * * @return JsonResponse * @throws FireflyException @@ -231,7 +282,7 @@ class RuleController extends Controller * Execute the given rule group on a set of existing transactions. * * @param RuleTriggerRequest $request - * @param Rule $rule + * @param Rule $rule * * @return JsonResponse */ @@ -268,14 +319,15 @@ class RuleController extends Controller /** * Update a rule. * - * @param RuleRequest $request - * @param Rule $rule + * @param RuleUpdateRequest $request + * @param Rule $rule * * @return JsonResponse */ - public function update(RuleRequest $request, Rule $rule): JsonResponse + public function update(RuleUpdateRequest $request, Rule $rule): JsonResponse { - $rule = $this->ruleRepository->update($rule, $request->getAll()); + $data = $request->getAll(); + $rule = $this->ruleRepository->update($rule, $data); $manager = new Manager(); $baseUrl = $request->getSchemeAndHttpHost() . '/api/v1'; @@ -289,50 +341,4 @@ class RuleController extends Controller return response()->json($manager->createData($resource)->toArray())->header('Content-Type', 'application/vnd.api+json'); } - - /** - * @param Request $request - * @param Rule $rule - * @return JsonResponse - */ - public function moveDown(Request $request, Rule $rule): JsonResponse - { - $this->ruleRepository->moveDown($rule); - $rule = $this->ruleRepository->find($rule->id); - $manager = new Manager(); - $baseUrl = $request->getSchemeAndHttpHost() . '/api/v1'; - $manager->setSerializer(new JsonApiSerializer($baseUrl)); - - /** @var RuleTransformer $transformer */ - $transformer = app(RuleTransformer::class); - $transformer->setParameters($this->parameters); - - $resource = new Item($rule, $transformer, 'rules'); - - return response()->json($manager->createData($resource)->toArray())->header('Content-Type', 'application/vnd.api+json'); - - } - - /** - * @param Request $request - * @param Rule $rule - * @return JsonResponse - */ - public function moveUp(Request $request, Rule $rule): JsonResponse - { - $this->ruleRepository->moveUp($rule); - $rule = $this->ruleRepository->find($rule->id); - $manager = new Manager(); - $baseUrl = $request->getSchemeAndHttpHost() . '/api/v1'; - $manager->setSerializer(new JsonApiSerializer($baseUrl)); - - /** @var RuleTransformer $transformer */ - $transformer = app(RuleTransformer::class); - $transformer->setParameters($this->parameters); - - $resource = new Item($rule, $transformer, 'rules'); - - return response()->json($manager->createData($resource)->toArray())->header('Content-Type', 'application/vnd.api+json'); - - } } diff --git a/app/Api/V1/Requests/RuleRequest.php b/app/Api/V1/Requests/RuleStoreRequest.php similarity index 98% rename from app/Api/V1/Requests/RuleRequest.php rename to app/Api/V1/Requests/RuleStoreRequest.php index 3924ef956f..6abc9b6027 100644 --- a/app/Api/V1/Requests/RuleRequest.php +++ b/app/Api/V1/Requests/RuleStoreRequest.php @@ -1,6 +1,6 @@ . + */ + +declare(strict_types=1); + +namespace FireflyIII\Api\V1\Requests; + +use FireflyIII\Rules\IsBoolean; +use function is_array; + + +/** + * Class RuleUpdateRequest + * + */ +class RuleUpdateRequest extends Request +{ + /** + * Authorize logged in users. + * + * @return bool + */ + public function authorize(): bool + { + // Only allow authenticated users + return auth()->check(); + } + + /** + * Get all data from the request. + * + * @return array + */ + public function getAll(): array + { + $strict = null; + $active = null; + $stopProcessing = null; + if (null !== $this->get('active')) { + $active = $this->boolean('active'); + } + if (null !== $this->get('strict')) { + $strict = $this->boolean('strict'); + } + if (null !== $this->get('stop_processing')) { + $stopProcessing = $this->boolean('stop_processing'); + } + + $data = [ + 'title' => $this->nullableString('title'), + 'description' => $this->nullableString('description'), + 'rule_group_id' => $this->nullableInteger('rule_group_id'), + 'rule_group_title' => $this->nullableString('rule_group_title'), + 'trigger' => $this->nullableString('trigger'), + 'strict' => $strict, + 'stop_processing' => $stopProcessing, + 'active' => $active, + 'triggers' => $this->getRuleTriggers(), + 'actions' => $this->getRuleActions(), + ]; + + return $data; + } + + /** + * The rules that the incoming request must be matched against. + * + * @return array + */ + public function rules(): array + { + $validTriggers = array_keys(config('firefly.rule-triggers')); + $validActions = array_keys(config('firefly.rule-actions')); + $rule = $this->route()->parameter('rule'); + + // some triggers and actions require text: + $contextTriggers = implode(',', config('firefly.context-rule-triggers')); + $contextActions = implode(',', config('firefly.context-rule-actions')); + $rules = [ + 'title' => sprintf('between:1,100|uniqueObjectForUser:rules,title,%d', $rule->id), + 'description' => 'between:1,5000|nullable', + 'rule_group_id' => 'belongsToUser:rule_groups', + 'rule_group_title' => 'nullable|between:1,255|belongsToUser:rule_groups,title', + 'trigger' => 'in:store-journal,update-journal', + 'triggers.*.type' => 'required|in:' . implode(',', $validTriggers), + 'triggers.*.value' => 'required_if:actions.*.type,' . $contextTriggers . '|min:1|ruleTriggerValue', + 'triggers.*.stop_processing' => [new IsBoolean], + 'triggers.*.active' => [new IsBoolean], + 'actions.*.type' => 'required|in:' . implode(',', $validActions), + 'actions.*.value' => 'required_if:actions.*.type,' . $contextActions . '|ruleActionValue', + 'actions.*.stop_processing' => [new IsBoolean], + 'actions.*.active' => [new IsBoolean], + 'strict' => [new IsBoolean], + 'stop_processing' => [new IsBoolean], + 'active' => [new IsBoolean], + ]; + + return $rules; + } + + /** + * @return array|null + */ + private function getRuleActions(): ?array + { + if (!$this->has('actions')) { + return null; + } + $actions = $this->get('actions'); + $return = []; + if (is_array($actions)) { + foreach ($actions as $action) { + $return[] = [ + 'type' => $action['type'], + 'value' => $action['value'], + 'active' => $this->convertBoolean((string)($action['active'] ?? 'false')), + 'stop_processing' => $this->convertBoolean((string)($action['stop_processing'] ?? 'false')), + ]; + } + } + + return $return; + } + + /** + * @return array|null + */ + private function getRuleTriggers(): ?array + { + if (!$this->has('triggers')) { + return null; + } + $triggers = $this->get('triggers'); + $return = []; + if (is_array($triggers)) { + foreach ($triggers as $trigger) { + $return[] = [ + 'type' => $trigger['type'], + 'value' => $trigger['value'], + 'active' => $this->convertBoolean((string)($trigger['active'] ?? 'false')), + 'stop_processing' => $this->convertBoolean((string)($trigger['stop_processing'] ?? 'false')), + ]; + } + } + + return $return; + } +} diff --git a/app/Http/Requests/Request.php b/app/Http/Requests/Request.php index 73e00eba3a..f68da39153 100644 --- a/app/Http/Requests/Request.php +++ b/app/Http/Requests/Request.php @@ -186,6 +186,10 @@ class Request extends FormRequest */ public function nullableInteger(string $field): ?int { + if (!$this->has($field)) { + return null; + } + $value = (string)$this->get($field); if ('' === $value) { return null; @@ -203,9 +207,10 @@ class Request extends FormRequest */ public function nullableString(string $field): ?string { - $string = app('steam')->cleanString((string)($this->get($field) ?? '')); - - return '' === $string ? null : $string; + if (!$this->has($field)) { + return null; + } + return app('steam')->cleanString((string)($this->get($field) ?? '')); } /**