merged branch egeloen/date-time-type (PR #3846)

This PR was merged into the master branch.

Commits
-------

bf9e238 [Form] Add options with_minutes to DateTimeType & TimeType

Discussion
----------

[Form] Add option with_minutes to the DateTimeType & TimeType

Bug fix: no
Feature addition: yes
Backwards compatibility break: no
Fixes the following tickets: -
Todo: -

Hey,

One of my project requires the datetime usage only with hours. I have submit a patch allowing to disable minutes like seconds are disabled.

---------------------------------------------------------------------------

by stloyd at 2012-04-09T16:26:11Z

You should also extend tests for those `Types`

---------------------------------------------------------------------------

by egeloen at 2012-04-09T16:31:51Z

Oups, I have looked at tests but I didn't find it at my first reading. I will do it :)

---------------------------------------------------------------------------

by stloyd at 2012-04-09T16:34:42Z

@egeloen Here you can find tests for Form Types: https://github.com/symfony/symfony/tree/master/src/Symfony/Component/Form/Tests/Extension/Core/Type

---------------------------------------------------------------------------

by egeloen at 2012-04-09T16:42:42Z

@stloyd I have added tests. Can you give me some feedbacks ?

---------------------------------------------------------------------------

by stloyd at 2012-04-09T16:46:33Z

@egeloen I'm not sure if we should allow user to set `with_minutes=false` and `with_seconds=true`. But in overall seems quite ok.

---------------------------------------------------------------------------

by egeloen at 2012-04-09T16:51:37Z

Yes, you're right. But I'm unsure how can I do this following the good way.

---------------------------------------------------------------------------

by inanimatt at 2012-05-03T15:46:02Z

Just make it throw an InvalidConfigurationException.php exception, no? :)

---------------------------------------------------------------------------

by egeloen at 2012-06-09T18:27:41Z

I have updated the PR in order to throw an ``InvalidConfigurationException`` if we enable seconds & disable minutes.

---------------------------------------------------------------------------

by egeloen at 2012-07-09T19:08:11Z

@bschussek I have removed the useless code.

I think I have found an issue about my PR. I have added 3 tests in order to show it. It seems if we disable minutes, the text widget is broken.

---------------------------------------------------------------------------

by stof at 2012-10-13T16:00:43Z

@egeloen can you rebase your PR as it conflicts with master ?

---------------------------------------------------------------------------

by egeloen at 2012-10-13T17:15:22Z

@stof rebase

Like explain previously, my PR is still failling if we disable minutes & use the text widget.

---------------------------------------------------------------------------

by egeloen at 2012-10-13T18:09:03Z

I have fixed the last issue. IMO, the PR can now be merge.

---------------------------------------------------------------------------

by stof at 2012-10-13T18:20:00Z

@bschussek @fabpot ping

---------------------------------------------------------------------------

by egeloen at 2012-10-16T18:13:00Z

@bschussek Do yo think this PR can be merge?

---------------------------------------------------------------------------

by egeloen at 2012-10-30T19:14:00Z

@fabpot is there something missing before merging?

---------------------------------------------------------------------------

by fabpot at 2012-10-31T08:22:55Z

I'm waiting for @bschussek approval.

---------------------------------------------------------------------------

by geoffrey-brier at 2012-11-13T10:49:52Z

I really need the `with_minute => false` enhancement on a project as I don't want to write CSS/JS hacks, could @bschussek approve/disapprove it so that I can make a decision?

---------------------------------------------------------------------------

by henrikbjorn at 2012-11-13T10:52:12Z

@geoffrey-brier you could do you own FieldType that extends the current one and add the option your self.

---------------------------------------------------------------------------

by egeloen at 2012-11-13T13:20:44Z

@bschussek Yes... :) I have updated the PR according to your feedback.

I needed to update the `DateTimeToStringTransformer` because it tries to create a `DateTime` only from the value (with no format). In my case, the `'03'` value is not enougt to create it. So, if the date time creation fails, it then try to create the datetime from the format. I don't know if it is the best approach but it works well.

By the way, why does it first try to create a `DateTime` without format, **then only** try to use the format ?

---------------------------------------------------------------------------

by bschussek at 2012-11-13T14:20:13Z

@egeloen Good question, I think the transformer is a bit flawed there. I'm working on that. The rest of the PR looks good. Thank you!

---------------------------------------------------------------------------

by bschussek at 2012-12-13T18:14:58Z

I fixed the transformer in #6333. Once that is merged into 2.1, and once 2.1 is merged into master after that, you can rebase this PR on master. Then we can merge it.

---------------------------------------------------------------------------

by egeloen at 2012-12-22T14:54:38Z

I have rebased & squashed commits. The PR is ready to merge. ping @fabpot
This commit is contained in:
Fabien Potencier 2012-12-22 22:01:49 +01:00
commit e20b7d9d3c
6 changed files with 157 additions and 18 deletions

View File

