8000 [Form] fix group sequence based validation by xabbuh · Pull Request #20975 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

[Form] fix group sequence based validation #20975

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Dec 19, 2016
Merged

[Form] fix group sequence based validation #20975

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading

Uh oh!

There was an error while loading. Please reload this page.
Diff view
Diff view
[Form] fix group sequence based validation
  • Loading branch information
@xabbuh
xabbuh committed Dec 17, 2016
commit fb91f74b34f2ea8064ad790677c4cb8207848ef0
15 changes: 11 additions & 4 deletions src/Symfony/Component/Form/Extension/Validator/Constraints/FormValidator.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@

use Symfony\Component\Form\FormInterface;
use Symfony\Component\Validator\Constraint;
use Symfony\Component\Validator\Constraints\GroupSequence;
use Symfony\Component\Validator\ConstraintValidator;
use Symfony\Component\Validator\Context\ExecutionContextInterface;
use Symfony\Component\Validator\Exception\UnexpectedTypeException;
Expand Down Expand Up @@ -49,10 +50,12 @@ public function validate($form, Constraint $constraint)

// Validate the data against its own constraints
if (self::allowDataWalking($form)) {
foreach ($groups as $group) {
if ($validator) {
$validator->atPath('data')->validate($form->getData(), null, $group);
} else {
if ($validator) {
if (is_array($groups) && count($groups) > 0 || $groups instanceof GroupSequence && count($groups->groups) > 0) {
$validator->atPath('data')->validate($form->getData(), null, $groups);
}
} else {
foreach ($groups as $group) {
// 2.4 API
$this->context->validate($form->getData(), 'data', $group, true);
}
Expand Down Expand Up @@ -218,6 +221,10 @@ private static function resolveValidationGroups($groups, FormInterface $form)
$groups = call_user_func($groups, $form);
}

if ($groups instanceof GroupSequence) {
return $groups;
}

return (array) $groups;
}
}
5 changes: 5 additions & 0 deletions src/Symfony/Component/Form/Extension/Validator/Type/BaseValidatorExtension.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
use Symfony\Component\Form\AbstractTypeExtension;
use Symfony\Component\OptionsResolver\Options;
use Symfony\Component\OptionsResolver\OptionsResolver;
use Symfony\Component\Validator\Constraints\GroupSequence;

/**
* Encapsulates common logic of {@link FormTypeValidatorExtension} and
Expand Down Expand Up @@ -42,6 +43,10 @@ public function configureOptions(OptionsResolver $resolver)
return $groups;
}

if ($groups instanceof GroupSequence) {
return $groups;
}

return (array) $groups;
};

Expand Down
50 changes: 30 additions & 20 deletions src/Symfony/Component/Form/Tests/Extension/Validator/Constraints/FormValidatorTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
use Symfony\Component\Form\Extension\Validator\Constraints\Form;
use Symfony\Component\Form\Extension\Validator\Constraints\FormValidator;
use Symfony\Component\Form\SubmitButtonBuilder;
use Symfony\Component\Validator\Constraints\GroupSequence;
use Symfony\Component\Validator\Constraints\NotNull;
use Symfony\Component\Validator\Constraints\NotBlank;
use Symfony\Component\Validator\Tests\Constraints\AbstractConstraintValidatorTest;
Expand Down Expand Up @@ -73,8 +74,7 @@ public function testValidate()
->setData($object)
->getForm();

$this->expectValidateAt(0, 'data', $object, 'group1');
$this->expectValidateAt(1, 'data', $object, 'group2');
$this->expectValidateAt(0, 'data', $object, array('group1', 'group2'));

$this->validator->validate($form, new Form());

Expand All @@ -96,12 +96,11 @@ public function testValidateConstraints()
->getForm();

// First default constraints
$this->expectValidateAt(0, 'data', $object, 'group1');
$this->expectValidateAt(1, 'data', $object, 'group2');
$this->expectValidateAt(0, 'data', $object, array('group1', 'group2'));

// Then custom constraints
$this->expectValidateValueAt(2, 'data', $object, $constraint1, 'group1');
$this->expectValidateValueAt(3, 'data', $object, $constraint2, 'group2');
$this->expectValidateValueAt(1, 'data', $object, $constraint1, 'group1');
$this->expectValidateValueAt(2, 'data', $object, $constraint2, 'group2');

$this->validator->validate($form, new Form());

Expand Down Expand Up @@ -135,7 +134,7 @@ public function testMissingConstraintIndex()
$form = new FormBuilder('name', '\stdClass', $this->dispatcher, $this->factory);
$form = $form->setData($object)->getForm();

$this->expectValidateAt(0, 'data', $object, 'Default');
$this->expectValidateAt(0, 'data', $object, array('Default'));

$this->validator->validate($form, new Form());

Expand Down Expand Up @@ -347,15 +346,29 @@ function () { throw new TransformationFailedException(); }
}

public function testHandleCallbackValidationGroups()
{
$object = $this->getMock('\stdClass');
$options = array('validation_groups' => new GroupSequence(array('group1', 'group2')));
$form = $this->getBuilder('name', '\stdClass', $options)
->setData($object)
->getForm();

$this->expectValidateAt(0, 'data', $object, new GroupSequence(array('group1', 'group2')));

$this->validator->validate($form, new Form());

$this->assertNoViolation();
}

public function testHandleGroupSequenceValidationGroups()
{
$object = $this->getMock('\stdClass');
$options = array('validation_groups' => array($this, 'getValidationGroups'));
$form = $this->getBuilder('name', '\stdClass', $options)
->setData($object)
->getForm();

$this->expectValidateAt(0, 'data', $object, 'group1');
$this->expectValidateAt(1, 'data', $object, 'group2');
$this->expectValidateAt(0, 'data', $object, array('group1', 'group2'));
EDBE
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like this bug fix is breaking BC with this behavioral change. But it seems the code was broken so I'm still 👍 for this one.

Copy link
Member Author

Choose a reason for hiding this comment

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

@HeahDude Which BC break do you refer to?

Copy link
Contributor

Choose a reason for hiding this comment

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

Arff, got confused by your refactoring :)


$this->validator->validate($form, new Form());

Expand All @@ -370,7 +383,7 @@ public function testDontExecuteFunctionNames()
->setData($object)
->getForm();

$this->expectValidateAt(0, 'data', $object, 'header');
$this->expectValidateAt(0, 'data', $object, array('header'));

$this->validator->validate($form, new Form());

Expand All @@ -387,8 +400,7 @@ public function testHandleClosureValidationGroups()
->setData($object)
->getForm();

$this->expectValidateAt(0, 'data', $object, 'group1');
$this->expectValidateAt(1, 'data', $object, 'group2');
$this->expectValidateAt(0, 'data', $object, array('group1', 'group2'));

$this->validator->validate($form, new Form());

Expand All @@ -414,7 +426,7 @@ public function testUseValidationGroupOfClickedButton()

$parent->submit(array('name' => $object, 'submit' => ''));

$this->expectValidateAt(0, 'data', $object, 'button_group');
$this->expectValidateAt(0, 'data', $object, array('button_group'));

$this->validator->validate($form, new Form());

Expand All @@ -440,7 +452,7 @@ public function testDontUseValidationGroupOfUnclickedButton()

$form->setData($object);

$this->expectValidateAt(0, 'data', $object, 'form_group');
$this->expectValidateAt(0, 'data', $object, array('form_group'));

$this->validator->validate($form, new Form());

Expand All @@ -464,7 +476,7 @@ public function testUseInheritedValidationGroup()

$form->setData($object);

$this->expectValidateAt(0, 'data', $object, 'group');
$this->expectValidateAt(0, 'data', $object, array('group'));

$this->validator->validate($form, new Form());

Expand All @@ -488,8 +500,7 @@ public function testUseInheritedCallbackValidationGroup()

$form->setData($object);

$this->expectValidateAt(0, 'data', $object, 'group1');
$this->expectValidateAt(1, 'data', $object, 'group2');
$this->expectValidateAt(0, 'data', $object, array('group1', 'group2'));

$this->validator->validate($form, new Form());

Expand All @@ -515,8 +526,7 @@ public function testUseInheritedClosureValidationGroup()

$form->setData($object);

$this->expectValidateAt(0, 'data', $object, 'group1');
$this->expectValidateAt(1, 'data', $object, 'group2');
$this->expectValidateAt(0, 'data', $object, array('group1', 'group2'));

$this->validator->validate($form, new Form());

Expand All @@ -530,7 +540,7 @@ public function testAppendPropertyPath()
->setData($object)
->getForm();

$this->expectValidateAt(0, 'data', $object, 'Default');
$this->expectValidateAt(0, 'data', $object, array('Default'));

$this->validator->validate($form, new Form());

Expand Down
10 changes: 10 additions & 0 deletions src/Symfony/Component/Form/Tests/Extension/Validator/Type/BaseValidatorExtensionTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
namespace Symfony\Component\Form\Tests\Extension\Validator\Type;

use Symfony\Component\Form\Test\FormInterface;
use Symfony\Component\Validator\Constraints\GroupSequence;

/**
* @author Bernhard Schussek <bschussek@gmail.com>
Expand Down Expand Up @@ -70,5 +71,14 @@ public function testValidationGroupsCanBeSetToClosure()
$this->assertInternalType('callable', $form->getConfig()->getOption('validation_groups'));
}

public function testValidationGroupsCanBeSetToGroupSequence()
{
$form = $this->createForm(array(
'validation_groups' => new GroupSequence(array('group1', 'group2')),
));

$this->assertInstanceOf('Symfony\Component\Validator\Constraints\GroupSequence', $form->getConfig()->getOption('validation_groups'));
}

abstract protected function createForm(array $options = array());
}
0