fix: migrate action expression validation to separate rule class

This commit is contained in:
Michael Thomas
2024-03-09 12:57:34 -05:00
parent 69ca88d9f8
commit c4bf2aae7d
7 changed files with 83 additions and 45 deletions

View File

@@ -25,7 +25,6 @@ namespace FireflyIII\Api\V1\Controllers\Models\Rule;
use FireflyIII\Api\V1\Controllers\Controller; use FireflyIII\Api\V1\Controllers\Controller;
use FireflyIII\Api\V1\Requests\Models\Rule\ValidateExpressionRequest; use FireflyIII\Api\V1\Requests\Models\Rule\ValidateExpressionRequest;
use FireflyIII\TransactionRules\Expressions\ActionExpression;
use Illuminate\Http\JsonResponse; use Illuminate\Http\JsonResponse;
/** /**
@@ -43,18 +42,8 @@ class ExpressionController extends Controller
*/ */
public function validateExpression(ValidateExpressionRequest $request): JsonResponse public function validateExpression(ValidateExpressionRequest $request): JsonResponse
{ {
$value = $request->getExpression();
$expr = new ActionExpression($value);
if ($expr->isValid()) {
return response()->json([ return response()->json([
"valid" => true, "valid" => true,
]); ]);
} else {
return response()->json([
"valid" => false,
"error" => $expr->getValidationError()->getMessage()
]);
}
} }
} }

View File

@@ -24,6 +24,7 @@ declare(strict_types=1);
namespace FireflyIII\Api\V1\Requests\Models\Rule; namespace FireflyIII\Api\V1\Requests\Models\Rule;
use FireflyIII\Rules\IsBoolean; use FireflyIII\Rules\IsBoolean;
use FireflyIII\Rules\IsValidActionExpression;
use FireflyIII\Support\Request\ChecksLogin; use FireflyIII\Support\Request\ChecksLogin;
use FireflyIII\Support\Request\ConvertsDataTypes; use FireflyIII\Support\Request\ConvertsDataTypes;
use FireflyIII\Support\Request\GetRuleConfiguration; use FireflyIII\Support\Request\GetRuleConfiguration;
@@ -123,7 +124,7 @@ class StoreRequest extends FormRequest
'triggers.*.stop_processing' => [new IsBoolean()], 'triggers.*.stop_processing' => [new IsBoolean()],
'triggers.*.active' => [new IsBoolean()], 'triggers.*.active' => [new IsBoolean()],
'actions.*.type' => 'required|in:'.implode(',', $validActions), 'actions.*.type' => 'required|in:'.implode(',', $validActions),
'actions.*.value' => 'required_if:actions.*.type,'.$contextActions.'|ruleActionExpression|ruleActionValue', 'actions.*.value' => ['required_if:actions.*.type,'.$contextActions, new IsValidActionExpression(), 'ruleActionValue'],
'actions.*.stop_processing' => [new IsBoolean()], 'actions.*.stop_processing' => [new IsBoolean()],
'actions.*.active' => [new IsBoolean()], 'actions.*.active' => [new IsBoolean()],
'strict' => [new IsBoolean()], 'strict' => [new IsBoolean()],

View File

@@ -25,6 +25,7 @@ namespace FireflyIII\Api\V1\Requests\Models\Rule;
use FireflyIII\Models\Rule; use FireflyIII\Models\Rule;
use FireflyIII\Rules\IsBoolean; use FireflyIII\Rules\IsBoolean;
use FireflyIII\Rules\IsValidActionExpression;
use FireflyIII\Support\Request\ChecksLogin; use FireflyIII\Support\Request\ChecksLogin;
use FireflyIII\Support\Request\ConvertsDataTypes; use FireflyIII\Support\Request\ConvertsDataTypes;
use FireflyIII\Support\Request\GetRuleConfiguration; use FireflyIII\Support\Request\GetRuleConfiguration;
@@ -140,7 +141,7 @@ class UpdateRequest extends FormRequest
'triggers.*.stop_processing' => [new IsBoolean()], 'triggers.*.stop_processing' => [new IsBoolean()],
'triggers.*.active' => [new IsBoolean()], 'triggers.*.active' => [new IsBoolean()],
'actions.*.type' => 'required|in:'.implode(',', $validActions), 'actions.*.type' => 'required|in:'.implode(',', $validActions),
'actions.*.value' => 'required_if:actions.*.type,'.$contextActions.'|ruleActionExpression|ruleActionValue', 'actions.*.value' => ['required_if:actions.*.type,'.$contextActions, new IsValidActionExpression(), 'ruleActionValue'],
'actions.*.stop_processing' => [new IsBoolean()], 'actions.*.stop_processing' => [new IsBoolean()],
'actions.*.active' => [new IsBoolean()], 'actions.*.active' => [new IsBoolean()],
'strict' => [new IsBoolean()], 'strict' => [new IsBoolean()],

View File

