[Validator] Add the errorPath option to the Expression constraint #41921
Conversation
|
Hey! I think @lmasforne has recently worked with this code. Maybe they can help review this? Cheers! Carsonbot |
| @@ -44,6 +44,7 @@ public function validate($value, Constraint $constraint) | |||
|
|
|||
| if (!$this->getExpressionLanguage()->evaluate($constraint->expression, $variables)) { | |||
| $this->context->buildViolation($constraint->message) | |||
| ->atPath((string) $constraint->errorPath) | |||
There was a problem hiding this comment.
the cast to string is not needed
There was a problem hiding this comment.
looks like it's needed https://github.com/symfony/symfony/pull/41921/checks?check_run_id=2971187070#step:8:510
sidz
force-pushed
the
control-expression-constraint-error-path
branch
from
795c385 to
a5f396d
Compare
| @@ -65,6 +67,7 @@ public function __construct( | |||
|
|
|||
| $this->message = $message ?? $this->message; | |||
| $this->values = $values ?? $this->values; | |||
| $this->errorPath = $errorPath ?? $this->errorPath; | |||
There was a problem hiding this comment.
$this->errorPath = $errorPath; is enough here a well
| @@ -44,6 +44,7 @@ public function validate($value, Constraint $constraint) | |||
|
|
|||
| if (!$this->getExpressionLanguage()->evaluate($constraint->expression, $variables)) { | |||
| $this->context->buildViolation($constraint->message) | |||
| ->atPath($constraint->errorPath) | |||
There was a problem hiding this comment.
I guess this should be called only when $constraint->errorPath !== null
There was a problem hiding this comment.
or cast to string which was originally added. So what would be the best way?
cc/ @nicolas-grekas
Is this needed ? Since you can already define the expression on the property for which you want the violation message to be associated with while still accessing the class context when using (see the "Mapping the Error to a Specific Field" section from https://symfony.com/doc/current/reference/constraints/Expression.html) |
|
@ogizanagi I see your point but let say I'm building an API and I need to validate smth against class and return violation property path equal to Based on your suggestion I'd say that Target = Class is not needed an need to be removed from |
sidz
force-pushed
the
control-expression-constraint-error-path
branch
3 times, most recently
from
fd7fb14 to
06fdea0
Compare
sidz
force-pushed
the
control-expression-constraint-error-path
branch
from
4dc63c0 to
3a79485
Compare
I don't think so, since it's still valid to have the expression and error mapped to the class, as it'll be exposed to another object property when embedded. |
sidz
force-pushed
the
control-expression-constraint-error-path
branch
from
6df3f61 to
3a79485
Compare
|
looks like CI is failed due to unrelated to this PR reason |
|
Should I rebase my branch against 6.1 as milestone 5.4 has been replaced by 6.1? |
PR adds the
errorPathoption to the Expression constraint.This option can be useful for cases when Expression constraint is applied on the Class Level and we need to define the error path.
Current behavior put an empty string to the
propertyPath.It works in the same way like in the
Uniqueconstraint.