merged branch iteman/date-validation-with-singletext-form-fix (PR #4434)
Commits -------20b556d
[Form] fixed a bug that caused input date validation not to be strict when using the single_text widget with a datetime field7e3213c
[Form] fixed a bug that caused input date validation not to be strict when using the single_text widget with a date field Discussion ---------- [Form] fixed a bug that caused input date validation not to be strict when using the single_text widget with a date/datetime field Bug fix: yes Feature addition: no Backwards compatibility break: no Symfony2 tests pass: yes Fixes the following tickets: - Todo: - License of the code: MIT Documentation PR: - Currently, input date validation is not strict when using the single_text widget with a date or datetime field. This will cause unexpected transformation of input date value as follows: * 2012-04-31 => 2012-05-01 * 2012-04-31 03:04:00 => 2012-05-01 03:04:00 Additionally this is not same as other transformation logic for date/datetime fields because they use the checkdate() function to validate input date values. The checkdate() function validates a date strictly. ```php <?php var_dump(checkdate(4, 31, 2012)); // bool(false) ``` BTW, the documentation of [IntlDateFormatter::setLenient()](http://php.net/manual/en/intldateformatter.setlenient.php) and [IntlDateFormatter::isLenient()](http://php.net/manual/en/intldateformatter.islenient.php) have been fixed recently. Please see https://bugs.php.net/bug.php?id=61896 **The default parser is lenient, not strict**. --------------------------------------------------------------------------- by travisbot at 2012-05-28T07:35:11Z This pull request [fails](http://travis-ci.org/symfony/symfony/builds/1453681) (merged20b556da
into167779e5
). --------------------------------------------------------------------------- by bschussek at 2012-05-28T09:25:23Z Quote on strict parsing: > Extra space, unrecognized tokens, or invalid values ("February 30th") are not accepted. Disabling lenient parsing makes entering dates in text boxes even harder than it is right now. --------------------------------------------------------------------------- by iteman at 2012-05-29T09:31:49Z In my opinion, in most applications, in particular enterprise applications, date/time handling is very important. Unexpected transformation of date/time may sometimes lead unexpected outcomes. In such applications, accepting invalid values is not important than avoiding unexpected transformation. **As specifications, "strict" should be the default, or at least, the "strict" option should be provided if the default is "lenient".** Moving on to the current behavior. If "lenient" is intended behavior, is there any way to know it? At least, there is nothing to be found in the tests and documentation for the date/datetime field. http://php.net/manual/en/intldateformatter.setlenient.php: > Extra space, unrecognized tokens, or invalid values ("February 30th") are not accepted. This is only applicable to *DateTimeToLocalizedStringTransformer* (using *IntlDateFormatter*) that is used by *DateType*, not applicable to *DateTimeToStringTransformer* (using *DateTime*) that is used by *DateTimeType*. Why is defferent between these implementation in the first place? On the one hand, strict validation by *checkdate()* has been adopted in the widgets except the single_text widget. Does this conflict with the single_text widget? Isn't there any problem to be determined "lenient" or "strict" by the widget type mainly from the point of view of the separation of concerns? Additionally, is there any way to avoid unexpected transformation in the current behavior without changing the specification of a domain object, or writing a new transfer object, or not using the single_text widget? Finally, I think that the current behavior is bad, but if the current behavior is not a bug, I think that the "strict" option should be provided in *DateType* and *DateTimeType*. Thanks. --------------------------------------------------------------------------- by tanakahisateru at 2012-05-29T13:45:12Z Simply, potentially some users can't find represented their own posts if "4" changed to "5" silently. I think, for regular users to understand why converted so is harder than to understand why blocked his date text. --------------------------------------------------------------------------- by fabpot at 2012-05-30T05:19:44Z I'm +1 for this change, but only on master. @bschussek? --------------------------------------------------------------------------- by sstok at 2012-05-30T07:44:04Z Date parsing with 'only' the Locale component is hard, especially the extra space or using / when only - is excepted. And there also a locale that uses dd. mm. yyyy. (dot + space). So to overcome this I have created a little helper class https://github.com/rollerworks/Locale. It works as follow. 1. First it searches all the numeric-script characters, and converts those to ASCII decimals 2. Then matches the converted input against an pre-compiled regex that accepts extra spaces or using '/' instead of '-', and omitting leading zero's, etc... 3. Validate the matched parts using checkdate(). I'm not suggesting to only use my Component, but making an it an option would be something to consider, no? Like an option mode: lenient/strict/match (match is the Rollerworks Locale Component or a-like). The only thing currently missing is Timezone parsing support, but that is not to hard. I only have not implemented this because there was no direct need for it at the time. --------------------------------------------------------------------------- by bschussek at 2012-05-30T07:49:11Z @fabpot Ok. In the long run we need to find a better solution anyway.
This commit is contained in:
commit
f541844c78
@ -157,6 +157,8 @@ class DateTimeToLocalizedStringTransformer extends BaseDateTimeTransformer
|
||||
$calendar = $this->calendar;
|
||||
$pattern = $this->pattern;
|
||||
|
||||
return new \IntlDateFormatter(\Locale::getDefault(), $dateFormat, $timeFormat, $timezone, $calendar, $pattern);
|
||||
$intlDateFormatter = new \IntlDateFormatter(\Locale::getDefault(), $dateFormat, $timeFormat, $timezone, $calendar, $pattern);
|
||||
$intlDateFormatter->setLenient(false);
|
||||
return $intlDateFormatter;
|
||||
}
|
||||
}
|
||||
|
@ -96,6 +96,10 @@ class DateTimeToStringTransformer extends BaseDateTimeTransformer
|
||||
|
||||
try {
|
||||
$dateTime = new \DateTime($value, new \DateTimeZone($this->outputTimezone));
|
||||
$lastErrors = \DateTime::getLastErrors();
|
||||
if (0 < $lastErrors['warning_count'] || 0 < $lastErrors['error_count']) {
|
||||
throw new \UnexpectedValueException(implode(', ', array_merge(array_values($lastErrors['warnings']), array_values($lastErrors['errors']))));
|
||||
}
|
||||
|
||||
// Force value to be in same format as given to transform
|
||||
if ($value !== $dateTime->format($this->format)) {
|
||||
|
@ -61,6 +61,7 @@ class DateType extends AbstractType
|
||||
\IntlDateFormatter::GREGORIAN,
|
||||
$pattern
|
||||
);
|
||||
$formatter->setLenient(false);
|
||||
|
||||
if ($options['widget'] === 'single_text') {
|
||||
$builder->appendClientTransformer(new DateTimeToLocalizedStringTransformer($options['data_timezone'], $options['user_timezone'], $format, \IntlDateFormatter::NONE, \IntlDateFormatter::GREGORIAN, $pattern));
|
||||
|
@ -286,4 +286,14 @@ class DateTimeToLocalizedStringTransformerTest extends DateTimeTestCase
|
||||
{
|
||||
new DateTimeToLocalizedStringTransformer(null, null, null, 'foobar');
|
||||
}
|
||||
|
||||
/**
|
||||
* @expectedException Symfony\Component\Form\Exception\TransformationFailedException
|
||||
*/
|
||||
public function testReverseTransformWithNonExistingDate()
|
||||
{
|
||||
$transformer = new DateTimeToLocalizedStringTransformer('UTC', 'UTC', \IntlDateFormatter::SHORT);
|
||||
|
||||
$this->assertDateTimeEquals($this->dateTimeWithoutSeconds, $transformer->reverseTransform('31.04.10 04:05'));
|
||||
}
|
||||
}
|
||||
|
@ -140,4 +140,13 @@ class DateTimeToStringTransformerTest extends DateTimeTestCase
|
||||
|
||||
$reverseTransformer->reverseTransform('2010-2010-2010');
|
||||
}
|
||||
|
||||
public function testReverseTransformWithNonExistingDate()
|
||||
{
|
||||
$reverseTransformer = new DateTimeToStringTransformer();
|
||||
|
||||
$this->setExpectedException('Symfony\Component\Form\Exception\TransformationFailedException');
|
||||
|
||||
$reverseTransformer->reverseTransform('2010-04-31');
|
||||
}
|
||||
}
|
||||
|
@ -234,4 +234,22 @@ class DateTimeTypeTest extends LocalizedTestCase
|
||||
$this->assertEquals(array(new FormError('Customized invalid message', array())), $form['date']->getErrors());
|
||||
$this->assertEquals(array(new FormError('Customized invalid message', array())), $form['time']->getErrors());
|
||||
}
|
||||
|
||||
public function testSubmit_invalidDateTimeSingleText()
|
||||
{
|
||||
$form = $this->factory->create('datetime', null, array(
|
||||
'data_timezone' => 'UTC',
|
||||
'user_timezone' => 'UTC',
|
||||
'input' => 'datetime',
|
||||
'widget' => 'single_text',
|
||||
'invalid_message' => 'Customized invalid message',
|
||||
));
|
||||
|
||||
$form->bind('2012-04-31 03:04:05');
|
||||
|
||||
$this->assertFalse($form->isValid());
|
||||
$this->assertNull($form->getData());
|
||||
$this->assertEquals('2012-04-31 03:04:05', $form->getClientData());
|
||||
$this->assertEquals(array(new FormError('Customized invalid message', array())), $form->getErrors());
|
||||
}
|
||||
}
|
||||
|
@ -13,6 +13,7 @@ namespace Symfony\Tests\Component\Form\Extension\Core\Type;
|
||||
|
||||
require_once __DIR__ . '/LocalizedTestCase.php';
|
||||
|
||||
use Symfony\Component\Form\FormError;
|
||||
|
||||
class DateTypeTest extends LocalizedTestCase
|
||||
{
|
||||
@ -460,4 +461,22 @@ class DateTypeTest extends LocalizedTestCase
|
||||
|
||||
$this->assertSame('single_text', $view->get('widget'));
|
||||
}
|
||||
|
||||
public function testInvalidDateWithSingleTextDateTime()
|
||||
{
|
||||
$form = $this->factory->create('date', null, array(
|
||||
'data_timezone' => 'UTC',
|
||||
'user_timezone' => 'UTC',
|
||||
'widget' => 'single_text',
|
||||
'input' => 'datetime',
|
||||
'invalid_message' => 'Customized invalid message',
|
||||
));
|
||||
|
||||
$form->bind('31.4.2012');
|
||||
|
||||
$this->assertFalse($form->isValid());
|
||||
$this->assertNull($form->getData());
|
||||
$this->assertEquals('31.4.2012', $form->getClientData());
|
||||
$this->assertEquals(array(new FormError('Customized invalid message', array())), $form->getErrors());
|
||||
}
|
||||
}
|
||||
|
Reference in New Issue
Block a user