Form action validation excempts

Thanks to @kinglozzer for doing the majority of work on this. See #2493.

admin/code/CMSForm.php

@@ -9,24 +9,6 @@
  */
 class CMSForm extends Form {
 	
-	/**
-	 * @var array
-	 */
-	protected $validationExemptActions = array();
-
-	/**
-	 * Always return true if the current form action is exempt from validation
-	 *
-	 * @return boolean
-	 */
-	public function validate() {
-		$buttonClicked = $this->buttonClicked();
-		return (
-			($buttonClicked && in_array($buttonClicked->actionName(), $this->getValidationExemptActions()))
-			|| parent::validate()
-		);
-	}
-
 	/**
 	 * Route validation error responses through response negotiator,
 	 * so they return the correct markup as expected by the requesting client.
@@ -49,25 +31,6 @@ class CMSForm extends Form {
 		}
 	}
 
-	/**
-	 * Set actions that are exempt from validation
-	 *
-	 * @param array
-	 */
-	public function setValidationExemptActions($actions) {
-		$this->validationExemptActions = $actions;
-		return $this;
-	}
-
-	/**
-	 * Get a list of actions that are exempt from validation
-	 *
-	 * @return array
-	 */
-	public function getValidationExemptActions() {
-		return $this->validationExemptActions;
-	}
-
 	/**
 	 * Sets the response negotiator
 	 * @param ResponseNegotiator $negotiator The response negotiator to use

admin/tests/CMSFormTest.php

@@ -1,127 +0,0 @@
-<?php
-
-/**
- * @package framework
- * @subpackage tests
- */
-class CMSFormTest extends FunctionalTest {
-
-	public function testValidationExemptActions() {
-		$response = $this->get('CMSFormTest_Controller');
-
-		$response = $this->submitForm(
-			'CMSForm_Form',
-			'action_doSubmit',
-			array(
-				'Email' => 'test@test.com'
-			)
-		);
-
-		// Firstly, assert that required fields still work when not using an exempt action
-		$this->assertPartialMatchBySelector(
-			'#CMSForm_Form_SomeRequiredField_Holder span.required',
-			array(
-				'"Some Required Field" is required'
-			),
-			'Required fields show a notification on field when left blank'
-		);
-
-		// Re-submit the form using validation-exempt button
-		$response = $this->submitForm(
-			'CMSForm_Form',
-			'action_doSubmitValidationExempt',
-			array(
-				'Email' => 'test@test.com'
-			)
-		);
-
-		// The required message should be empty if validation was skipped
-		$items = $this->cssParser()->getBySelector('#CMSForm_Form_SomeRequiredField_Holder span.required');
-		$this->assertEmpty($items);
-
-		// And the session message should show up is submitted successfully
-		$this->assertPartialMatchBySelector(
-			'#CMSForm_Form_error',
-			array(
-				'Validation skipped'
-			),
-			'Form->sessionMessage() shows up after reloading the form'
-		);
-	}
-
-	public function testSetValidationExemptActions() {
-		$form = $this->getStubForm();
-
-		$form->setValidationExemptActions(array('exemptaction'));
-		$exemptActions = $form->getValidationExemptActions();
-		$this->assertEquals('exemptaction', $exemptActions[0]);
-	}
-
-	protected function getStubForm() {
-		$form = new CMSForm(
-			new CMSFormTest_Controller(),
-			'CMSForm',
-			new FieldList(),
-			new FieldList()
-		);
-
-		return $form;
-	}
-
-}
-
-class CMSFormTest_Controller extends Controller implements TestOnly {
-
-	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('CMSFormTest_Controller', $this->request->latestParam('Action'),
-			$this->request->latestParam('ID'), $action);
-	}
-
-	public function Form() {
-		$form = new CMSForm(
-			$this,
-			'Form',
-			new FieldList(
-				new EmailField('Email'),
-				new TextField('SomeRequiredField'),
-				new CheckboxSetField('Boxes', null, array('1'=>'one','2'=>'two'))
-			),
-			new FieldList(
-				new FormAction('doSubmit'),
-				new FormAction('doSubmitValidationExempt')
-			),
-			new RequiredFields(
-				'Email',
-				'SomeRequiredField'
-			)
-		);
-		$form->setValidationExemptActions(array('doSubmitValidationExempt'));
-		$form->setResponseNegotiator('foo'); // We aren't testing AJAX responses, so just set anything
-		$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 doSubmitValidationExempt($data, $form, $request) {
-		$form->sessionMessage('Validation skipped', 'good');
-		return $this->redirectBack();
-	}
-
-	public function getViewer($action = null) {
-		return new SSViewer('BlankPage');
-	}
-
-}

forms/Form.php

@@ -152,6 +152,11 @@ class Form extends RequestHandler {
 	 */
 	protected $attributes = array();
 
+	/**
+	 * @var array
+	 */
+	protected $validationExemptActions = array();
+
 	private static $allowed_actions = array(
 		'handleField',
 		'httpSubmission',
@@ -581,6 +586,25 @@ class Form extends RequestHandler {
 		return $this;
 	}
 
+	/**
+	 * Set actions that are exempt from validation
+	 * 
+	 * @param array
+	 */
+	public function setValidationExemptActions($actions) {
+		$this->validationExemptActions = $actions;
+		return $this;
+	}
+
+	/**
+	 * Get a list of actions that are exempt from validation
+	 * 
+	 * @return array
+	 */
+	public function getValidationExemptActions() {
+		return $this->validationExemptActions;
+	}
+
 	/**
 	 * Convert this form to another format.
 	 */
@@ -1188,6 +1212,7 @@ class Form extends RequestHandler {
 	 *
 	 * This includes form validation, if it fails, we redirect back
 	 * to the form with appropriate error messages.
+	 * Always return true if the current form action is exempt from validation
 	 *
 	 * Triggered through {@link httpSubmission()}.
 	 *
@@ -1197,6 +1222,11 @@ class Form extends RequestHandler {
 	 * @return boolean
 	 */
 	public function validate(){
+		$buttonClicked = $this->buttonClicked();
+		if($buttonClicked && in_array($buttonClicked->actionName(), $this->getValidationExemptActions())) {
+			return true;
+		}
+
 		if($this->validator){
 			$errors = $this->validator->validate();
 
@@ -1535,7 +1565,10 @@ class Form extends RequestHandler {
 	 * @return FormAction
 	 */
 	public function buttonClicked() {
-		foreach($this->actions->dataFields() as $action) {
+		$actions = $this->actions->dataFields();
+		if(!$actions) return;
+
+		foreach($actions as $action) {
 			if($action->hasMethod('actionname') && $this->buttonClickedFunc == $action->actionName()) {
 				return $action;
 			}

tests/forms/FormTest.php

@@ -228,6 +228,47 @@ class FormTest extends FunctionalTest {
 		);
 	}
 
+	public function testValidationExemptActions() {
+		$response = $this->get('FormTest_Controller');
+
+		$response = $this->submitForm(
+			'Form_Form',
+			'action_doSubmit',
+			array(
+				'Email' => 'test@test.com'
+			)
+		);
+			
+		// Firstly, assert that required fields still work when not using an exempt action
+		$this->assertPartialMatchBySelector(
+			'#Form_Form_SomeRequiredField_Holder .required',
+			array('"Some Required Field" is required'),
+			'Required fields show a notification on field when left blank'
+		);
+
+		// Re-submit the form using validation-exempt button
+		$response = $this->submitForm(
+			'Form_Form',
+			'action_doSubmitValidationExempt',
+			array(
+				'Email' => 'test@test.com'
+			)
+		);
+
+		// The required message should be empty if validation was skipped
+		$items = $this->cssParser()->getBySelector('#Form_Form_SomeRequiredField_Holder .required');
+		$this->assertEmpty($items);
+
+		// And the session message should show up is submitted successfully
+		$this->assertPartialMatchBySelector(
+			'#Form_Form_error',
+			array(
+				'Validation skipped'
+			),
+			'Form->sessionMessage() shows up after reloading the form'
+		);
+	}
+	
 	public function testSessionValidationMessage() {
 		$this->get('FormTest_Controller');
 
@@ -634,13 +675,15 @@ class FormTest_Controller extends Controller implements TestOnly {
 				new CheckboxSetField('Boxes', null, array('1'=>'one','2'=>'two'))
 			),
 			new FieldList(
-				new FormAction('doSubmit')
+				new FormAction('doSubmit'),
+				new FormAction('doSubmitValidationExempt')
 			),
 			new RequiredFields(
 				'Email',
 				'SomeRequiredField'
 			)
 		);
+		$form->setValidationExemptActions(array('doSubmitValidationExempt'));
 		$form->disableSecurityToken(); // Disable CSRF protection for easier form submission handling
 
 		return $form;
@@ -651,6 +694,11 @@ class FormTest_Controller extends Controller implements TestOnly {
 		return $this->redirectBack();
 	}
 
+	public function doSubmitValidationExempt($data, $form, $request) {
+		$form->sessionMessage('Validation skipped', 'good');
+		return $this->redirectBack();
+	}
+
 	public function getViewer($action = null) {
 		return new SSViewer('BlankPage');
 	}