Skip to content

[Form] Fixed empty data for compound date types #29182

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
Nov 15, 2018
Merged

[Form] Fixed empty data for compound date types #29182

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
Diff view
Diff view
14 changes: 14 additions & 0 deletions src/Symfony/Component/Form/Extension/Core/Type/DateTimeType.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,9 @@ public function buildForm(FormBuilderInterface $builder, array $options)
));
}
} else {
// when the form is compound the entries of the array are ignored in favor of children data
// so we need to handle the cascade setting here
$emptyData = $builder->getEmptyData() ?: array();
// Only pass a subset of the options to children
$dateOptions = array_intersect_key($options, array_flip(array(
'years',
Expand All @@ -105,6 +108,10 @@ public function buildForm(FormBuilderInterface $builder, array $options)
'invalid_message_parameters',
)));

if (isset($emptyData['date'])) {
$dateOptions['empty_data'] = $emptyData['date'];
}

$timeOptions = array_intersect_key($options, array_flip(array(
'hours',
'minutes',
Expand All @@ -121,6 +128,10 @@ public function buildForm(FormBuilderInterface $builder, array $options)
'invalid_message_parameters',
)));

if (isset($emptyData['time'])) {
$timeOptions['empty_data'] = $emptyData['time'];
}

if (false === $options['label']) {
$dateOptions['label'] = false;
$timeOptions['label'] = false;
Expand Down Expand Up @@ -226,6 +237,9 @@ public function configureOptions(OptionsResolver $resolver)
// this option.
'data_class' => null,
'compound' => $compound,
'empty_data' => function (Options $options) {
return $options['compound'] ? array() : '';
Copy link

Choose a reason for hiding this comment

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

Hi @nicolas-grekas ,
Could you explain, why empty data is empty string instead of null as it was before?

Copy link

Choose a reason for hiding this comment

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

We are using DateTimeToRfc3339Transformer, and it's expecting null as an empty value, otherwise throws an exception, and probably the same issue will be with DateTimeToLocalizedStringTransformer

Copy link
Member

Choose a reason for hiding this comment

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

empty_data is in the view format. This value should not be passed to transform() but to reverseTransform(). Can you please open a new issue with fine steps that allow to reproduce if you think you found a bug?

Copy link

@anyt anyt Jan 4, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

Hm, based on examples from the documentation and according to the code I thought empty_data is a model format, not view, as it could be new $data_class().

Need to investigate then.
Thank you for the fast response!

},
));

// Don't add some defaults in order to preserve the defaults
Expand Down
17 changes: 17 additions & 0 deletions src/Symfony/Component/Form/Extension/Core/Type/DateType.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,21 @@ public function buildForm(FormBuilderInterface $builder, array $options)

$yearOptions = $monthOptions = $dayOptions = array(
'error_bubbling' => true,
'empty_data' => '',
);
// when the form is compound the entries of the array are ignored in favor of children data
// so we need to handle the cascade setting here
$emptyData = $builder->getEmptyData() ?: array();

if (isset($emptyData['year'])) {
$yearOptions['empty_data'] = $emptyData['year'];
}
if (isset($emptyData['month'])) {
$monthOptions['empty_data'] = $emptyData['month'];
}
if (isset($emptyData['day'])) {
$dayOptions['empty_data'] = $emptyData['day'];
}

if (isset($options['invalid_message'])) {
$dayOptions['invalid_message'] = $options['invalid_message'];
Expand Down Expand Up @@ -272,6 +286,9 @@ public function configureOptions(OptionsResolver $resolver)
// this option.
'data_class' => null,
'compound' => $compound,
'empty_data' => function (Options $options) {
return $options['compound'] ? array() : '';
},
'choice_translation_domain' => false,
));

Expand Down
17 changes: 17 additions & 0 deletions src/Symfony/Component/Form/Extension/Core/Type/TimeType.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,15 @@ public function buildForm(FormBuilderInterface $builder, array $options)
} else {
$hourOptions = $minuteOptions = $secondOptions = array(
'error_bubbling' => true,
'empty_data' => '',
);
// when the form is compound the entries of the array are ignored in favor of children data
// so we need to handle the cascade setting here
$emptyData = $builder->getEmptyData() ?: array();

if (isset($emptyData['hour'])) {
$hourOptions['empty_data'] = $emptyData['hour'];
}

if (isset($options['invalid_message'])) {
$hourOptions['invalid_message'] = $options['invalid_message'];
Expand Down Expand Up @@ -138,10 +146,16 @@ public function buildForm(FormBuilderInterface $builder, array $options)
$builder->add('hour', self::$widgets[$options['widget']], $hourOptions);

if ($options['with_minutes']) {
if (isset($emptyData['minute'])) {
$minuteOptions['empty_data'] = $emptyData['minute'];
}
$builder->add('minute', self::$widgets[$options['widget']], $minuteOptions);
}

if ($options['with_seconds']) {
if (isset($emptyData['second'])) {
$secondOptions['empty_data'] = $emptyData['second'];
}
$builder->add('second', self::$widgets[$options['widget']], $secondOptions);
}

Expand Down Expand Up @@ -265,6 +279,9 @@ public function configureOptions(OptionsResolver $resolver)
// representation is not \DateTime, but an array, we need to unset
// this option.
'data_class' => null,
'empty_data' => function (Options $options) {
return $options['compound'] ? array() : '';
},
'compound' => $compound,
'choice_translation_domain' => false,
));
Expand Down
27 changes: 27 additions & 0 deletions src/Symfony/Component/Form/Tests/Extension/Core/Type/DateTimeTypeTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -631,4 +631,31 @@ public function testSubmitNullUsesDefaultEmptyData($emptyData = array(), $expect
$this->assertSame($expectedData, $form->getNormData());
$this->assertSame($expectedData, $form->getData());
}

/**
* @dataProvider provideEmptyData
*/
public function testSubmitNullUsesDateEmptyData($widget, $emptyData, $expectedData)
{
$form = $this->factory->create(static::TESTED_TYPE, null, array(
'widget' => $widget,
'empty_data' => $emptyData,
));
$form->submit(null);

$this->assertSame($emptyData, $form->getViewData());
$this->assertEquals($expectedData, $form->getNormData());
$this->assertEquals($expectedData, $form->getData());
}

public function provideEmptyData()
{
$expectedData = \DateTime::createFromFormat('Y-m-d H:i', '2018-11-11 21:23');

return array(
'Simple field' => array('single_text', '2018-11-11T21:23:00', $expectedData),
'Compound text field' => array('text', array('date' => array('year' => '2018', 'month' => '11', 'day' => '11'), 'time' => array('hour' => '21', 'minute' => '23')), $expectedData),
'Compound choice field' => array('choice', array('date' => array('year' => '2018', 'month' => '11', 'day' => '11'), 'time' => array('hour' => '21', 'minute' => '23')), $expectedData),
);
}
}
27 changes: 19 additions & 8 deletions src/Symfony/Component/Form/Tests/Extension/Core/Type/DateTypeTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1011,25 +1011,36 @@ public function testSubmitNullUsesDefaultEmptyData($emptyData = array(), $expect
));
$form->submit(null);

// view transformer write back empty strings in the view data
// view transformer writes back empty strings in the view data
$this->assertSame(array('year' => '', 'month' => '', 'day' => ''), $form->getViewData());
$this->assertSame($expectedData, $form->getNormData());
$this->assertSame($expectedData, $form->getData());
}

public function testSingleTextSubmitNullUsesDefaultEmptyData()
/**
* @dataProvider provideEmptyData
*/
public function testSubmitNullUsesDateEmptyData($widget, $emptyData, $expectedData)
{
$emptyData = '2018-11-11';
$form = $this->factory->create(static::TESTED_TYPE, null, array(
'widget' => 'single_text',
'widget' => $widget,
'empty_data' => $emptyData,
));
$form->submit(null);

$date = new \DateTime($emptyData);

$this->assertSame($emptyData, $form->getViewData());
$this->assertEquals($date, $form->getNormData());
$this->assertEquals($date, $form->getData());
$this->assertEquals($expectedData, $form->getNormData());
$this->assertEquals($expectedData, $form->getData());
}

public function provideEmptyData()
{
$expectedData = \DateTime::createFromFormat('Y-m-d H:i:s', '2018-11-11 00:00:00');

return array(
'Simple field' => array('single_text', '2018-11-11', $expectedData),
'Compound text fields' => array('text', array('year' => '2018', 'month' => '11', 'day' => '11'), $expectedData),
'Compound choice fields' => array('choice', array('year' => '2018', 'month' => '11', 'day' => '11'), $expectedData),
);
}
}
29 changes: 28 additions & 1 deletion src/Symfony/Component/Form/Tests/Extension/Core/Type/TimeTypeTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -805,9 +805,36 @@ public function testSubmitNullUsesDefaultEmptyData($emptyData = array(), $expect
));
$form->submit(null);

// view transformer write back empty strings in the view data
// view transformer writes back empty strings in the view data
$this->assertSame(array('hour' => '', 'minute' => ''), $form->getViewData());
$this->assertSame($expectedData, $form->getNormData());
$this->assertSame($expectedData, $form->getData());
}

/**
* @dataProvider provideEmptyData
*/
public function testSubmitNullUsesDateEmptyData($widget, $emptyData, $expectedData)
{
$form = $this->factory->create(static::TESTED_TYPE, null, array(
'widget' => $widget,
'empty_data' => $emptyData,
));
$form->submit(null);

$this->assertSame($emptyData, $form->getViewData());
$this->assertEquals($expectedData, $form->getNormData());
$this->assertEquals($expectedData, $form->getData());
}

public function provideEmptyData()
{
$expectedData = \DateTime::createFromFormat('Y-m-d H:i', '1970-01-01 21:23');

return array(
'Simple field' => array('single_text', '21:23', $expectedData),
'Compound text field' => array('text', array('hour' => '21', 'minute' => '23'), $expectedData),
'Compound choice field' => array('choice', array('hour' => '21', 'minute' => '23'), $expectedData),
);
}
}