feat: surface expression validation errors when creating or updating rules

This commit is contained in:
Michael Thomas 2024-03-07 12:23:32 -05:00
parent b572c1dcd3
commit 438f602961
7 changed files with 58 additions and 14 deletions

View File

@ -74,15 +74,14 @@ class ExpressionController extends Controller
$expressionLanguage = ExpressionLanguageFactory::get(); $expressionLanguage = ExpressionLanguageFactory::get();
$evaluator = new ActionExpressionEvaluator($expressionLanguage, $expr); $evaluator = new ActionExpressionEvaluator($expressionLanguage, $expr);
try { if ($evaluator->isValid()) {
$evaluator->lint();
return response()->json([ return response()->json([
"valid" => true, "valid" => true,
]); ]);
} catch (SyntaxError $e) { } else {
return response()->json([ return response()->json([
"valid" => false, "valid" => false,
"error" => $e->getMessage() "error" => $evaluator->getValidationError()->getMessage()
]); ]);
} }
} }

View File

@ -123,7 +123,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.'|ruleActionValue', 'actions.*.value' => 'required_if:actions.*.type,'.$contextActions.'|ruleActionExpression|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

@ -140,7 +140,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.'|ruleActionValue', 'actions.*.value' => 'required_if:actions.*.type,'.$contextActions.'|ruleActionExpression|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

@ -147,7 +147,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|ruleActionValue', $contextActions), 'actions.*.value' => sprintf('required_if:actions.*.type,%s|min:0|max:1024|ruleActionExpression|ruleActionValue', $contextActions),
'strict' => 'in:0,1', 'strict' => 'in:0,1',
]; ];

View File

@ -31,9 +31,10 @@ class ActionExpressionEvaluator
{ {
private static array $NAMES = array("transaction"); private static array $NAMES = array("transaction");
private ExpressionLanguage $expressionLanguage;
private string $expr; private string $expr;
private bool $isExpression; private bool $isExpression;
private ExpressionLanguage $expressionLanguage; private ?SyntaxError $validationError;
public function __construct(ExpressionLanguage $expressionLanguage, string $expr) public function __construct(ExpressionLanguage $expressionLanguage, string $expr)
{ {
@ -41,6 +42,7 @@ class ActionExpressionEvaluator
$this->expr = $expr; $this->expr = $expr;
$this->isExpression = self::isExpression($expr); $this->isExpression = self::isExpression($expr);
$this->validationError = $this->validate();
} }
private static function isExpression(string $expr): bool private static function isExpression(string $expr): bool
@ -48,17 +50,17 @@ class ActionExpressionEvaluator
return str_starts_with($expr, "="); return str_starts_with($expr, "=");
} }
public function isValid(): bool private function validate(): ?SyntaxError
{ {
if (!$this->isExpression) { if (!$this->isExpression) {
return true; return null;
} }
try { try {
$this->lint(array()); $this->lint();
return true; return null;
} catch (SyntaxError $e) { } catch (SyntaxError $e) {
return false; return $e;
} }
} }
@ -67,7 +69,7 @@ class ActionExpressionEvaluator
$this->expressionLanguage->lint($expr, self::$NAMES); $this->expressionLanguage->lint($expr, self::$NAMES);
} }
public function lint(): void private function lint(): void
{ {
if (!$this->isExpression) { if (!$this->isExpression) {
return; return;
@ -76,6 +78,16 @@ class ActionExpressionEvaluator
$this->lintExpression(substr($this->expr, 1)); $this->lintExpression(substr($this->expr, 1));
} }
public function isValid(): bool
{
return $this->validationError === null;
}
public function getValidationError()
{
return $this->validationError;
}
private function evaluateExpression(string $expr, array $journal): string private function evaluateExpression(string $expr, array $journal): string
{ {
$result = $this->expressionLanguage->evaluate($expr, [ $result = $this->expressionLanguage->evaluate($expr, [

View File

@ -35,6 +35,8 @@ use FireflyIII\Repositories\Budget\BudgetRepositoryInterface;
use FireflyIII\Repositories\PiggyBank\PiggyBankRepositoryInterface; use FireflyIII\Repositories\PiggyBank\PiggyBankRepositoryInterface;
use FireflyIII\Services\Password\Verifier; use FireflyIII\Services\Password\Verifier;
use FireflyIII\Support\ParseDateString; use FireflyIII\Support\ParseDateString;
use FireflyIII\TransactionRules\Expressions\ActionExpressionEvaluator;
use FireflyIII\TransactionRules\Factory\ExpressionLanguageFactory;
use FireflyIII\User; use FireflyIII\User;
use Illuminate\Validation\Validator; use Illuminate\Validation\Validator;
use PragmaRX\Google2FA\Exceptions\IncompatibleWithGoogleAuthenticatorException; use PragmaRX\Google2FA\Exceptions\IncompatibleWithGoogleAuthenticatorException;
@ -253,6 +255,31 @@ class FireflyValidator extends Validator
return 1 === $count; return 1 === $count;
} }
public function validateRuleActionExpression(string $attribute, string $value = null): bool
{
$value ??= '';
$el = ExpressionLanguageFactory::get();
$evaluator = new ActionExpressionEvaluator($el, $value);
return $evaluator->isValid();
}
public function replaceRuleActionExpression(string $message, string $attribute): string
{
$value = $this->getValue($attribute);
$el = ExpressionLanguageFactory::get();
$evaluator = new ActionExpressionEvaluator($el, $value);
$err = $evaluator->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:
@ -268,6 +295,11 @@ class FireflyValidator extends Validator
return false; return false;
} }
// if value is an expression, assume valid
if (str_starts_with($value, '=')) {
return true;
}
// if it's set_budget, verify the budget name: // if it's set_budget, verify the budget name:
if ('set_budget' === $actionType) { if ('set_budget' === $actionType) {
/** @var BudgetRepositoryInterface $repository */ /** @var BudgetRepositoryInterface $repository */

View File

@ -46,6 +46,7 @@ return [
'reconciled_forbidden_field' => 'This transaction is already reconciled, you cannot change the ":field"', 'reconciled_forbidden_field' => 'This transaction is already reconciled, you cannot change the ":field"',
'deleted_user' => 'Due to security constraints, you cannot register using this email address.', 'deleted_user' => 'Due to security constraints, you cannot register using this email address.',
'rule_trigger_value' => 'This value is invalid for the selected trigger.', 'rule_trigger_value' => 'This value is invalid for the selected trigger.',
'rule_action_expression' => 'Invalid expression. :error',
'rule_action_value' => 'This value is invalid for the selected action.', 'rule_action_value' => 'This value is invalid for the selected action.',
'file_already_attached' => 'Uploaded file ":name" is already attached to this object.', 'file_already_attached' => 'Uploaded file ":name" is already attached to this object.',
'file_attached' => 'Successfully uploaded file ":name".', 'file_attached' => 'Successfully uploaded file ":name".',