From 518045257e32be90781a853ab187428ff4c00948 Mon Sep 17 00:00:00 2001 From: Damian Mooyman Date: Thu, 3 Apr 2014 09:33:18 +1300 Subject: [PATCH] BUG Fixed handling of numbers in certain locales. Fixes #2161 --- forms/FormField.php | 3 +- forms/NumericField.php | 116 +++++++++++++---- tests/forms/NumericFieldTest.php | 210 ++++++++++++++++++++++++++++--- 3 files changed, 290 insertions(+), 39 deletions(-) diff --git a/forms/FormField.php b/forms/FormField.php index 40fdba1c2..693c50253 100644 --- a/forms/FormField.php +++ b/forms/FormField.php @@ -423,6 +423,7 @@ class FormField extends RequestHandler { * Set the field value. * * @param mixed $value + * @param mixed $data Optional data source passed in by {@see Form::loadDataFrom} * @return FormField Self reference */ public function setValue($value) { @@ -444,7 +445,7 @@ class FormField extends RequestHandler { * have to worry about linking the two. */ public function setForm($form) { - $this->form = $form; + $this->form = $form; return $this; } diff --git a/forms/NumericField.php b/forms/NumericField.php index d6219074b..4fd615b2a 100644 --- a/forms/NumericField.php +++ b/forms/NumericField.php @@ -2,13 +2,63 @@ /** * Text input field with validation for numeric values. Supports validating - * the numeric value as to the {@link i18n::get_locale()} value. + * the numeric value as to the {@link i18n::get_locale()} value, or an + * overridden locale specific to this field. * * @package forms * @subpackage fields-formattedinput */ class NumericField extends TextField { + /** + * Override locale for this field + * + * @var string + */ + protected $locale = null; + + public function setValue($value, $data = array()) { + require_once "Zend/Locale/Format.php"; + + // If passing in a non-string number, or a value + // directly from a dataobject then localise this number + if ((is_numeric($value) && !is_string($value)) || + ($value && $data instanceof DataObject) + ){ + $locale = new Zend_Locale($this->getLocale()); + $this->value = Zend_Locale_Format::toNumber($value, array('locale' => $locale)); + } else { + // If an invalid number, store it anyway, but validate() will fail + $this->value = $this->clean($value); + } + return $this; + } + + /** + * In some cases and locales, validation expects non-breaking spaces + * + * @param string $input + * @return string The input value, with all spaces replaced with non-breaking spaces + */ + protected function clean($input) { + $nbsp = html_entity_decode(' ', null, 'UTF-8'); + return str_replace(' ', $nbsp, trim($input)); + } + + /** + * Determine if the current value is a valid number in the current locale + * + * @return bool + */ + protected function isNumeric() { + require_once "Zend/Locale/Format.php"; + $locale = new Zend_Locale($this->getLocale()); + return Zend_Locale_Format::isNumber( + $this->clean($this->value), + array('locale' => $locale) + ); + } + public function Type() { return 'numeric text'; } @@ -18,31 +68,33 @@ class NumericField extends TextField { return true; } - require_once THIRDPARTY_PATH."/Zend/Locale/Format.php"; + if($this->isNumeric()) return true; - $valid = Zend_Locale_Format::isNumber( - trim($this->value), - array('locale' => i18n::get_locale()) + $validator->validationError( + $this->name, + _t( + 'NumericField.VALIDATION', "'{value}' is not a number, only numbers can be accepted for this field", + array('value' => $this->value) + ), + "validation" ); - - if(!$valid) { - $validator->validationError( - $this->name, - _t( - 'NumericField.VALIDATION', "'{value}' is not a number, only numbers can be accepted for this field", - array('value' => $this->value) - ), - "validation" - ); - - return false; - } - - return true; + return false; } - + + /** + * Extracts the number value from the localised string value + * + * @return string number value + */ public function dataValue() { - return (is_numeric($this->value)) ? $this->value : 0; + require_once "Zend/Locale/Format.php"; + if(!$this->isNumeric()) return 0; + $locale = new Zend_Locale($this->getLocale()); + $number = Zend_Locale_Format::getNumber( + $this->clean($this->value), + array('locale' => $locale) + ); + return $number; } /** @@ -54,6 +106,26 @@ class NumericField extends TextField { return $field; } + /** + * Gets the current locale this field is set to + * + * @return string + */ + public function getLocale() { + return $this->locale ?: i18n::get_locale(); + } + + /** + * Override the locale for this field + * + * @param string $locale + * @return $this + */ + public function setLocale($locale) { + $this->locale = $locale; + return $this; + } + } class NumericField_Readonly extends ReadonlyField { diff --git a/tests/forms/NumericFieldTest.php b/tests/forms/NumericFieldTest.php index a7fd5ad27..c7c549107 100644 --- a/tests/forms/NumericFieldTest.php +++ b/tests/forms/NumericFieldTest.php @@ -8,34 +8,212 @@ class NumericFieldTest extends SapphireTest { protected $usesDatabase = false; - public function testValidator() { - i18n::set_locale('en_US'); + /** + * In some cases and locales, validation expects non-breaking spaces. + * + * Duplicates non-public NumericField::clean method + * + * @param string $input + * @return string The input value, with all spaces replaced with non-breaking spaces + */ + protected function clean($input) { + $nbsp = html_entity_decode(' ', null, 'UTF-8'); + return str_replace(' ', $nbsp, trim($input)); + } + protected function checkInputValidation($locale, $tests) { + i18n::set_locale($locale); $field = new NumericField('Number'); - $field->setValue('12.00'); - $validator = new RequiredFields('Number'); - $this->assertTrue($field->validate($validator)); - $field->setValue('12,00'); - $this->assertFalse($field->validate($validator)); + foreach($tests as $input => $output) { + // Both decimal and thousands B + $field->setValue($input); + if($output === false) { + $this->assertFalse( + $field->validate($validator), + "Expect validation to fail for input $input in locale $locale" + ); + $this->assertEquals( + 0, + $field->dataValue(), + "Expect invalid value to be rewritten to 0 in locale $locale" + ); + + // Even invalid values shouldn't be rewritten + $this->assertEquals( + $this->clean($input), + $field->Value(), + "Expected input $input to be saved in the field in locale $locale" + ); + } else { + $this->assertTrue( + $field->validate($validator), + "Expect validation to succeed for $input in locale $locale" + ); + $this->assertEquals( + $output, + $field->dataValue(), + "Expect value $input to be mapped to $output in locale $locale" + ); + } + } + } + + /** + * Test that data loaded in via Form::loadDataFrom(DataObject) will populate the field correctly, + * and can format the database value appropriately for the frontend + * + * @param string $locale + * @param array $tests + */ + public function checkDataFormatting($locale, $tests) { + i18n::set_locale($locale); + $field = new NumericField('Number'); + $form = new Form(new Controller(), 'Form', new FieldList($field), new FieldList()); + $dataObject = new NumericFieldTest_Object(); + + foreach($tests as $input => $output) { + // Given a dataobject as a context, the field should assume the field value is not localised + $dataObject->Number = (string)$input; + $form->loadDataFrom($dataObject, Form::MERGE_CLEAR_MISSING); + + // Test value + $this->assertEquals( + $input, + $field->dataValue(), + "Expected $input loaded via dataobject to be left intact in locale $locale" + ); + + // Test expected formatted value (Substitute nbsp for spaces) + $this->assertEquals( + $this->clean($output), + $field->Value(), + "Expected $input to be formatted as $output in locale $locale" + ); + } + } + + /** + * German locale values (same as dutch) + */ + public function testGermanLocales() { + $this->checkDataFormatting('de_DE', $formatting = array( + '13000' => "13.000", + '15' => '15', + '12.0' => '12,0', + '12.1' => '12,1', + '14000.5' => "14.000,5", + )); + + $this->checkDataFormatting('nl_NL', $formatting); + + $this->checkInputValidation('de_DE', $validation = array( + '13000' => 13000, + '12,00' => 12.00, + '12.00' => false, + '11 000' => false, + '11.000' => 11000, + '11,000' => 11.0, + '15 000,5' => false, + '15 000.5' => false, + '15.000,5' => 15000.5, + '15,000.5' => false, + )); + + $this->checkInputValidation('nl_NL', $validation); + } + + /** + * French locale values + */ + public function testFrenchLocales() { + $this->checkDataFormatting('fr_FR', array( + '13000' => "13 000", + '15' => '15', + '12.0' => '12,0', + '12.1' => '12,1', + '14000.5' => "14 000,5", + )); + + $this->checkInputValidation('fr_FR', array( + '13000' => 13000, + '12,00' => 12.00, + '12.00' => false, + '11 000' => 11000, + '11.000' => false, + '11,000' => 11.000, + '15 000,5' => 15000.5, + '15 000.5' => false, + '15.000,5' => false, + '15,000.5' => false, + )); + } + + /** + * US locale values + */ + public function testUSLocales() { + $this->checkDataFormatting('en_US', array( + '13000' => "13,000", + '15' => '15', + '12.0' => '12.0', + '12.1' => '12.1', + '14000.5' => "14,000.5", + )); + + $this->checkInputValidation('en_US', array( + '13000' => 13000, + '12,00' => false, + '12.00' => 12.00, + '11 000' => false, + '11.000' => 11.0, + '11,000' => 11000, + '15 000,5' => false, + '15 000.5' => false, + '15.000,5' => false, + '15,000.5' => 15000.5, + )); + } + + /** + * Test empty values + */ + public function testEmptyValidator() { + i18n::set_locale('en_US'); + $field = new NumericField('Number'); + $validator = new RequiredFields('Number'); + + // Treats '0' as given for the sake of required fields $field->setValue('0'); $this->assertTrue($field->validate($validator)); + $this->assertEquals(0, $field->dataValue()); + + // Treat literal 0 + $field->setValue(0); + $this->assertTrue($field->validate($validator)); + $this->assertEquals(0, $field->dataValue()); + + // Should fail the 'required but not given' test + $field->setValue(''); + $this->assertFalse($field->validate($validator)); $field->setValue(false); $this->assertFalse($field->validate($validator)); + } - i18n::set_locale('de_DE'); - $field->setValue('12,00'); - $validator = new RequiredFields(); - $this->assertTrue($field->validate($validator)); - - $field->setValue('12.00'); - $this->assertFalse($field->validate($validator)); - - $field->setValue(0); + public function testReadonly() { + i18n::set_locale('en_US'); + $field = new NumericField('Number'); $this->assertRegExp("#]+>\s*0\s*<\/span>#", "".$field->performReadonlyTransformation()->Field().""); } } + +class NumericFieldTest_Object extends DataObject implements TestOnly { + + private static $db = array( + 'Number' => 'Float' + ); +} \ No newline at end of file