@ -148,7 +148,7 @@
{{ block('form_widget_simple') }}
{% else %}
<div {{ block('widget_container_attributes') }}>
{{ form_widget(form.hour, { 'attr': { 'size': '1' } }) }}:{{ form_widget(form.minute, { 'attr': { 'size': '1' } }) }}{% if with_seconds %}:{{ form_widget(form.second, { 'attr': { 'size': '1' } }) }}{% endif %}
{{ form_widget(form.hour, { 'attr': { 'size': '1' } }) }}{% if with_minutes %}:{{ form_widget(form.minute, { 'attr': { 'size': '1' } }) }}{% endif %}{% if with_seconds %}:{{ form_widget(form.second, { 'attr': { 'size': '1' } }) }}{% endif %}
</div>
{% endif %}
{% endspaceless %}

View File

@ -6,8 +6,11 @@
// There should be no spaces between the colons and the widgets, that's why
// this block is written in a single PHP tag
echo $view['form']->widget($form['hour'], array('attr' => array('size' => 1)));
echo ':';
echo $view['form']->widget($form['minute'], array('attr' => array('size' => 1)));
if ($with_minutes) {
echo ':';
echo $view['form']->widget($form['minute'], array('attr' => array('size' => 1)));
}
if ($with_seconds) {
echo ':';

View File

@ -69,9 +69,14 @@ class DateTimeType extends AbstractType
*/
public function buildForm(FormBuilderInterface $builder, array $options)
{
$parts = array('year', 'month', 'day', 'hour', 'minute');
$parts = array('year', 'month', 'day', 'hour');
$dateParts = array('year', 'month', 'day');
$timeParts = array('hour', 'minute');
$timeParts = array('hour');
if ($options['with_minutes']) {
$parts[] = 'minute';
$timeParts[] = 'minute';
}
if ($options['with_seconds']) {
$parts[] = 'second';
@ -118,6 +123,7 @@ class DateTimeType extends AbstractType
'hours',
'minutes',
'seconds',
'with_minutes',
'with_seconds',
'empty_value',
'required',
@ -223,6 +229,7 @@ class DateTimeType extends AbstractType
'widget' => null,
'date_widget' => $dateWidget,
'time_widget' => $timeWidget,
'with_minutes' => true,
'with_seconds' => false,
// Don't modify \DateTime classes by reference, we treat
// them like immutable value objects

View File

@ -15,6 +15,7 @@ use Symfony\Component\Form\AbstractType;
use Symfony\Component\Form\FormInterface;
use Symfony\Component\Form\FormBuilderInterface;
use Symfony\Component\Form\ReversedTransformer;
use Symfony\Component\Form\Exception\InvalidConfigurationException;
use Symfony\Component\Form\Extension\Core\DataTransformer\DateTimeToStringTransformer;
use Symfony\Component\Form\Extension\Core\DataTransformer\DateTimeToTimestampTransformer;
use Symfony\Component\Form\Extension\Core\DataTransformer\DateTimeToArrayTransformer;
@ -29,10 +30,20 @@ class TimeType extends AbstractType
*/
public function buildForm(FormBuilderInterface $builder, array $options)
{
$parts = array('hour', 'minute');
$format = 'H:i';
$parts = array('hour');
$format = 'H';
if ($options['with_seconds'] && !$options['with_minutes']) {
throw new InvalidConfigurationException('You can not disable minutes if you have enabled seconds.');
}
if ($options['with_minutes']) {
$format .= ':i';
$parts[] = 'minute';
}
if ($options['with_seconds']) {
$format = 'H:i:s';
$format .= ':s';
$parts[] = 'second';
}
@ -49,15 +60,19 @@ class TimeType extends AbstractType
foreach ($options['hours'] as $hour) {
$hours[$hour] = str_pad($hour, 2, '0', STR_PAD_LEFT);
}
foreach ($options['minutes'] as $minute) {
$minutes[$minute] = str_pad($minute, 2, '0', STR_PAD_LEFT);
}
// Only pass a subset of the options to children
$hourOptions['choices'] = $hours;
$hourOptions['empty_value'] = $options['empty_value']['hour'];
$minuteOptions['choices'] = $minutes;
$minuteOptions['empty_value'] = $options['empty_value']['minute'];
if ($options['with_minutes']) {
foreach ($options['minutes'] as $minute) {
$minutes[$minute] = str_pad($minute, 2, '0', STR_PAD_LEFT);
}
$minuteOptions['choices'] = $minutes;
$minuteOptions['empty_value'] = $options['empty_value']['minute'];
}
if ($options['with_seconds']) {
$seconds = array();
@ -72,17 +87,23 @@ class TimeType extends AbstractType
// Append generic carry-along options
foreach (array('required', 'translation_domain') as $passOpt) {
$hourOptions[$passOpt] = $minuteOptions[$passOpt] = $options[$passOpt];
$hourOptions[$passOpt] = $options[$passOpt];
if ($options['with_minutes']) {
$minuteOptions[$passOpt] = $options[$passOpt];
}
if ($options['with_seconds']) {
$secondOptions[$passOpt] = $options[$passOpt];
}
}
}
$builder
->add('hour', $options['widget'], $hourOptions)
->add('minute', $options['widget'], $minuteOptions)
;
$builder->add('hour', $options['widget'], $hourOptions);
if ($options['with_minutes']) {
$builder->add('minute', $options['widget'], $minuteOptions);
}
if ($options['with_seconds']) {
$builder->add('second', $options['widget'], $secondOptions);
@ -113,6 +134,7 @@ class TimeType extends AbstractType
{
$view->vars = array_replace($view->vars, array(
'widget' => $options['widget'],
'with_minutes' => $options['with_minutes'],
'with_seconds' => $options['with_seconds'],
));
@ -167,6 +189,7 @@ class TimeType extends AbstractType
'seconds' => range(0, 59),
'widget' => 'choice',
'input' => 'datetime',
'with_minutes' => true,
'with_seconds' => false,
'model_timezone' => $modelTimezone,
'view_timezone' => $viewTimezone,

View File

@ -94,6 +94,35 @@ class DateTimeTypeTest extends LocalizedTestCase
$this->assertEquals($dateTime->format('U'), $form->getData());
}
public function testSubmit_withoutMinutes()
{
$form = $this->factory->create('datetime', null, array(
'data_timezone' => 'UTC',
'user_timezone' => 'UTC',
'date_widget' => 'choice',
'time_widget' => 'choice',
'input' => 'datetime',
'with_minutes' => false,
));
$form->setData(new \DateTime('2010-06-02 03:04:05 UTC'));
$input = array(
'date' => array(
'day' => '2',
'month' => '6',
'year' => '2010',
),
'time' => array(
'hour' => '3',
),
);
$form->bind($input);
$this->assertDateTimeEquals(new \DateTime('2010-06-02 03:00:00 UTC'), $form->getData());
}
public function testSubmit_withSeconds()
{
$form = $this->factory->create('datetime', null, array(

View File

@ -111,6 +111,22 @@ class TimeTypeTest extends LocalizedTestCase
$this->assertEquals('03:04', $form->getViewData());
}
public function testSubmit_datetimeSingleTextWithoutMinutes()
{
$form = $this->factory->create('time', null, array(
'data_timezone' => 'UTC',
'user_timezone' => 'UTC',
'input' => 'datetime',
'widget' => 'single_text',
'with_minutes' => false,
));
$form->bind('03');
$this->assertEquals(new \DateTime('1970-01-01 03:00:00 UTC'), $form->getData());
$this->assertEquals('03', $form->getViewData());
}
public function testSubmit_arraySingleText()
{
$form = $this->factory->create('time', null, array(
@ -131,6 +147,26 @@ class TimeTypeTest extends LocalizedTestCase
$this->assertEquals('03:04', $form->getViewData());
}
public function testSubmit_arraySingleTextWithoutMinutes()
{
$form = $this->factory->create('time', null, array(
'data_timezone' => 'UTC',
'user_timezone' => 'UTC',
'input' => 'array',
'widget' => 'single_text',
'with_minutes' => false,
));
$data = array(
'hour' => '3',
);
$form->bind('03');
$this->assertEquals($data, $form->getData());
$this->assertEquals('03', $form->getViewData());
}
public function testSubmit_arraySingleTextWithSeconds()
{
$form = $this->factory->create('time', null, array(
@ -168,6 +204,36 @@ class TimeTypeTest extends LocalizedTestCase
$this->assertEquals('03:04', $form->getViewData());
}
public function testSubmit_stringSingleTextWithoutMinutes()
{
$form = $this->factory->create('time', null, array(
'data_timezone' => 'UTC',
'user_timezone' => 'UTC',
'input' => 'string',
'widget' => 'single_text',
'with_minutes' => false,
));
$form->bind('03');
$this->assertEquals('03:00:00', $form->getData());
$this->assertEquals('03', $form->getViewData());
}
public function testSetData_withoutMinutes()
{
$form = $this->factory->create('time', null, array(
'data_timezone' => 'UTC',
'user_timezone' => 'UTC',
'input' => 'datetime',
'with_minutes' => false,
));
$form->setData(new \DateTime('03:04:05 UTC'));
$this->assertEquals(array('hour' => 3), $form->getClientData());
}
public function testSetData_withSeconds()
{
$form = $this->factory->create('time', null, array(
@ -561,4 +627,15 @@ class TimeTypeTest extends LocalizedTestCase
$this->assertSame(array(), $form['second']->getErrors());
$this->assertSame(array($error), $form->getErrors());
}
/**
* @expectedException Symfony\Component\Form\Exception\InvalidConfigurationException
*/
public function testInitializeWithSecondsAndWithoutMinutes()
{
$this->factory->create('time', null, array(
'with_minutes' => false,
'with_seconds' => true,
));
}
}