Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: validate objects #801

Open
wants to merge 1 commit into
base: 2.x
Choose a base branch
from
Open

feat: validate objects #801

wants to merge 1 commit into from

Conversation

nikophil
Copy link
Member

@nikophil nikophil commented Feb 2, 2025

@nikophil nikophil changed the title validator feat: validate objects Feb 2, 2025
break;
}
}
$application = new Application(new TestKernel($_ENV['APP_ENV'], true));
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is easier for debugging


->set('.zenstruck_foundry.validation_listener', ValidationListener::class)
->args([service('validator')])
->tag('kernel.event_listener', ['event' => AfterInstantiate::class, 'method' => '__invoke'])
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was wondering if I should listen AfterPersist event, and trigger validation on this event for persisting factories, but it feels wrong to validate after persisting

$violations = $this->validator->validate($event->object);

if ($violations->count() > 0) {
throw new ValidationFailedException($event->object, $violations);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm wondering if we should throw this exception or another one from Foundry's namespace. This ones is kinda ugly:

Symfony\Component\Validator\Exception\ValidationFailedException: Object(Zenstruck\Foundry\Tests\Fixture\Entity\EntityForValidation).name:
    This value should not be blank. (code c1051bb4-d103-4f74-8988-acbcafc7fdc3)

Comment on lines +92 to +94
->arrayNode('validation')
->info('Automatically validate the objects created.')
->canBeEnabled()
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it seems logical to add this config under instantiator conf, but this could be moved outisde from it.

{
public function __construct(
#[ORM\Column()]
#[Assert\NotBlank()]
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello @nikophil, what about validation groups?

Copy link
Member Author

@nikophil nikophil Feb 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hi @COil 👋

yeah good call, I'll add them as a parameter of withValidation() method. Or maybe a new method withValidationGroups()

@nikophil nikophil requested a review from kbond February 2, 2025 21:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants