Skip to content
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

Jump to bottom

[Form] Fix handling of empty_data's \Closure value in Date/Time form types #34123

Merged
merged 1 commit into from Feb 3, 2020
+83 −13
Merged

[Form] Fix handling of empty_data's \Closure value in Date/Time form types #34123

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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
16 changes: 14 additions & 2 deletions src/Symfony/Component/Form/Extension/Core/Type/DateTimeType.php
View file
Open in desktop
Expand Up @@ -107,7 +107,17 @@ public function buildForm(FormBuilderInterface $builder, array $options)
'invalid_message_parameters',
]));

if (isset($emptyData['date'])) {
if ($emptyData instanceof \Closure) {
$lazyEmptyData = static function ($option) use ($emptyData) {
yceruto marked this conversation as resolved.
Show resolved Hide resolved
return static function (FormInterface $form) use ($emptyData, $option) {
$emptyData = $emptyData($form->getParent());

return isset($emptyData[$option]) ? $emptyData[$option] : '';
};
};

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

Expand All @@ -126,7 +136,9 @@ public function buildForm(FormBuilderInterface $builder, array $options)
'invalid_message_parameters',
]));

if (isset($emptyData['time'])) {
if ($emptyData instanceof \Closure) {
Copy link
Member

Choose a reason for hiding this comment

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

What about doing the same as what you did in the DateType then?

if ($emptyData instanceof \Closure) {
    $lazyEmptyData = static function ($option) use ($emptyData) {
        return static function (FormInterface $form) use ($emptyData, $option) {
            $emptyData = $emptyData($form->getParent());
            return isset($emptyData[$option]) ? $emptyData[$option] : '';
        };
    };
    $dateOptions['empty_data'] = $lazyEmptyData('date');
} else {
    if (isset($emptyData['date'])) {
        $dateOptions['empty_data'] = $emptyData['date'];
    }

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

Copy link
Contributor

Choose a reason for hiding this comment

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

To do that the $timeOptions initialization should be moved before the first empty data definition.

Copy link
Contributor

Choose a reason for hiding this comment

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

I can see there are conflicting comments from Nicolas and Christian, so maybe we should always set $lazyEmptyData base on the instance of once, then test if $lazyEmptyData is null to define the options.

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

Expand Down
30 changes: 22 additions & 8 deletions src/Symfony/Component/Form/Extension/Core/Type/DateType.php
View file
Open in desktop
Expand Up @@ -81,14 +81,28 @@ public function buildForm(FormBuilderInterface $builder, array $options)
// so we need to handle the cascade setting here
$emptyData = $builder->getEmptyData() ?: [];

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 ($emptyData instanceof \Closure) {
$lazyEmptyData = static function ($option) use ($emptyData) {
yceruto marked this conversation as resolved.
Show resolved Hide resolved
return static function (FormInterface $form) use ($emptyData, $option) {
$emptyData = $emptyData($form->getParent());

return isset($emptyData[$option]) ? $emptyData[$option] : '';
};
};

$yearOptions['empty_data'] = $lazyEmptyData('year');
$monthOptions['empty_data'] = $lazyEmptyData('month');
$dayOptions['empty_data'] = $lazyEmptyData('day');
} else {
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'])) {
Expand Down
20 changes: 17 additions & 3 deletions src/Symfony/Component/Form/Extension/Core/Type/TimeType.php
View file
Open in desktop
Expand Up @@ -76,7 +76,17 @@ public function buildForm(FormBuilderInterface $builder, array $options)
// so we need to handle the cascade setting here
$emptyData = $builder->getEmptyData() ?: [];

if (isset($emptyData['hour'])) {
if ($emptyData instanceof \Closure) {
$lazyEmptyData = static function ($option) use ($emptyData) {
yceruto marked this conversation as resolved.
Show resolved Hide resolved
return static function (FormInterface $form) use ($emptyData, $option) {
$emptyData = $emptyData($form->getParent());

return isset($emptyData[$option]) ? $emptyData[$option] : '';
};
};

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

Expand Down Expand Up @@ -143,14 +153,18 @@ public function buildForm(FormBuilderInterface $builder, array $options)
$builder->add('hour', self::$widgets[$options['widget']], $hourOptions);

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

if ($options['with_seconds']) {
if (isset($emptyData['second'])) {
if ($emptyData instanceof \Closure) {
$secondOptions['empty_data'] = $lazyEmptyData('second');
} elseif (isset($emptyData['second'])) {
$secondOptions['empty_data'] = $emptyData['second'];
}
$builder->add('second', self::$widgets[$options['widget']], $secondOptions);
Expand Down
10 changes: 10 additions & 0 deletions src/Symfony/Component/Form/Tests/Extension/Core/Type/DateTimeTypeTest.php
View file
Open in desktop
Expand Up @@ -12,6 +12,7 @@
namespace Symfony\Component\Form\Tests\Extension\Core\Type;

use Symfony\Component\Form\FormError;
use Symfony\Component\Form\FormInterface;

class DateTimeTypeTest extends BaseTypeTest
{
Expand Down Expand Up @@ -608,6 +609,9 @@ public function testSubmitNullUsesDateEmptyData($widget, $emptyData, $expectedDa
]);
$form->submit(null);

if ($emptyData instanceof \Closure) {
$emptyData = $emptyData($form);
}
$this->assertSame($emptyData, $form->getViewData());
$this->assertEquals($expectedData, $form->getNormData());
$this->assertEquals($expectedData, $form->getData());
Expand All @@ -616,11 +620,17 @@ public function testSubmitNullUsesDateEmptyData($widget, $emptyData, $expectedDa
public function provideEmptyData()
{
$expectedData = \DateTime::createFromFormat('Y-m-d H:i', '2018-11-11 21:23');
$lazyEmptyData = static function (FormInterface $form) {
return $form->getConfig()->getCompound() ? ['date' => ['year' => '2018', 'month' => '11', 'day' => '11'], 'time' => ['hour' => '21', 'minute' => '23']] : '2018-11-11T21:23:00';
};

return [
'Simple field' => ['single_text', '2018-11-11T21:23:00', $expectedData],
'Compound text field' => ['text', ['date' => ['year' => '2018', 'month' => '11', 'day' => '11'], 'time' => ['hour' => '21', 'minute' => '23']], $expectedData],
'Compound choice field' => ['choice', ['date' => ['year' => '2018', 'month' => '11', 'day' => '11'], 'time' => ['hour' => '21', 'minute' => '23']], $expectedData],
'Simple field lazy' => ['single_text', $lazyEmptyData, $expectedData],
'Compound text field lazy' => ['text', $lazyEmptyData, $expectedData],
'Compound choice field lazy' => ['choice', $lazyEmptyData, $expectedData],
];
}
}
10 changes: 10 additions & 0 deletions src/Symfony/Component/Form/Tests/Extension/Core/Type/DateTypeTest.php
View file
Open in desktop
Expand Up @@ -13,6 +13,7 @@

use Symfony\Component\Form\ChoiceList\View\ChoiceView;
use Symfony\Component\Form\FormError;
use Symfony\Component\Form\FormInterface;
use Symfony\Component\Intl\Util\IntlTestHelper;

class DateTypeTest extends BaseTypeTest
Expand Down Expand Up @@ -985,6 +986,9 @@ public function testSubmitNullUsesDateEmptyData($widget, $emptyData, $expectedDa
]);
$form->submit(null);

if ($emptyData instanceof \Closure) {
$emptyData = $emptyData($form);
}
$this->assertSame($emptyData, $form->getViewData());
$this->assertEquals($expectedData, $form->getNormData());
$this->assertEquals($expectedData, $form->getData());
Expand All @@ -993,11 +997,17 @@ public function testSubmitNullUsesDateEmptyData($widget, $emptyData, $expectedDa
public function provideEmptyData()
{
$expectedData = \DateTime::createFromFormat('Y-m-d H:i:s', '2018-11-11 00:00:00');
$lazyEmptyData = static function (FormInterface $form) {
return $form->getConfig()->getCompound() ? ['year' => '2018', 'month' => '11', 'day' => '11'] : '2018-11-11';
};

return [
'Simple field' => ['single_text', '2018-11-11', $expectedData],
'Compound text fields' => ['text', ['year' => '2018', 'month' => '11', 'day' => '11'], $expectedData],
'Compound choice fields' => ['choice', ['year' => '2018', 'month' => '11', 'day' => '11'], $expectedData],
'Simple field lazy' => ['single_text', $lazyEmptyData, $expectedData],
'Compound text fields lazy' => ['text', $lazyEmptyData, $expectedData],
'Compound choice fields lazy' => ['choice', $lazyEmptyData, $expectedData],
];
}
}
10 changes: 10 additions & 0 deletions src/Symfony/Component/Form/Tests/Extension/Core/Type/TimeTypeTest.php
View file
Open in desktop
Expand Up @@ -13,6 +13,7 @@

use Symfony\Component\Form\ChoiceList\View\ChoiceView;
use Symfony\Component\Form\FormError;
use Symfony\Component\Form\FormInterface;

class TimeTypeTest extends BaseTypeTest
{
Expand Down Expand Up @@ -785,6 +786,9 @@ public function testSubmitNullUsesDateEmptyData($widget, $emptyData, $expectedDa
]);
$form->submit(null);

if ($emptyData instanceof \Closure) {
$emptyData = $emptyData($form);
}
$this->assertSame($emptyData, $form->getViewData());
$this->assertEquals($expectedData, $form->getNormData());
$this->assertEquals($expectedData, $form->getData());
Expand All @@ -793,11 +797,17 @@ public function testSubmitNullUsesDateEmptyData($widget, $emptyData, $expectedDa
public function provideEmptyData()
{
$expectedData = \DateTime::createFromFormat('Y-m-d H:i', '1970-01-01 21:23');
$lazyEmptyData = static function (FormInterface $form) {
return $form->getConfig()->getCompound() ? ['hour' => '21', 'minute' => '23'] : '21:23';
};

return [
'Simple field' => ['single_text', '21:23', $expectedData],
'Compound text field' => ['text', ['hour' => '21', 'minute' => '23'], $expectedData],
'Compound choice field' => ['choice', ['hour' => '21', 'minute' => '23'], $expectedData],
'Simple field lazy' => ['single_text', $lazyEmptyData, $expectedData],
'Compound text field lazy' => ['text', $lazyEmptyData, $expectedData],
'Compound choice field lazy' => ['choice', $lazyEmptyData, $expectedData],
];
}
}