Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

[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.

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
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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;
}
}
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
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'));
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
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());
}
Morty Proxy This is a proxified and sanitized view of the page, visit original site.