Skip to content

Commit

Permalink
Merge pull request #8184 from open-sausages/pulls/4.1/fix-issue-with-…
Browse files Browse the repository at this point in the history 
…loading-submitted-value-from-session

This is a cherry-picked PR from 4.2 -> 4.2.0
  • Loading branch information
@dhensby
dhensby committed Jul 23, 2018
2 parents 5f6eec6 + c77042a commit b6db400
Show file tree
Hide file tree
Showing 3 changed files with 255 additions and 8 deletions.
24 changes: 20 additions & 4 deletions src/Forms/Form.php
View file
Expand Up @@ -346,7 +346,7 @@ public function restoreFormState()
// 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;
}
Expand Down Expand Up @@ -1317,9 +1317,11 @@ public function validationResult()
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.
Expand All @@ -1340,6 +1342,7 @@ public function validationResult()
* {@link saveInto()}.
*
* @uses FieldList->dataFields()
* @uses FormField->setSubmittedValue()
* @uses FormField->setValue()
*
* @param array|DataObject $data
Expand All @@ -1359,6 +1362,11 @@ public function validationResult()
* 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
*
Expand Down Expand Up @@ -1389,6 +1397,14 @@ public function loadDataFrom($data, $mergeStrategy = 0, $fieldList = null)
$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) {
Expand Down
136 changes: 132 additions & 4 deletions tests/php/Forms/FormTest.php
View file
Expand Up @@ -8,6 +8,7 @@
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;
Expand All @@ -17,16 +18,20 @@
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;

/**
Expand All @@ -46,6 +51,7 @@ class FormTest extends FunctionalTest
TestController::class,
ControllerWithSecurityToken::class,
ControllerWithStrictPostCheck::class,
ControllerWithSpecialSubmittedValueFields::class
];

protected static $disable_themes = true;
Expand Down Expand Up @@ -261,6 +267,46 @@ public function testLoadDataFromClearMissingFields()
);
}

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();
Expand All @@ -275,10 +321,10 @@ public function testLookupFieldDisabledSaving()
$form->loadDataFrom(
array(
'Players' => array(
14,
18,
22
),
14,
18,
22
),
)
);
$form->saveInto($object);
Expand Down Expand Up @@ -969,6 +1015,64 @@ public function testPasswordPostback($allow)
}
}

/**
* 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(
Expand All @@ -978,4 +1082,28 @@ protected function getStubForm()
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()
);
}
}
103 changes: 103 additions & 0 deletions tests/php/Forms/FormTest/ControllerWithSpecialSubmittedValueFields.php
View file
@@ -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');
}
}

0 comments on commit b6db400

Please sign in to comment.