diff --git a/src/Forms/Form.php b/src/Forms/Form.php index 0619f78f5..3f5f95771 100644 --- a/src/Forms/Form.php +++ b/src/Forms/Form.php @@ -346,7 +346,7 @@ class Form extends ViewableData implements HasRequestHandler // load data in from previous submission upon error $data = $this->getSessionData(); if (isset($data)) { - $this->loadDataFrom($data); + $this->loadDataFrom($data, self::MERGE_AS_INTERNAL_VALUE); } return $this; } @@ -1317,9 +1317,11 @@ class Form extends ViewableData implements HasRequestHandler return $result; } - const MERGE_DEFAULT = 0; - const MERGE_CLEAR_MISSING = 1; - const MERGE_IGNORE_FALSEISH = 2; + const MERGE_DEFAULT = 0b0000; + const MERGE_CLEAR_MISSING = 0b0001; + const MERGE_IGNORE_FALSEISH = 0b0010; + const MERGE_AS_INTERNAL_VALUE = 0b0100; + const MERGE_AS_SUBMITTED_VALUE = 0b1000; /** * Load data from the given DataObject or array. @@ -1340,6 +1342,7 @@ class Form extends ViewableData implements HasRequestHandler * {@link saveInto()}. * * @uses FieldList->dataFields() + * @uses FormField->setSubmittedValue() * @uses FormField->setValue() * * @param array|DataObject $data @@ -1359,6 +1362,11 @@ class Form extends ViewableData implements HasRequestHandler * Passing IGNORE_FALSEISH means that any false-ish value in {@link $data} won't replace * a field's value. * + * Passing MERGE_AS_INTERNAL_VALUE forces the data to be parsed using the internal representation of the matching + * form field. This is helpful if you are loading an array of values retrieved from `Form::getData()` and you + * do not want them parsed as submitted data. MERGE_AS_SUBMITTED_VALUE does the opposite and forces the data to be + * parsed as it would be submitted from a form. + * * For backwards compatibility reasons, this parameter can also be set to === true, which is the same as passing * CLEAR_MISSING * @@ -1389,6 +1397,14 @@ class Form extends ViewableData implements HasRequestHandler $submitted = false; } + // Using the `MERGE_AS_INTERNAL_VALUE` or `MERGE_AS_SUBMITTED_VALUE` flags users can explicitly specify which + // `setValue` method to use. + if (($mergeStrategy & self::MERGE_AS_INTERNAL_VALUE) == self::MERGE_AS_INTERNAL_VALUE) { + $submitted = false; + } elseif (($mergeStrategy & self::MERGE_AS_SUBMITTED_VALUE) == self::MERGE_AS_SUBMITTED_VALUE) { + $submitted = true; + } + // dont include fields without data $dataFields = $this->Fields()->dataFields(); if (!$dataFields) { diff --git a/tests/php/Forms/FormTest.php b/tests/php/Forms/FormTest.php index ae7e1d7bd..3322eb1b5 100644 --- a/tests/php/Forms/FormTest.php +++ b/tests/php/Forms/FormTest.php @@ -8,6 +8,7 @@ use SilverStripe\Control\Session; use SilverStripe\Dev\CSSContentParser; use SilverStripe\Dev\FunctionalTest; use SilverStripe\Forms\DateField; +use SilverStripe\Forms\DatetimeField; use SilverStripe\Forms\FieldList; use SilverStripe\Forms\FileField; use SilverStripe\Forms\Form; @@ -17,16 +18,20 @@ use SilverStripe\Forms\LookupField; use SilverStripe\Forms\NumericField; use SilverStripe\Forms\PasswordField; use SilverStripe\Forms\Tests\FormTest\ControllerWithSecurityToken; +use SilverStripe\Forms\Tests\FormTest\ControllerWithSpecialSubmittedValueFields; use SilverStripe\Forms\Tests\FormTest\ControllerWithStrictPostCheck; use SilverStripe\Forms\Tests\FormTest\Player; use SilverStripe\Forms\Tests\FormTest\Team; use SilverStripe\Forms\Tests\FormTest\TestController; +use SilverStripe\Forms\Tests\ValidatorTest\TestValidator; use SilverStripe\Forms\TextareaField; use SilverStripe\Forms\TextField; +use SilverStripe\Forms\TimeField; use SilverStripe\ORM\ValidationResult; use SilverStripe\Security\NullSecurityToken; use SilverStripe\Security\RandomGenerator; use SilverStripe\Security\SecurityToken; +use SilverStripe\View\ArrayData; use SilverStripe\View\SSViewer; /** @@ -46,6 +51,7 @@ class FormTest extends FunctionalTest TestController::class, ControllerWithSecurityToken::class, ControllerWithStrictPostCheck::class, + ControllerWithSpecialSubmittedValueFields::class ]; protected static $disable_themes = true; @@ -261,6 +267,46 @@ class FormTest extends FunctionalTest ); } + public function testLoadDataFromWithForceSetValueFlag() + { + // Get our data formatted in internal value and in submitted value + // We're using very esoteric date and time format + $dataInSubmittedValue = [ + 'SomeDateTimeField' => 'Fri, Jun 15, \'18 17:28:05', + 'SomeTimeField' => '05 o\'clock PM 28 05' + ]; + $dataInInternalValue = [ + 'SomeDateTimeField' => '2018-06-15 17:28:05', + 'SomeTimeField' => '17:28:05' + ]; + + // Test loading our data with the MERGE_AS_INTERNAL_VALUE + $form = $this->getStubFormWithWeirdValueFormat(); + $form->loadDataFrom($dataInInternalValue, Form::MERGE_AS_INTERNAL_VALUE); + + $this->assertEquals( + $dataInInternalValue, + $form->getData() + ); + + // Test loading our data with the MERGE_AS_SUBMITTED_VALUE and an data passed as an object + $form = $this->getStubFormWithWeirdValueFormat(); + $form->loadDataFrom(ArrayData::create($dataInSubmittedValue), Form::MERGE_AS_SUBMITTED_VALUE); + $this->assertEquals( + $dataInInternalValue, + $form->getData() + ); + + // Test loading our data without the MERGE_AS_INTERNAL_VALUE and without MERGE_AS_SUBMITTED_VALUE + $form = $this->getStubFormWithWeirdValueFormat(); + $form->loadDataFrom($dataInSubmittedValue); + + $this->assertEquals( + $dataInInternalValue, + $form->getData() + ); + } + public function testLookupFieldDisabledSaving() { $object = new Team(); @@ -275,10 +321,10 @@ class FormTest extends FunctionalTest $form->loadDataFrom( array( 'Players' => array( - 14, - 18, - 22 - ), + 14, + 18, + 22 + ), ) ); $form->saveInto($object); @@ -969,6 +1015,64 @@ class FormTest extends FunctionalTest } } + /** + * This test confirms that when a form validation fails, the submitted value are stored in the session and are + * reloaded correctly once the form is re-rendered. This indirectly test `Form::restoreFormState`, + * `Form::setSessionData`, `Form::getSessionData` and `Form::clearFormState`. + */ + public function testRestoreFromState() + { + // Use a specially crafted controlled for this request. The associated form contains fields that override the + // `setSubmittedValue` and require an internal format that differs from the submitted format. + $this->get('FormTest_ControllerWithSpecialSubmittedValueFields')->getBody(); + + // Posting our form. This should fail and redirect us to the form page and preload our submit value + $response = $this->post( + 'FormTest_ControllerWithSpecialSubmittedValueFields/Form', + array( + 'SomeDateField' => '15/06/2018', + 'SomeFrenchNumericField' => '9 876,5432', + 'SomeFrenchMoneyField' => [ + 'Amount' => '9 876,54', + 'Currency' => 'NZD' + ] + // Validation will fail because we leave out SomeRequiredField + ), + [] + ); + + // Test our reloaded form field + $body = $response->getBody(); + $this->assertContains( + 'assertContains( + 'assertContains( + 'assertContains( + 'assertEmpty( + $this->mainSession->session()->get('FormInfo.Form_Form'), + 'Our form was reloaded successfully. That should have cleared our session.' + ); + } + protected function getStubForm() { return new Form( @@ -978,4 +1082,28 @@ class FormTest extends FunctionalTest new FieldList() ); } + + /** + * Some fields handle submitted values differently from their internal values. This forms contains 2 such fields + * * a SomeDateTimeField that expect a date such as `Fri, Jun 15, '18 17:28:05`, + * * a SomeTimeField that expects it's time as `05 o'clock PM 28 05` + * + * @return Form + */ + protected function getStubFormWithWeirdValueFormat() + { + return new Form( + Controller::curr(), + 'Form', + new FieldList( + $dateField = DatetimeField::create('SomeDateTimeField') + ->setHTML5(false) + ->setDatetimeFormat("EEE, MMM d, ''yy HH:mm:ss"), + $timeField = TimeField::create('SomeTimeField') + ->setHTML5(false) + ->setTimeFormat("hh 'o''clock' a mm ss") // Swatch Internet Time format + ), + new FieldList() + ); + } } diff --git a/tests/php/Forms/FormTest/ControllerWithSpecialSubmittedValueFields.php b/tests/php/Forms/FormTest/ControllerWithSpecialSubmittedValueFields.php new file mode 100644 index 000000000..c373b124a --- /dev/null +++ b/tests/php/Forms/FormTest/ControllerWithSpecialSubmittedValueFields.php @@ -0,0 +1,103 @@ +setRequest(Controller::curr()->getRequest()); + } + } + + private static $allowed_actions = array('Form'); + + private static $url_handlers = array( + '$Action//$ID/$OtherID' => "handleAction", + ); + + protected $template = 'BlankPage'; + + public function Link($action = null) + { + return Controller::join_links( + 'FormTest_ControllerWithSpecialSubmittedValueFields', + $this->getRequest()->latestParam('Action'), + $this->getRequest()->latestParam('ID'), + $action + ); + } + + public function Form() + { + $form = new Form( + $this, + 'Form', + new FieldList( + new TextField('SomeRequiredField'), + DateField::create('SomeDateField') + ->setHTML5(false) + ->setDateFormat('dd/MM/yyyy') + ->setValue('2000-01-01'), + NumericField::create('SomeFrenchNumericField') + ->setHTML5(false) + ->setLocale('fr_FR') + ->setScale(4) + ->setValue(12345.6789), + MoneyField::create('SomeFrenchMoneyField') + ->setValue('100.5 EUR') + ->setLocale('fr_FR') + ), + new FieldList( + FormAction::create('doSubmit') + ), + new RequiredFields( + 'SomeRequiredField' + ) + ); + $form->setValidationExemptActions(array('doSubmitValidationExempt')); + $form->disableSecurityToken(); // Disable CSRF protection for easier form submission handling + + return $form; + } + + public function doSubmit($data, $form, $request) + { + $form->sessionMessage('Test save was successful', 'good'); + return $this->redirectBack(); + } + + public function doTriggerException($data, $form, $request) + { + $result = new ValidationResult(); + $result->addFieldError('Email', 'Error on Email field'); + $result->addError('Error at top of form'); + throw new ValidationException($result); + } + + public function getViewer($action = null) + { + return new SSViewer('BlankPage'); + } +}