@@ -24,23 +24,42 @@ declare(strict_types=1);
namespace FireflyIII\Api\V1\Requests\Models\Rule; namespace FireflyIII\Api\V1\Requests\Models\Rule;
use FireflyIII\Rules\IsValidActionExpression;
use FireflyIII\Support\Request\ChecksLogin; use FireflyIII\Support\Request\ChecksLogin;
use FireflyIII\Support\Request\ConvertsDataTypes; use Illuminate\Contracts\Validation\Validator;
use Illuminate\Foundation\Http\FormRequest; use Illuminate\Foundation\Http\FormRequest;
use Illuminate\Validation\ValidationException;
/** /**
* Class ValidateExpressionRequest * Class ValidateExpressionRequest
*/ */
class ValidateExpressionRequest extends FormRequest class ValidateExpressionRequest extends FormRequest
{ {
use ConvertsDataTypes;
use ChecksLogin; use ChecksLogin;
/** public function rules(): array
* @return string
*/
public function getExpression(): string
{ {
return $this->convertString("expression"); return ['expression' => ['required', new IsValidActionExpression()]];
}
/**
* Handle a failed validation attempt.
*
* @param \Illuminate\Contracts\Validation\Validator $validator
* @return void
*
* @throws \Illuminate\Validation\ValidationException
*/
protected function failedValidation(Validator $validator): void
{
$error = $validator->errors()->first('expression');
throw new ValidationException(
$validator,
response()->json([
'valid' => false,
'error' => $error
], 200)
);
} }
} }

View File

@@ -24,6 +24,7 @@ declare(strict_types=1);
namespace FireflyIII\Http\Requests; namespace FireflyIII\Http\Requests;
use FireflyIII\Models\Rule; use FireflyIII\Models\Rule;
use FireflyIII\Rules\IsValidActionExpression;
use FireflyIII\Support\Request\ChecksLogin; use FireflyIII\Support\Request\ChecksLogin;
use FireflyIII\Support\Request\ConvertsDataTypes; use FireflyIII\Support\Request\ConvertsDataTypes;
use FireflyIII\Support\Request\GetRuleConfiguration; use FireflyIII\Support\Request\GetRuleConfiguration;
@@ -147,7 +148,7 @@ class RuleFormRequest extends FormRequest
'triggers.*.type' => 'required|in:'.implode(',', $validTriggers), 'triggers.*.type' => 'required|in:'.implode(',', $validTriggers),
'triggers.*.value' => sprintf('required_if:triggers.*.type,%s|max:1024|min:1|ruleTriggerValue', $contextTriggers), 'triggers.*.value' => sprintf('required_if:triggers.*.type,%s|max:1024|min:1|ruleTriggerValue', $contextTriggers),
'actions.*.type' => 'required|in:'.implode(',', $validActions), 'actions.*.type' => 'required|in:'.implode(',', $validActions),
'actions.*.value' => sprintf('required_if:actions.*.type,%s|min:0|max:1024|ruleActionExpression|ruleActionValue', $contextActions), 'actions.*.value' => [sprintf('required_if:actions.*.type,%s|min:0|max:1024', $contextActions), new IsValidActionExpression(), 'ruleActionValue'],
'strict' => 'in:0,1', 'strict' => 'in:0,1',
]; ];

View File

@@ -0,0 +1,48 @@
<?php
/*
*
* IsValidActionExpression.php
* Copyright (c) 2024 Michael Thomas
*
* This file is part of Firefly III (https://github.com/firefly-iii).
*
* This program is free software: you can redistribute it and/or modify
* it under the terms of the GNU Affero General Public License as
* published by the Free Software Foundation, either version 3 of the
* License, or (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU Affero General Public License for more details.
*
* You should have received a copy of the GNU Affero General Public License
* along with this program. If not, see <https://www.gnu.org/licenses/>.
*/
namespace FireflyIII\Rules;
use Closure;
use Illuminate\Contracts\Validation\ValidationRule;
use FireflyIII\TransactionRules\Expressions\ActionExpression;
class IsValidActionExpression implements ValidationRule
{
/**
* Check that the given action expression is syntactically valid.
*
* @param \Closure(string): \Illuminate\Translation\PotentiallyTranslatedString $fail
*/
public function validate(string $attribute, mixed $value, Closure $fail): void
{
$value ??= '';
$expr = new ActionExpression($value);
if (!$expr->isValid()) {
$fail('validation.rule_action_expression')->translate([
'error' => $expr->getValidationError()->getMessage()
]);
}
}
}

View File

@@ -254,27 +254,6 @@ class FireflyValidator extends Validator
return 1 === $count; return 1 === $count;
} }
public function validateRuleActionExpression(string $attribute, string $value = null): bool
{
$value ??= '';
$expr = new ActionExpression($value);
return $expr->isValid();
}
public function replaceRuleActionExpression(string $message, string $attribute): string
{
$value = $this->getValue($attribute);
$expr = new ActionExpression($value);
$err = $expr->getValidationError();
if ($err == null) {
return $message;
}
return str_replace(":error", $err->getMessage(), $message);
}
public function validateRuleActionValue(string $attribute, string $value = null): bool public function validateRuleActionValue(string $attribute, string $value = null): bool
{ {
// first, get the index from this string: // first, get the index from this string: