Merge pull request #409 from pulls/core-validation

This commit is contained in:
Daniel Hensby 2016-04-22 14:44:59 +01:00
commit b70c66e950
No known key found for this signature in database
GPG Key ID: B00D1E9767F0B06E
9 changed files with 164 additions and 109 deletions

View File

@ -22,7 +22,7 @@ class UserFormValidator extends RequiredFields {
// Page at top level, or after another page is ok // Page at top level, or after another page is ok
if(empty($stack) || (count($stack) === 1 && $stack[0] instanceof EditableFormStep)) { if(empty($stack) || (count($stack) === 1 && $stack[0] instanceof EditableFormStep)) {
$stack = array($field); $stack = array($field);
$conditionalStep = $field->DisplayRules()->count() > 0; $conditionalStep = $field->EffectiveDisplayRules()->count() > 0;
continue; continue;
} }

View File

@ -27,6 +27,9 @@ class UserForm extends Form {
$actions->setForm($this); $actions->setForm($this);
$this->setValidator($this->getRequiredFields()); $this->setValidator($this->getRequiredFields());
// This needs to be re-evaluated since fields have been assigned
$this->setupFormErrors();
// Number each page // Number each page
$stepNumber = 1; $stepNumber = 1;
foreach($this->getSteps() as $step) { foreach($this->getSteps() as $step) {
@ -46,6 +49,16 @@ class UserForm extends Form {
$this->extend('updateForm'); $this->extend('updateForm');
} }
public function setupFormErrors()
{
// Suppress setupFormErrors if fields haven't been bootstrapped
if($this->fields && $this->fields->exists()) {
return parent::setupFormErrors();
}
return $this;
}
/** /**
* Used for partial caching in the template. * Used for partial caching in the template.
* *
@ -90,6 +103,7 @@ class UserForm extends Form {
} }
$fields->clearEmptySteps(); $fields->clearEmptySteps();
$this->extend('updateFormFields', $fields); $this->extend('updateFormFields', $fields);
$fields->setForm($this);
return $fields; return $fields;
} }
@ -115,7 +129,7 @@ class UserForm extends Form {
} }
$this->extend('updateFormActions', $actions); $this->extend('updateFormActions', $actions);
$actions->setForm($this);
return $actions; return $actions;
} }
@ -127,37 +141,16 @@ class UserForm extends Form {
public function getRequiredFields() { public function getRequiredFields() {
// Generate required field validator // Generate required field validator
$requiredNames = $this $requiredNames = $this
->controller ->getController()
->Fields() ->Fields()
->filter('Required', true) ->filter('Required', true)
->column('Name'); ->column('Name');
$required = new RequiredFields($requiredNames); $required = new RequiredFields($requiredNames);
$this->extend('updateRequiredFields', $required); $this->extend('updateRequiredFields', $required);
$required->setForm($this);
return $required; return $required;
} }
/**
* Override validation so conditional fields can be validated correctly.
*
* @return boolean
*/
public function validate() {
$data = $this->getData();
Session::set("FormInfo.{$this->FormName()}.data", $data);
Session::clear("FormInfo.{$this->FormName()}.errors");
foreach ($this->controller->Fields() as $key => $field) {
$field->validateField($data, $this);
}
if(Session::get("FormInfo.{$this->FormName()}.errors")) {
return false;
}
return true;
}
/** /**
* Override some we can add UserForm specific attributes to the form. * Override some we can add UserForm specific attributes to the form.
* *

View File

@ -426,7 +426,7 @@ class UserDefinedForm_Controller extends Page_Controller {
} }
// Check for field dependencies / default // Check for field dependencies / default
foreach($field->DisplayRules() as $rule) { foreach($field->EffectiveDisplayRules() as $rule) {
// Get the field which is effected // Get the field which is effected
$formFieldWatch = EditableFormField::get()->byId($rule->ConditionFieldID); $formFieldWatch = EditableFormField::get()->byId($rule->ConditionFieldID);

View File

@ -8,7 +8,12 @@ use SilverStripe\Forms\SegmentField;
* *
* @package userforms * @package userforms
* *
* @property string Name * @property string $Name
* @property string $Title
* @property string $Default
* @property int $Sort
* @property bool $Required
* @property string $CustomErrorMessage
* @method UserDefinedForm Parent() Parent page * @method UserDefinedForm Parent() Parent page
* @method DataList DisplayRules() List of EditableCustomRule objects * @method DataList DisplayRules() List of EditableCustomRule objects
*/ */
@ -203,14 +208,39 @@ class EditableFormField extends DataObject {
// Validation // Validation
$validationFields = $this->getFieldValidationOptions(); $validationFields = $this->getFieldValidationOptions();
if($validationFields) { if($validationFields && $validationFields->count()) {
$fields->addFieldsToTab( $fields->addFieldsToTab('Root.Validation', $validationFields);
'Root.Validation', }
$this->getFieldValidationOptions()
// Add display rule fields
$displayFields = $this->getDisplayRuleFields();
if($displayFields && $displayFields->count()) {
$fields->addFieldsToTab('Root.DisplayRules', $displayFields);
}
$this->extend('updateCMSFields', $fields);
return $fields;
}
/**
* Return fields to display on the 'Display Rules' tab
*
* @return FieldList
*/
protected function getDisplayRuleFields() {
// Check display rules
if($this->Required) {
return new FieldList(
LabelField::create(_t(
'EditableFormField.DISPLAY_RULES_DISABLED',
'Display rules are not enabled for required fields. ' .
'Please uncheck "Is this field Required?" under "Validation" to re-enable.'
))->addExtraClass('message warning')
); );
} }
$allowedClasses = array_keys($this->getEditableFieldClasses(false));
$self = $this; $self = $this;
$allowedClasses = array_keys($this->getEditableFieldClasses(false));
$editableColumns = new GridFieldEditableColumns(); $editableColumns = new GridFieldEditableColumns();
$editableColumns->setDisplayFields(array( $editableColumns->setDisplayFields(array(
'Display' => '', 'Display' => '',
@ -251,7 +281,7 @@ class EditableFormField extends DataObject {
new GridFieldDeleteAction() new GridFieldDeleteAction()
); );
$fields->addFieldsToTab('Root.DisplayRules', array( return new FieldList(
CheckboxField::create('ShowOnLoad') CheckboxField::create('ShowOnLoad')
->setDescription(_t( ->setDescription(_t(
'EditableFormField.SHOWONLOAD', 'EditableFormField.SHOWONLOAD',
@ -263,16 +293,9 @@ class EditableFormField extends DataObject {
$this->DisplayRules(), $this->DisplayRules(),
$customRulesConfig $customRulesConfig
) )
)); );
$this->extend('updateCMSFields', $fields);
return $fields;
} }
/**
* @return void
*/
public function onBeforeWrite() { public function onBeforeWrite() {
parent::onBeforeWrite(); parent::onBeforeWrite();
@ -320,24 +343,24 @@ class EditableFormField extends DataObject {
return false; return false;
} }
/** /**
* Return whether a user can delete this form field * Return whether a user can delete this form field
* based on whether they can edit the page * based on whether they can edit the page
* *
* @param Member $member * @param Member $member
* @return bool * @return bool
*/ */
public function canDelete($member = null) { public function canDelete($member = null) {
return $this->canEdit($member); return $this->canEdit($member);
} }
/** /**
* Return whether a user can edit this form field * Return whether a user can edit this form field
* based on whether they can edit the page * based on whether they can edit the page
* *
* @param Member $member * @param Member $member
* @return bool * @return bool
*/ */
public function canEdit($member = null) { public function canEdit($member = null) {
$parent = $this->Parent(); $parent = $this->Parent();
if($parent && $parent->exists()) { if($parent && $parent->exists()) {
@ -346,7 +369,7 @@ class EditableFormField extends DataObject {
// Fallback to secure admin permissions // Fallback to secure admin permissions
return parent::canEdit($member); return parent::canEdit($member);
} }
/** /**
* Return whether a user can view this form field * Return whether a user can view this form field
@ -678,7 +701,7 @@ class EditableFormField extends DataObject {
*/ */
protected function updateFormField($field) { protected function updateFormField($field) {
// set the error / formatting messages // set the error / formatting messages
$field->setCustomValidationMessage($this->getErrorMessage()); $field->setCustomValidationMessage($this->getErrorMessage()->RAW());
// set the right title on this field // set the right title on this field
if($this->RightTitle) { if($this->RightTitle) {
@ -741,34 +764,6 @@ class EditableFormField extends DataObject {
return DBField::create_field('Varchar', $errorMessage); return DBField::create_field('Varchar', $errorMessage);
} }
/**
* Validate the field taking into account its custom rules.
*
* @param Array $data
* @param UserForm $form
*
* @return boolean
*/
public function validateField($data, $form) {
if($this->Required && $this->DisplayRules()->Count() == 0) {
$formField = $this->getFormField();
if(isset($data[$this->Name])) {
$formField->setValue($data[$this->Name]);
}
if(
!isset($data[$this->Name]) ||
!$data[$this->Name] ||
!$formField->validate($form->getValidator())
) {
$form->addErrorMessage($this->Name, $this->getErrorMessage()->HTML(), 'error', false);
}
}
return true;
}
/** /**
* Invoked by UserFormUpgradeService to migrate settings specific to this field from CustomSettings * Invoked by UserFormUpgradeService to migrate settings specific to this field from CustomSettings
* to the field proper * to the field proper
@ -874,4 +869,17 @@ class EditableFormField extends DataObject {
return EditableFormFieldValidator::create() return EditableFormFieldValidator::create()
->setRecord($this); ->setRecord($this);
} }
/**
* Determine effective display rules for this field.
*
* @return SS_List
*/
public function EffectiveDisplayRules() {
if($this->Required) {
return new ArrayList();
}
return $this->DisplayRules();
}
} }

View File

@ -38,7 +38,7 @@
}, },
"extra": { "extra": {
"branch-alias": { "branch-alias": {
"dev-master": "3.1.x-dev" "dev-master": "4.0.x-dev"
} }
} }
} }

View File

@ -55,7 +55,8 @@ class EditableFormFieldTest extends FunctionalTest {
// form has 2 fields - a checkbox and a text field // form has 2 fields - a checkbox and a text field
// it has 1 rule - when ticked the checkbox hides the text field // it has 1 rule - when ticked the checkbox hides the text field
$this->assertEquals($rules->Count(), 1); $this->assertEquals(1, $rules->Count());
$this->assertEquals($rules, $checkbox->EffectiveDisplayRules());
$checkboxRule = $rules->First(); $checkboxRule = $rules->First();
$checkboxRule->ConditionFieldID = $field->ID; $checkboxRule->ConditionFieldID = $field->ID;
@ -63,6 +64,10 @@ class EditableFormFieldTest extends FunctionalTest {
$this->assertEquals($checkboxRule->Display, 'Hide'); $this->assertEquals($checkboxRule->Display, 'Hide');
$this->assertEquals($checkboxRule->ConditionOption, 'HasValue'); $this->assertEquals($checkboxRule->ConditionOption, 'HasValue');
$this->assertEquals($checkboxRule->FieldValue, '6'); $this->assertEquals($checkboxRule->FieldValue, '6');
// If field is required then all custom rules are disabled
$checkbox->Required = true;
$this->assertEquals(0, $checkbox->EffectiveDisplayRules()->count());
} }
function testEditableDropdownField() { function testEditableDropdownField() {

View File

@ -7,16 +7,9 @@ class EditableLiteralFieldTest extends SapphireTest {
public function setUp() { public function setUp() {
parent::setUp(); parent::setUp();
Config::nest();
HtmlEditorConfig::set_active('cms'); HtmlEditorConfig::set_active('cms');
} }
public function tearDown() {
Config::unnest();
parent::tearDown();
}
/** /**
* Tests the sanitisation of HTML content * Tests the sanitisation of HTML content
*/ */

View File

@ -8,7 +8,7 @@ class UserDefinedFormControllerTest extends FunctionalTest {
static $fixture_file = 'UserDefinedFormTest.yml'; static $fixture_file = 'UserDefinedFormTest.yml';
function testProcess() { public function testProcess() {
$form = $this->setupFormFrontend(); $form = $this->setupFormFrontend();
$controller = new UserDefinedFormControllerTest_Controller($form); $controller = new UserDefinedFormControllerTest_Controller($form);
@ -60,7 +60,40 @@ class UserDefinedFormControllerTest extends FunctionalTest {
$this->assertStringEndsWith('finished#uff', $response->getHeader('Location')); $this->assertStringEndsWith('finished#uff', $response->getHeader('Location'));
} }
function testFinished() { public function testValidation() {
$form = $this->setupFormFrontend('email-form');
// Post with no fields
$this->get($form->URLSegment);
$response = $this->submitForm('UserForm_Form', null, array());
$this->assertPartialMatchBySelector(
'.field .message',
array('This field is required')
);
// Post with all fields, but invalid email
$this->get($form->URLSegment);
$this->submitForm('UserForm_Form', null, array(
'required-email' => 'invalid',
'required-text' => 'bob'
));
$this->assertPartialMatchBySelector(
'.field .message',
array('Please enter an email address')
);
// Post with only required
$this->get($form->URLSegment);
$this->submitForm('UserForm_Form', null, array(
'required-text' => 'bob'
));
$this->assertPartialMatchBySelector(
'p',
array("Thanks, we've received your submission.")
);
}
public function testFinished() {
$form = $this->setupFormFrontend(); $form = $this->setupFormFrontend();
// set formProcessed and SecurityID to replicate the form being filled out // set formProcessed and SecurityID to replicate the form being filled out
@ -72,7 +105,7 @@ class UserDefinedFormControllerTest extends FunctionalTest {
$this->assertContains($form->OnCompleteMessage ,$response->getBody()); $this->assertContains($form->OnCompleteMessage ,$response->getBody());
} }
function testAppendingFinished() { public function testAppendingFinished() {
$form = $this->setupFormFrontend(); $form = $this->setupFormFrontend();
// replicate finished being added to the end of the form URL without the form being filled out // replicate finished being added to the end of the form URL without the form being filled out
@ -84,7 +117,7 @@ class UserDefinedFormControllerTest extends FunctionalTest {
$this->assertNotContains($form->OnCompleteMessage ,$response->getBody()); $this->assertNotContains($form->OnCompleteMessage ,$response->getBody());
} }
function testForm() { public function testForm() {
$form = $this->objFromFixture('UserDefinedForm', 'basic-form-page'); $form = $this->objFromFixture('UserDefinedForm', 'basic-form-page');
$controller = new UserDefinedFormControllerTest_Controller($form); $controller = new UserDefinedFormControllerTest_Controller($form);
@ -103,7 +136,7 @@ class UserDefinedFormControllerTest extends FunctionalTest {
$this->assertEquals(count($controller->Form()->getValidator()->getRequired()), 1); $this->assertEquals(count($controller->Form()->getValidator()->getRequired()), 1);
} }
function testGetFormFields() { public function testGetFormFields() {
// generating the fieldset of fields // generating the fieldset of fields
$form = $this->objFromFixture('UserDefinedForm', 'basic-form-page'); $form = $this->objFromFixture('UserDefinedForm', 'basic-form-page');
@ -125,7 +158,7 @@ class UserDefinedFormControllerTest extends FunctionalTest {
$firstStep = $formSteps->first(); $firstStep = $formSteps->first();
$firstField = $firstStep->getChildren()->first(); $firstField = $firstStep->getChildren()->first();
$this->assertEquals('Custom Error Message', $firstField->getCustomValidationMessage()->getValue()); $this->assertEquals('Custom Error Message', $firstField->getCustomValidationMessage());
$this->assertEquals($firstField->Title(), 'Required Text Field <span class=\'required-identifier\'>*</span>'); $this->assertEquals($firstField->Title(), 'Required Text Field <span class=\'required-identifier\'>*</span>');
// test custom right title // test custom right title
@ -146,7 +179,8 @@ class UserDefinedFormControllerTest extends FunctionalTest {
$this->assertFalse($controller->Form()->getFormFields()->exists()); $this->assertFalse($controller->Form()->getFormFields()->exists());
} }
function testGetFormActions() { public function testGetFormActions()
{
// generating the fieldset of actions // generating the fieldset of actions
$form = $this->objFromFixture('UserDefinedForm', 'basic-form-page'); $form = $this->objFromFixture('UserDefinedForm', 'basic-form-page');
@ -155,6 +189,7 @@ class UserDefinedFormControllerTest extends FunctionalTest {
// by default will have 1 submit button which links to process // by default will have 1 submit button which links to process
$expected = new FieldList(new FormAction('process', 'Submit')); $expected = new FieldList(new FormAction('process', 'Submit'));
$expected->setForm($controller->Form());
$this->assertEquals($actions, $expected); $this->assertEquals($actions, $expected);
@ -165,11 +200,12 @@ class UserDefinedFormControllerTest extends FunctionalTest {
$expected = new FieldList(new FormAction('process', 'Custom Button')); $expected = new FieldList(new FormAction('process', 'Custom Button'));
$expected->push(new ResetFormAction("clearForm", "Clear")); $expected->push(new ResetFormAction("clearForm", "Clear"));
$expected->setForm($controller->Form());
$this->assertEquals($actions, $expected); $this->assertEquals($actions, $expected);
} }
function testRenderingIntoFormTemplate() { public function testRenderingIntoFormTemplate() {
$form = $this->setupFormFrontend(); $form = $this->setupFormFrontend();
$form->Content = 'This is some content without a form nested between it'; $form->Content = 'This is some content without a form nested between it';
@ -184,7 +220,7 @@ class UserDefinedFormControllerTest extends FunctionalTest {
$this->checkTemplateIsCorrect($parser); $this->checkTemplateIsCorrect($parser);
} }
function testRenderingIntoTemplateWithSubstringReplacement() { public function testRenderingIntoTemplateWithSubstringReplacement() {
$form = $this->setupFormFrontend(); $form = $this->setupFormFrontend();
$controller = new UserDefinedFormControllerTest_Controller($form); $controller = new UserDefinedFormControllerTest_Controller($form);
@ -195,9 +231,14 @@ class UserDefinedFormControllerTest extends FunctionalTest {
$this->checkTemplateIsCorrect($parser); $this->checkTemplateIsCorrect($parser);
} }
/**
function setupFormFrontend() { * Publish a form for use on the frontend
$form = $this->objFromFixture('UserDefinedForm', 'basic-form-page'); *
* @param string $fixtureName
* @return UserDefinedForm
*/
protected function setupFormFrontend($fixtureName = 'basic-form-page') {
$form = $this->objFromFixture('UserDefinedForm', $fixtureName);
$this->logInWithPermission('ADMIN'); $this->logInWithPermission('ADMIN');
$form->doPublish(); $form->doPublish();
@ -208,7 +249,7 @@ class UserDefinedFormControllerTest extends FunctionalTest {
return $form; return $form;
} }
function checkTemplateIsCorrect($parser) { public function checkTemplateIsCorrect($parser) {
$this->assertArrayHasKey(0, $parser->getBySelector('form#UserForm_Form')); $this->assertArrayHasKey(0, $parser->getBySelector('form#UserForm_Form'));
// check for the input // check for the input
@ -228,12 +269,12 @@ class UserDefinedFormControllerTest extends FunctionalTest {
} }
} }
class UserDefinedFormControllerTest_Controller extends UserDefinedForM_Controller implements TestOnly { class UserDefinedFormControllerTest_Controller extends UserDefinedForm_Controller implements TestOnly {
/** /**
* Overloaded to avoid inconsistencies between 2.4.2 and 2.4.3 (disables all security tokens in unit tests by default) * Overloaded to avoid inconsistencies between 2.4.2 and 2.4.3 (disables all security tokens in unit tests by default)
*/ */
function Form() { public function Form() {
$form = parent::Form(); $form = parent::Form();
if($form) $form->disableSecurityToken(); if($form) $form->disableSecurityToken();

View File

@ -101,6 +101,12 @@ EditableTextField:
some-field: some-field:
Name: SomeField Name: SomeField
another-required:
Name: required-text
Title: Required Text Field
Required: true
CustomErrorMessage: 'This field is required'
EditableDropdown: EditableDropdown:
basic-dropdown: basic-dropdown:
Name: basic-dropdown Name: basic-dropdown
@ -137,6 +143,11 @@ EditableEmailField:
Name: email-field Name: email-field
Title: Email Title: Email
another-email-field:
Name: required-email
Title: Enter your email
CustomErrorMessage: 'That email is not valid'
EditableRadioField: EditableRadioField:
radio-field: radio-field:
Name: radio-option Name: radio-option
@ -251,3 +262,7 @@ UserDefinedForm:
empty-page: empty-page:
Title: 'Page with empty step' Title: 'Page with empty step'
Fields: =>EditableFormStep.form6step1, =>EditableTextField.field-1, =>EditableFormStep.form6step2, =>EditableTextField.field-2, =>EditableFormStep.form6step3 Fields: =>EditableFormStep.form6step1, =>EditableTextField.field-1, =>EditableFormStep.form6step2, =>EditableTextField.field-2, =>EditableFormStep.form6step3
email-form:
Title: 'Page with email field'
Fields: =>EditableEmailField.another-email-field, =>EditableTextField.another-required