8000 merged branch bschussek/issue3288 (PR #3290) · symfony/symfony@be2e67c · GitHub
[go: up one dir, main page]
Skip to content

Commit be2e67c

Browse files
fabpotfabpot
committed
merged branch bschussek/issue3288 (PR #3290)
Commits ------- 22c8f80 [Form] Fixed issues mentioned in the PR comments 3b1b570 [Form] Fixed: The "date", "time" and "datetime" types can be initialized with \DateTime objects 88ef52d [Form] Improved FormType::getDefaultOptions() to see default options defined in parent types b9facfc [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
2 parents 78e6a2f + 22c8f80 commit be2e67c

File tree

14 files changed

+138
-20
lines changed

14 files changed

+138
-20
lines changed

‎CHANGELOG-2.1.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -208,6 +208,8 @@ To get the diff between two versions, go to https://github.com/symfony/symfony/c
208208
* forms now don't create an empty object anymore if they are completely
209209
empty and not required. The empty value for such forms is null.
210210
* added constant Guess::VERY_HIGH_CONFIDENCE
211+
* FormType::getDefaultOptions() now sees default options defined by parent types
212+
* [BC BREAK] FormType::getParent() does not see default options anymore
211213

212214
### HttpFoundation
213215

‎UPGRADE-2.1.md

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -229,3 +229,22 @@ UPGRADE FROM 2.0 to 2.1
229229
return false;
230230
}
231231
}
232+
233+
* The options passed to `getParent` of the form types don't contain default
234+
options anymore
235+
236+
You should check with `isset` if options exist before checking their value.
237+
238+
Before:
239+
240+
public function getParent(array $options)
241+
{
242+
return 'single_text' === $options['widget'] ? 'text' : 'choice';
243+
}
244+
245+
After:
246+
247+
public function getParent(array $options)
248+
{
249+
return isset($options['widget']) && 'single_text' === $options['widget'] ? 'text' : 'choice';
250+
}

‎src/Symfony/Bridge/Doctrine/Form/Type/DoctrineType.php

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,6 @@ public function getDefaultOptions(array $options)
5050
'property' => null,
5151
'query_builder' => null,
5252
'loader' => null,
53-
'choices' => null,
5453
'group_by' => null,
5554
);
5655

‎src/Symfony/Component/Form/Extension/Core/DataTransformer/ChoicesToBooleanArrayTransformer.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -107,7 +107,7 @@ public function reverseTransform($values)
107107
}
108108

109109
if (count($unknown) > 0) {
110-
throw new TransformationFailedException('The choices "' . implode('", "', $unknown, $code, $previous) . '" where not found');
110+
throw new TransformationFailedException('The choices "' . implode('", "', $unknown) . '" were not found');
111111
}
112112

113113
return $result;

‎src/Symfony/Component/Form/Extension/Core/Type/ChoiceType.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -153,7 +153,7 @@ public function getDefaultOptions(array $options)
153153
'multiple' => false,
154154
'expanded' => false,
155155
'choice_list' => null,
156-
'choices' => array(),
156+
'choices' => null,
157157
'preferred_choices' => array(),
158158
'value_strategy' => ChoiceList::GENERATE,
159159
'index_strategy' => ChoiceList::GENERATE,
@@ -170,7 +170,7 @@ public function getDefaultOptions(array $options)
170170
*/
171171
public function getParent(array $options)
172172
{
173-
return $options['expanded'] ? 'form' : 'field';
173+
return isset($options['expanded']) && $options['expanded'] ? 'form' : 'field';
174174
}
175175

176176
/**

‎src/Symfony/Component/Form/Extension/Core/Type/DateTimeType.php

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -158,6 +158,11 @@ public function getDefaultOptions(array $options)
158158
'widget' => null,
159159
// This will overwrite "empty_value" child options
160160
'empty_value' => null,
161+
// If initialized with a \DateTime object, FieldType initializes
162+
// this option to "\DateTime". Since the internal, normalized
163+
// representation is not \DateTime, but an array, we need to unset
164+
// this option.
165+
'data_class' => null,
161166
);
162167
}
163168

@@ -200,7 +205,7 @@ public function getAllowedOptionValues(array $options)
200205
*/
201206
public function getParent(array $options)
202207
{
203-
return 'single_text' === $options['widget'] ? 'field' : 'form';
208+
return isset($options['widget']) && 'single_text' === $options['widget'] ? 'field' : 'form';
204209
}
205210

