BUG Fixed handling of numbers in certain locales.

Fixes #2161

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;
 	}
 	

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(
+		$validator->validationError(
-			trim($this->value), 
+			$this->name,
-			array('locale' => i18n::get_locale())
+			_t(
+				'NumericField.VALIDATION', "'{value}' is not a number, only numbers can be accepted for this field",
+				array('value' => $this->value)
+			),
+			"validation"
 		);
-
+		return false;
-		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;
 	}
-	
+
+	/**
+	 * 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 {

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');
+		foreach($tests as $input => $output) {
-		$this->assertFalse($field->validate($validator));
+			// 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');
+	public function testReadonly() {
-		$field->setValue('12,00');
+		i18n::set_locale('en_US');
-		$validator = new RequiredFields();
+		$field = new NumericField('Number');
-		$this->assertTrue($field->validate($validator));
-
-		$field->setValue('12.00');
-		$this->assertFalse($field->validate($validator));
-		
-		$field->setValue(0);
 		$this->assertRegExp("#<span[^>]+>\s*0\s*<\/span>#", "".$field->performReadonlyTransformation()->Field()."");
 	}
 
 }
+
+class NumericFieldTest_Object extends DataObject implements TestOnly {
+
+	private static $db = array(
+		'Number' => 'Float'
+	);
+}