merged branch bschussek/issue3288 (PR #3290)
Commits -------22c8f80
[Form] Fixed issues mentioned in the PR comments3b1b570
[Form] Fixed: The "date", "time" and "datetime" types can be initialized with \DateTime objects88ef52d
[Form] Improved FormType::getDefaultOptions() to see default options defined in parent typesb9facfc
[Form] Removed undefined variables in exception constructor Discussion ---------- [Form] Fixed: "date", "time" and "datetime" fields can be initialized with \DateTime objects Bug fix: yes Feature addition: yes Backwards compatibility break: **yes** Symfony2 tests pass: yes Fixes the following tickets: #3288 Todo: - ![Travis Build Status](https://secure.travis-ci.org/bschussek/symfony.png?branch=issue3288) Fixed exception that was thrown when doing: $builder->add('createdAt', 'date', array('data' => new \DateTime())); On a side note, the options passed to `FieldType::getDefaultOptions` now always also contain the default options of any parent types. This is necessary if you want to be independent of how `getDefaultOptions` is implemented in the parent type and still rely on the already defined values. As a result, `FieldType::getParent` doesn't see default options anymore. This shouldn't be a big problem, because this method only relies on options in few cases. If it does, options now need to be checked for existence with `isset` before being used (BC break). --------------------------------------------------------------------------- by bschussek at 2012-02-09T16:14:46Z @fabpot Ready to merge. --------------------------------------------------------------------------- by bschussek at 2012-02-10T12:15:04Z @fabpot Ready to merge
This commit is contained in:
commit
be2e67c1ab
@ -208,6 +208,8 @@ To get the diff between two versions, go to https://github.com/symfony/symfony/c
|
|||||||
* forms now don't create an empty object anymore if they are completely
|
* forms now don't create an empty object anymore if they are completely
|
||||||
empty and not required. The empty value for such forms is null.
|
empty and not required. The empty value for such forms is null.
|
||||||
* added constant Guess::VERY_HIGH_CONFIDENCE
|
* added constant Guess::VERY_HIGH_CONFIDENCE
|
||||||
|
* FormType::getDefaultOptions() now sees default options defined by parent types
|
||||||
|
* [BC BREAK] FormType::getParent() does not see default options anymore
|
||||||
|
|
||||||
### HttpFoundation
|
### HttpFoundation
|
||||||
|
|
||||||
|
@ -229,3 +229,22 @@ UPGRADE FROM 2.0 to 2.1
|
|||||||
return false;
|
return false;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
* The options passed to `getParent` of the form types don't contain default
|
||||||
|
options anymore
|
||||||
|
|
||||||
|
You should check with `isset` if options exist before checking their value.
|
||||||
|
|
||||||
|
Before:
|
||||||
|
|
||||||
|
public function getParent(array $options)
|
||||||
|
{
|
||||||
|
return 'single_text' === $options['widget'] ? 'text' : 'choice';
|
||||||
|
}
|
||||||
|
|
||||||
|
After:
|
||||||
|
|
||||||
|
public function getParent(array $options)
|
||||||
|
{
|
||||||
|
return isset($options['widget']) && 'single_text' === $options['widget'] ? 'text' : 'choice';
|
||||||
|
}
|
||||||
|
@ -50,7 +50,6 @@ abstract class DoctrineType extends AbstractType
|
|||||||
'property' => null,
|
'property' => null,
|
||||||
'query_builder' => null,
|
'query_builder' => null,
|
||||||
'loader' => null,
|
'loader' => null,
|
||||||
'choices' => null,
|
|
||||||
'group_by' => null,
|
'group_by' => null,
|
||||||
);
|
);
|
||||||
|
|
||||||
|
@ -107,7 +107,7 @@ class ChoicesToBooleanArrayTransformer implements DataTransformerInterface
|
|||||||
}
|
}
|
||||||
|
|
||||||
if (count($unknown) > 0) {
|
if (count($unknown) > 0) {
|
||||||
throw new TransformationFailedException('The choices "' . implode('", "', $unknown, $code, $previous) . '" where not found');
|
throw new TransformationFailedException('The choices "' . implode('", "', $unknown) . '" were not found');
|
||||||
}
|
}
|
||||||
|
|
||||||
return $result;
|
return $result;
|
||||||
|
@ -153,7 +153,7 @@ class ChoiceType extends AbstractType
|
|||||||
'multiple' => false,
|
'multiple' => false,
|
||||||
'expanded' => false,
|
'expanded' => false,
|
||||||
'choice_list' => null,
|
'choice_list' => null,
|
||||||
'choices' => array(),
|
'choices' => null,
|
||||||
'preferred_choices' => array(),
|
'preferred_choices' => array(),
|
||||||
'value_strategy' => ChoiceList::GENERATE,
|
'value_strategy' => ChoiceList::GENERATE,
|
||||||
'index_strategy' => ChoiceList::GENERATE,
|
'index_strategy' => ChoiceList::GENERATE,
|
||||||
@ -170,7 +170,7 @@ class ChoiceType extends AbstractType
|
|||||||
*/
|
*/
|
||||||
public function getParent(array $options)
|
public function getParent(array $options)
|
||||||
{
|
{
|
||||||
return $options['expanded'] ? 'form' : 'field';
|
return isset($options['expanded']) && $options['expanded'] ? 'form' : 'field';
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
|
@ -158,6 +158,11 @@ class DateTimeType extends AbstractType
|
|||||||
'widget' => null,
|
'widget' => null,
|
||||||
// This will overwrite "empty_value" child options
|
// This will overwrite "empty_value" child options
|
||||||
'empty_value' => null,
|
'empty_value' => null,
|
||||||
|
// If initialized with a \DateTime object, FieldType initializes
|
||||||
|
// this option to "\DateTime". Since the internal, normalized
|
||||||
|
// representation is not \DateTime, but an array, we need to unset
|
||||||
|
// this option.
|
||||||
|
'data_class' => null,
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
@ -200,7 +205,7 @@ class DateTimeType extends AbstractType
|
|||||||
*/
|
*/
|
||||||
public function getParent(array $options)
|
public function getParent(array $options)
|
||||||
{
|
{
|
||||||
return 'single_text' === $options['widget'] ? 'field' : 'form';
|
return isset($options['widget']) && 'single_text' === $options['widget'] ? 'field' : 'form';
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
|
@ -183,6 +183,11 @@ class DateType extends AbstractType
|
|||||||
// them like immutable value objects
|
// them like immutable value objects
|
||||||
'by_reference' => false,
|
'by_reference' => false,
|
||||||
'error_bubbling' => false,
|
'error_bubbling' => false,
|
||||||
|
// If initialized with a \DateTime object, FieldType initializes
|
||||||
|
// this option to "\DateTime". Since the internal, normalized
|
||||||
|
// representation is not \DateTime, but an array, we need to unset
|
||||||
|
// this option.
|
||||||
|
'data_class' => null,
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
@ -211,7 +216,7 @@ class DateType extends AbstractType
|
|||||||
*/
|
*/
|
||||||
public function getParent(array $options)
|
public function getParent(array $options)
|
||||||
{
|
{
|
||||||
return 'single_text' === $options['widget'] ? 'field' : 'form';
|
return isset($options['widget']) && 'single_text' === $options['widget'] ? 'field' : 'form';
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
|
@ -156,6 +156,11 @@ class TimeType extends AbstractType
|
|||||||
// them like immutable value objects
|
// them like immutable value objects
|
||||||
'by_reference' => false,
|
'by_reference' => false,
|
||||||
'error_bubbling' => false,
|
'error_bubbling' => false,
|
||||||
|
// If initialized with a \DateTime object, FieldType initializes
|
||||||
|
// this option to "\DateTime". Since the internal, normalized
|
||||||
|
// representation is not \DateTime, but an array, we need to unset
|
||||||
|
// this option.
|
||||||
|
'data_class' => null,
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
@ -184,7 +189,7 @@ class TimeType extends AbstractType
|
|||||||
*/
|
*/
|
||||||
public function getParent(array $options)
|
public function getParent(array $options)
|
||||||
{
|
{
|
||||||
return 'single_text' === $options['widget'] ? 'field' : 'form';
|
return isset($options['widget']) && 'single_text' === $options['widget'] ? 'field' : 'form';
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
|
@ -31,7 +31,7 @@ class TimezoneType extends AbstractType
|
|||||||
'value_strategy' => ChoiceList::COPY_CHOICE,
|
'value_strategy' => ChoiceList::COPY_CHOICE,
|
||||||
);
|
);
|
||||||
|
|
||||||
if (!isset($options['choice_list']) && !isset($options['choices'])) {
|
if (empty($options['choice_list']) && empty($options['choices'])) {
|
||||||
$defaultOptions['choices'] = self::getTimezones();
|
$defaultOptions['choices'] = self::getTimezones();
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -213,16 +213,20 @@ class FormFactory implements FormFactoryInterface
|
|||||||
*/
|
*/
|
||||||
public function createNamedBuilder($type, $name, $data = null, array $options = array(), FormBuilder $parent = null)
|
public function createNamedBuilder($type, $name, $data = null, array $options = array(), FormBuilder $parent = null)
|
||||||
{
|
{
|
||||||
$builder = null;
|
|
||||||
$types = array();
|
|
||||||
$knownOptions = array();
|
|
||||||
$passedOptions = array_keys($options);
|
|
||||||
$optionValues = array();
|
|
||||||
|
|
||||||
if (!array_key_exists('data', $options)) {
|
if (!array_key_exists('data', $options)) {
|
||||||
$options['data'] = $data;
|
$options['data'] = $data;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
$builder = null;
|
||||||
|
$types = array();
|
||||||
|
$defaultOptions = array();
|
||||||
|
$optionValues = array();
|
||||||
|
$passedOptions = $options;
|
||||||
|
|
||||||
|
// Bottom-up determination of the type hierarchy
|
||||||
|
// Start with the actual type and look for the parent type
|
||||||
|
// The complete hierarchy is saved in $types, the first entry being
|
||||||
|
// the root and the last entry being the leaf (the concrete type)
|
||||||
while (null !== $type) {
|
while (null !== $type) {
|
||||||
if ($type instanceof FormTypeInterface) {
|
if ($type instanceof FormTypeInterface) {
|
||||||
if ($type->getName() == $type->getParent($options)) {
|
if ($type->getName() == $type->getParent($options)) {
|
||||||
@ -236,7 +240,23 @@ class FormFactory implements FormFactoryInterface
|
|||||||
throw new UnexpectedTypeException($type, 'string or Symfony\Component\Form\FormTypeInterface');
|
throw new UnexpectedTypeException($type, 'string or Symfony\Component\Form\FormTypeInterface');
|
||||||
}
|
}
|
||||||
|
|
||||||
$defaultOptions = $type->getDefaultOptions($options);
|
array_unshift($types, $type);
|
||||||
|
|
||||||
|
// getParent() cannot see default options set by this type nor
|
||||||
|
// default options set by parent types
|
||||||
|
// As a result, the options always have to be checked for
|
||||||
|
// existence with isset() before using them in this method.
|
||||||
|
$type = $type->getParent($options);
|
||||||
|
}
|
||||||
|
|
||||||
|
// Top-down determination of the options and default options
|
||||||
|
foreach ($types as $type) {
|
||||||
|
// Merge the default options of all types to an array of default
|
||||||
|
// options. Default options of children override default options
|
||||||
|
// of parents.
|
||||||
|
// Default options of ancestors are already visible in the $options
|
||||||
|
// array passed to the following methods.
|
||||||
|
$defaultOptions = array_replace($defaultOptions, $type->getDefaultOptions($options));
|
||||||
$optionValues = array_merge_recursive($optionValues, $type->getAllowedOptionValues($options));
|
$optionValues = array_merge_recursive($optionValues, $type->getAllowedOptionValues($options));
|
||||||
|
|
||||||
foreach ($type->getExtensions() as $typeExtension) {
|
foreach ($type->getExtensions() as $typeExtension) {
|
||||||
@ -244,20 +264,23 @@ class FormFactory implements FormFactoryInterface
|
|||||||
$optionValues = array_merge_recursive($optionValues, $typeExtension->getAllowedOptionValues($options));
|
$optionValues = array_merge_recursive($optionValues, $typeExtension->getAllowedOptionValues($options));
|
||||||
}
|
}
|
||||||
|
|
||||||
$options = array_replace($defaultOptions, $options);
|
// In each turn, the options are replaced by the combination of
|
||||||
$knownOptions = array_merge($knownOptions, array_keys($defaultOptions));
|
// the currently known default options and the passed options.
|
||||||
array_unshift($types, $type);
|
// It is important to merge with $passedOptions and not with
|
||||||
$type = $type->getParent($options);
|
// $options, otherwise default options of parents would override
|
||||||
|
// default options of child types.
|
||||||
|
$options = array_replace($defaultOptions, $passedOptions);
|
||||||
}
|
}
|
||||||
|
|
||||||
$type = end($types);
|
$type = end($types);
|
||||||
|
$knownOptions = array_keys($defaultOptions);
|
||||||
$diff = array_diff(self::$requiredOptions, $knownOptions);
|
$diff = array_diff(self::$requiredOptions, $knownOptions);
|
||||||
|
|
||||||
if (count($diff) > 0) {
|
if (count($diff) > 0) {
|
||||||
throw new TypeDefinitionException(sprintf('Type "%s" should support the option(s) "%s"', $type->getName(), implode('", "', $diff)));
|
throw new TypeDefinitionException(sprintf('Type "%s" should support the option(s) "%s"', $type->getName(), implode('", "', $diff)));
|
||||||
}
|
}
|
||||||
|
|
||||||
$diff = array_diff($passedOptions, $knownOptions);
|
$diff = array_diff(array_keys($passedOptions), $knownOptions);
|
||||||
|
|
||||||
if (count($diff) > 1) {
|
if (count($diff) > 1) {
|
||||||
throw new CreationException(sprintf('The options "%s" do not exist. Known options are: "%s"', implode('", "', $diff), implode('", "', $knownOptions)));
|
throw new CreationException(sprintf('The options "%s" do not exist. Known options are: "%s"', implode('", "', $diff), implode('", "', $knownOptions)));
|
||||||
|
@ -258,4 +258,12 @@ class DateTimeTypeTest extends LocalizedTestCase
|
|||||||
$this->assertEquals(array(new FormError('Customized invalid message', array())), $form['date']->getErrors());
|
$this->assertEquals(array(new FormError('Customized invalid message', array())), $form['date']->getErrors());
|
||||||
$this->assertEquals(array(new FormError('Customized invalid message', array())), $form['time']->getErrors());
|
$this->assertEquals(array(new FormError('Customized invalid message', array())), $form['time']->getErrors());
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Bug fix
|
||||||
|
public function testInitializeWithDateTime()
|
||||||
|
{
|
||||||
|
// Throws an exception if "data_class" option is not explicitely set
|
||||||
|
// to null in the type
|
||||||
|
$this->factory->create('datetime', new \DateTime());
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
@ -530,4 +530,12 @@ class DateTypeTest extends LocalizedTestCase
|
|||||||
|
|
||||||
$this->assertSame('single_text', $view->get('widget'));
|
$this->assertSame('single_text', $view->get('widget'));
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Bug fix
|
||||||
|
public function testInitializeWithDateTime()
|
||||||
|
{
|
||||||
|
// Throws an exception if "data_class" option is not explicitely set
|
||||||
|
// to null in the type
|
||||||
|
$this->factory->create('date', new \DateTime());
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
@ -221,6 +221,42 @@ class FieldTypeTest extends TypeTestCase
|
|||||||
$this->assertEquals($author, $form->getData());
|
$this->assertEquals($author, $form->getData());
|
||||||
}
|
}
|
||||||
|
|
||||||
|
public function testBindWithEmptyDataCreatesObjectIfInitiallyBoundWithObject()
|
||||||
|
{
|
||||||
|
$form = $this->factory->create('form', null, array(
|
||||||
|
// data class is inferred from the passed object
|
||||||
|
'data' => new Author(),
|
||||||
|
'required' => false,
|
||||||
|
));
|
||||||
|
$form->add($this->factory->createNamed('field', 'firstName'));
|
||||||
|
$form->add($this->factory->createNamed('field', 'lastName'));
|
||||||
|
|
||||||
|
$form->setData(null);
|
||||||
|
// partially empty, still an object is created
|
||||||
|
$form->bind(array('firstName' => 'Bernhard', 'lastName' => ''));
|
||||||
|
|
||||||
|
$author = new Author();
|
||||||
|
$author->firstName = 'Bernhard';
|
||||||
|
$author->setLastName('');
|
||||||
|
|
||||||
|
$this->assertEquals($author, $form->getData());
|
||||||
|
}
|
||||||
|
|
||||||
|
public function testBindWithEmptyDataDoesNotCreateObjectIfDataClassIsNull()
|
||||||
|
{
|
||||||
|
$form = $this->factory->create('form', null, array(
|
||||||
|
'data' => new Author(),
|
||||||
|
'data_class' => null,
|
||||||
|
'required' => false,
|
||||||
|
));
|
||||||
|
$form->add($this->factory->createNamed('field', 'firstName'));
|
||||||
|
|
||||||
|
$form->setData(null);
|
||||||
|
$form->bind(array('firstName' => 'Bernhard'));
|
||||||
|
|
||||||
|
$this->assertSame(array('firstName' => 'Bernhard'), $form->getData());
|
||||||
|
}
|
||||||
|
|
||||||
public function testBindEmptyWithEmptyDataCreatesNoObjectIfNotRequired()
|
public function testBindEmptyWithEmptyDataCreatesNoObjectIfNotRequired()
|
||||||
{
|
{
|
||||||
$form = $this->factory->create('form', null, array(
|
$form = $this->factory->create('form', null, array(
|
||||||
|
@ -401,4 +401,12 @@ class TimeTypeTest extends LocalizedTestCase
|
|||||||
|
|
||||||
$this->assertTrue($form->isPartiallyFilled());
|
$this->assertTrue($form->isPartiallyFilled());
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Bug fix
|
||||||
|
public function testInitializeWithDateTime()
|
||||||
|
{
|
||||||
|
// Throws an exception if "data_class" option is not explicitely set
|
||||||
|
// to null in the type
|
||||||
|
$this->factory->create('time', new \DateTime());
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
Reference in New Issue
Block a user