206211
/**

‎src/Symfony/Component/Form/Extension/Core/Type/DateType.php

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -183,6 +183,11 @@ public function getDefaultOptions(array $options)
183183
// them like immutable value objects
184184
'by_reference' => false,
185185
'error_bubbling' => false,
186+
// If initialized with a \DateTime object, FieldType initializes
187+
// this option to "\DateTime". Since the internal, normalized
188+
// representation is not \DateTime, but an array, we need to unset
189+
// this option.
190+
'data_class' => null,
186191
);
187192
}
188193

@@ -211,7 +216,7 @@ public function getAllowedOptionValues(array $options)
211216
*/
212217
public function getParent(array $options)
213218
{
214-
return 'single_text' === $options['widget'] ? 'field' : 'form';
219+
return isset($options['widget']) && 'single_text' === $options['widget'] ? 'field' : 'form';
215220
}
216221

217222
/**

‎src/Symfony/Component/Form/Extension/Core/Type/TimeType.php

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -156,6 +156,11 @@ public function getDefaultOptions(array $options)
156156
// them like immutable value objects
157157
'by_reference' => false,
158158
'error_bubbling' => false,
159+
// If initialized with a \DateTime object, FieldType initializes
160+
// this option to "\DateTime". Since the internal, normalized
161+
// representation is not \DateTime, but an array, we need to unset
162+
// this option.
163+
'data_class' => null,
159164
);
160165
}
161166

@@ -184,7 +189,7 @@ public function getAllowedOptionValues(array $options)
184189
*/
185190
public function getParent(array $options)
186191
{
187-
return 'single_text' === $options['widget'] ? 'field' : 'form';
192+
return isset($options['widget']) && 'single_text' === $options['widget'] ? 'field' : 'form';
188193
}
189194

190195
/**

‎src/Symfony/Component/Form/Extension/Core/Type/TimezoneType.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ public function getDefaultOptions(array $options)
3131
'value 7802 _strategy' => ChoiceList::COPY_CHOICE,
3232
);
3333

34-
if (!isset($options['choice_list']) && !isset($options['choices'])) {
34+
if (empty($options['choice_list']) && empty($options['choices'])) {
3535
$defaultOptions['choices'] = self::getTimezones();
3636
}
3737

‎src/Symfony/Component/Form/FormFactory.php

Lines changed: 35 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -213,16 +213,20 @@ public function createBuilder($type, $data = null, array $options = array(), For
213213
*/
214214
public function createNamedBuilder($type, $name, $data = null, array $options = array(), FormBuilder $parent = null)
215215
{
216-
$builder = null;
217-
$types = array();
218-
$knownOptions = array();
219-
$passedOptions = array_keys($options);
220-
$optionValues = array();
221-
222216
if (!array_key_exists('data', $options)) {
223217
$options['data'] = $data;
224218
}
225219

220+
$builder = null;
221+
$types = array();
222+
$defaultOptions = array();
223+
$optionValues = array();
224+
$passedOptions = $options;
225+
226+
// Bottom-up determination of the type hierarchy
227+
// Start with the actual type and look for the parent type
228+
// The complete hierarchy is saved in $types, the first entry being
229+
// the root and the last entry being the leaf (the concrete type)
226230
while (null !== $type) {
227231
if ($type instanceof FormTypeInterface) {
228232
if ($type->getName() == $type->getParent($options)) {
@@ -236,28 +240,47 @@ public function createNamedBuilder($type, $name, $data = null, array $options =
236240
throw new UnexpectedTypeException($type, 'string or Symfony\Component\Form\FormTypeInterface');
237241
}
238242

239-
$defaultOptions = $type->getDefaultOptions($options);
243+
array_unshift($types, $type);
244+
245+
// getParent() cannot see default options set by this type nor
246+
// default options set by parent types
247+
// As a result, the options always have to be checked for
248+
// existence with isset() before using them in this method.
249+
$type = $type->getParent($options);
250+
}
251+
252+
// Top-down determination of the options and default options
253+
foreach ($types as $type) {
254+
// Merge the default options of all types to an array of default
255+
// options. Default options of children override default options
256+
// of parents.
257+
// Default options of ancestors are already visible in the $options
258+
// array passed to the following methods.
259+
$defaultOptions = array_replace($defaultOptions, $type->getDefaultOptions($options));
240260
$optionValues = array_merge_recursive($optionValues, $type->getAllowedOptionValues($options));
241261

242262
foreach ($type->getExtensions() as $typeExtension) {
243263
$defaultOptions = array_replace($defaultOptions, $typeExtension->getDefaultOptions($options));
244264
$optionValues = array_merge_recursive($optionValues, $typeExtension->getAllowedOptionValues($options));
245265
}
246266

247-
$options = array_replace($defaultOptions, $options);
248-
$knownOptions = array_merge($knownOptions, array_keys($defaultOptions));
249-
array_unshift($types, $type);
250-
$type = $type->getParent($options);
267+
// In each turn, the options are replaced by the combination of
268+
// the currently known default options and the passed options.
269+
// It is important to merge with $passedOptions and not with
270+
// $options, otherwise default options of parents would override
271+
// default options of child types.
272+
$options = array_replace($defaultOptions, $passedOptions);
251273
}
252274

253275
$type = end($types);
276+
$knownOptions = array_keys($defaultOptions);
254277
$diff = array_diff(self::$requiredOptions, $knownOptions);
255278

256279
if (count($diff) > 0) {
257280
throw new TypeDefinitionException(sprintf('Type "%s" should support the option(s) "%s"', $type->getName(), implode('", "', $diff)));
258281
}
259282

260-
$diff = array_diff($passedOptions, $knownOptions);
283+
$diff = array_diff(array_keys($passedOptions), $knownOptions);
261284

262285
if (count($diff) > 1) {
263286
throw new CreationException(sprintf('The options "%s" do not exist. Known options are: "%s"', implode('", "', $diff), implode('", "', $knownOptions)));

‎tests/Symfony/Tests/Component/Form/Extension/Core/Type/DateTimeTypeTest.php

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -258,4 +258,12 @@ public function testSubmit_invalidDateTime()
258258
$this->assertEquals(array(new FormError('Customized invalid message', array())), $form['date']->getErrors());
259259
$this->assertEquals(array(new FormError('Customized invalid message', array())), $form['time']->getErrors());
260260
}
261+
262+
// Bug fix
263+
public function testInitializeWithDateTime()
264+
{
265+
// Throws an exception if "data_class" option is not explicitely set
266+
// to null in the type
267+
$this->factory->create('datetime', new \DateTime());
268+
}
261269
}

‎tests/Symfony/Tests/Component/Form/Extension/Core/Type/DateTypeTest.php

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -530,4 +530,12 @@ public function testPassWidgetToView()
530530

531531
$this->assertSame('single_text', $view->get('widget'));
532532
}
533+
534+
// Bug fix
535+
public function testInitializeWithDateTime()
536+
{
537+
// Throws an exception if "data_class" option is not explicitely set
538+
// to null in the type
539+
$this->factory->create('date', new \DateTime());
540+
}
533541
}

