mirror of
https://github.com/silverstripe/silverstripe-framework
synced 2024-10-22 14:05:37 +02:00
FIX Don't reload form session data using FormField::setSubmittedValue… (#8184)
This commit is contained in:
Maxime Rainville
Damian Mooyman
parent
8181dc4fd2
commit
3f80e2dc67
@ -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;
|
||||
}
|
||||
@ -1332,9 +1332,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.
|
||||
@ -1355,6 +1357,7 @@ class Form extends ViewableData implements HasRequestHandler
|
||||
* {@link saveInto()}.
|
||||
*
|
||||
* @uses FieldList->dataFields()
|
||||
* @uses FormField->setSubmittedValue()
|
||||
* @uses FormField->setValue()
|
||||
*
|
||||
* @param array|DataObject $data
|
||||
@ -1374,6 +1377,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
|
||||
*
|
||||
@ -1404,6 +1412,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) {
|
||||
|
||||
@ -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(
|
||||
'<input type="text" name="SomeDateField" value="15/06/2018"',
|
||||
$body,
|
||||
'Our reloaded form should contain a SomeDateField with the value "15/06/2018"'
|
||||
);
|
||||
|
||||
$this->assertContains(
|
||||
'<input type="text" name="SomeFrenchNumericField" value="9 876,5432" ',
|
||||
$body,
|
||||
'Our reloaded form should contain a SomeFrenchNumericField with the value "9 876,5432"'
|
||||
);
|
||||
|
||||
$this->assertContains(
|
||||
'<input type="text" name="SomeFrenchMoneyField[Currency]" value="NZD"',
|
||||
$body,
|
||||
'Our reloaded form should contain a SomeFrenchMoneyField[Currency] with the value "NZD"'
|
||||
);
|
||||
|
||||
$this->assertContains(
|
||||
'<input type="text" name="SomeFrenchMoneyField[Amount]" value="9 876,54" ',
|
||||
$body,
|
||||
'Our reloaded form should contain a SomeFrenchMoneyField[Amount] with the value "9 876,54"'
|
||||
);
|
||||
|
||||
$this->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()
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
@ -0,0 +1,103 @@
|
||||
<?php
|
||||
|
||||
namespace SilverStripe\Forms\Tests\FormTest;
|
||||
|
||||
use SilverStripe\Control\Controller;
|
||||
use SilverStripe\Dev\TestOnly;
|
||||
use SilverStripe\Forms\CheckboxSetField;
|
||||
use SilverStripe\Forms\DateField;
|
||||
use SilverStripe\Forms\EmailField;
|
||||
use SilverStripe\Forms\FieldList;
|
||||
use SilverStripe\Forms\Form;
|
||||
use SilverStripe\Forms\FormAction;
|
||||
use SilverStripe\Forms\MoneyField;
|
||||
use SilverStripe\Forms\NumericField;
|
||||
use SilverStripe\Forms\RequiredFields;
|
||||
use SilverStripe\Forms\TextField;
|
||||
use SilverStripe\ORM\ValidationException;
|
||||
use SilverStripe\ORM\ValidationResult;
|
||||
use SilverStripe\View\SSViewer;
|
||||
|
||||
/**
|
||||
* @skipUpgrade
|
||||
*/
|
||||
class ControllerWithSpecialSubmittedValueFields extends Controller implements TestOnly
|
||||
{
|
||||
public function __construct()
|
||||
{
|
||||
parent::__construct();
|
||||
if (Controller::has_curr()) {
|
||||
$this->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');
|
||||
}
|
||||
}
|
||||
Loading…
Reference in New Issue
Block a user