YCRO-300 add backend validation for info collector forms #56
Conversation
| @@ -0,0 +1,2 @@ | |||
| this_value_should_not_be_blank: 'This value should not be blank.' | |||
| email_is_not_valid: 'Email is not valid.' No newline at end of file | |||
| static fn ($attr) => $value->value->attribute($attr) === null || $value->value->attribute($attr) === '' || $value->value->attribute($attr) === false, | ||
| ) !== [] | ||
| ) { | ||
| if (!property_exists($constraint, 'message')) { |
There was a problem hiding this comment.
Why do we need to check for existence of message property since we know we're dealing with RequiredField constraint which has the message property?
| { | ||
| public function validate(mixed $value, Constraint $constraint): void | ||
| { | ||
| /** @var FieldData $value */ |
There was a problem hiding this comment.
We should check if $value and $constraint are of correct type and just return if not, instead of using type coercion. Something like https://github.com/netgen-layouts/layouts-core/blob/master/lib/Validator/LayoutNameValidator.php#L33-L43
| @@ -0,0 +1,2 @@ | |||
| this_value_should_not_be_blank: 'This value should not be blank.' | |||
There was a problem hiding this comment.
We should add prefixes to these message identifiers:
required_field.this_value_should_not_be_blank and custom_email.email_is_not_valid.
This way we allow adding more messages in the future.
|
|
||
| $emailValue = $value->value->email; | ||
|
|
||
| if (filter_var($emailValue, FILTER_VALIDATE_EMAIL) === false) { |
There was a problem hiding this comment.
Instead of filter_var, we should use egulias/email-validator which is better and closer to specification.
| public function buildForm(FormBuilderInterface $builder, array $options): void | ||
| { | ||
| foreach ($builder->getData()->getCollectedFields() as $identifier => $fieldData) { | ||
| /** @var FieldData $fieldData */ |
There was a problem hiding this comment.
Do we need this @var? If yes, please use FQCN in the typehint.
| { | ||
| $resolver->setDefaults([ | ||
| 'translation_domain' => 'validators', | ||
| 'attr' => [ |
There was a problem hiding this comment.
Why do we need this? Isn't this something for template to control and not the form extension?
| public function buildForm(FormBuilderInterface $builder, array $options): void | ||
| { | ||
| foreach ($builder->getData()->getCollectedFields() as $identifier => $fieldData) { | ||
| /** @var FieldData $fieldData */ |
There was a problem hiding this comment.
Do we need this @var? If yes, please use FQCN in the typehint.
| return [InformationCollectionType::class]; | ||
| } | ||
|
|
||
| public function configureOptions(OptionsResolver $resolver): void |
There was a problem hiding this comment.
We don't need this configureOptions method, we're not translating anything here, besides validators domain is default anyways.
| public function configureOptions(OptionsResolver $resolver): void | ||
| { | ||
| $resolver->setDefaults([ | ||
| 'translation_domain' => 'validators', |
There was a problem hiding this comment.
We don't need this here, we're not translating anything here, besides validators domain is default anyways.
I’ve added extensions for required field and email validation on the backend. These were missing in the bundle, but since the issue was already solved in charter split, I reused that solution here