‎tests/Symfony/Tests/Component/Form/Extension/Core/Type/FieldTypeTest.php

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -221,6 +221,42 @@ public function testBindWithEmptyDataCreatesObjectIfClassAvailable()
221221
$this->assertEquals($author, $form->getData());
222222
}
223223

224+
public function testBindWithEmptyDataCreatesObjectIfInitiallyBoundWithObject()
225+
{
226+
$form = $this->factory->create('form', null, array(
227+
// data class is inferred from the passed object
228+
'data' => new Author(),
229+
'required' => false,
230+
));
231+
$form->add($this->factory->createNamed('field', 'firstName'));
232+
$form->add($this->factory->createNamed('field', 'lastName'));
233+
234+
$form->setData(null);
235+
// partially empty, still an object is created
236+
$form->bind(array('firstName' => 'Bernhard', 'lastName' => ''));
237+
238+
$author = new Author();
239+
$author->firstName = 'Bernhard';
240+
$author->setLastName('');
241+
242+
$this->assertEquals($author, $form->getData());
243+
}
244+
245+
public function testBindWithEmptyDataDoesNotCreateObjectIfDataClassIsNull()
246+
{
247+
$form = $this->factory->create('form', null, array(
248+
'data' => new Author(),
249+
'data_class' => null,
250+
'required' => false,
251+
));
252+
$form->add($this->factory->createNamed('field', 'firstName'));
253+
254+
$form->setData(null);
255+
$form->bind(array('firstName' => 'Bernhard'));
256+
257+
$this->assertSame(array('firstName' => 'Bernhard'), $form->getData());
258+
}
259+
224260
public function testBindEmptyWithEmptyDataCreatesNoObjectIfNotRequired()
225261
{
226262
$form = $this->factory->create('form', null, array(

‎tests/Symfony/Tests/Component/Form/Extension/Core/Type/TimeTypeTest.php

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -401,4 +401,12 @@ public function testIsPartiallyFilled_returnsTrueIfChoiceAndSecondsEmpty()
401401

402402
$this->assertTrue($form->isPartiallyFilled());
403403
}
404+
405+
// Bug fix
406+
public function testInitializeWithDateTime()
407+
{
408+
// Throws an exception if "data_class" option is not explicitely set
409+
// to null in the type
410+
$this->factory->create('time', new \DateTime());
411+
}
404412
}

0 commit comments

Comments
 (0)
0