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 handling of empty_data's \Closure value in Date/Time form types #34123

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
Feb 3, 2020
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Fix handling of empty_data's \Closure value in Date/Time form types
  • Loading branch information
@yceruto
yceruto committed Nov 11, 2019
commit 4939f0e32304329207e4248de35406bfc0c23c61
16 changes: 14 additions & 2 deletions 16 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 @@ -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 30 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 @@ -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 20 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 @@ -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 10 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 @@ -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 10 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 @@ -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 10 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 @@ -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],
];
}
}
Morty Proxy This is a proxified and sanitized view of the page, visit original site.