Change method to only return true/false.

null/true/false is complicated. Fix issues with some of the test fixtures. imalsonotrequired was actually required as it's last rule didn't specify allowEmpty = true and had a range validation rule.
cakephp · Mar 22, 2012 · 69e63b1 · 69e63b1
1 parent cabb0d0
commit 69e63b1
Show file tree

Hide file tree

Showing 2 changed files with 36 additions and 29 deletions.
diff --git a/lib/Cake/Test/Case/View/Helper/FormHelperTest.php b/lib/Cake/Test/Case/View/Helper/FormHelperTest.php
@@ -106,15 +106,18 @@ class Contact extends CakeTestModel {
 		'imrequired' => array('rule' => array('between', 5, 30), 'allowEmpty' => false),
 		'imrequiredonupdate' => array('notEmpty' => array('rule' => 'alphaNumeric', 'on' => 'update')),
 		'imrequiredoncreate' => array('required' => array('rule' => 'alphaNumeric', 'on' => 'create')),
-		'imrequiredonboth' => array('required' => array('rule' => 'alphaNumeric')),
+		'imrequiredonboth' => array(
+			'required' => array('rule' => 'alphaNumeric', 'allowEmpty' => true),
+			'check' => array('rule' => 'alphaNumeric')
+		),
 		'string_required' => 'notEmpty',
 		'imalsorequired' => array('rule' => 'alphaNumeric', 'allowEmpty' => false),
 		'imrequiredtoo' => array('rule' => 'notEmpty'),
 		'required_one' => array('required' => array('rule' => array('notEmpty'))),
 		'imnotrequired' => array('required' => false, 'rule' => 'alphaNumeric', 'allowEmpty' => true),
 		'imalsonotrequired' => array(
-			'alpha' => array('rule' => 'alphaNumeric','allowEmpty' => true),
-			'between' => array('rule' => array('between', 5, 30)),
+			'alpha' => array('rule' => 'alphaNumeric', 'allowEmpty' => true),
+			'between' => array('rule' => array('between', 5, 30), 'allowEmpty' => true),
 		),
 		'imnotrequiredeither' => array('required' => true, 'rule' => array('between', 5, 30), 'allowEmpty' => true),
 	);

diff --git a/lib/Cake/View/Helper/FormHelper.php b/lib/Cake/View/Helper/FormHelper.php
@@ -249,10 +249,8 @@ protected function _isRequiredField($validateProperties) {
 			}
 
 			foreach ($validateProperties as $rule => $validateProp) {
-				$parse = $this->_parseRuleRequired($validateProp);
-				if ($parse === false) {
-					return false;
-				} elseif ($parse == null){
+				$isRequired = $this->_isRequiredRule($validateProp);
+				if ($isRequired === false) {
 					continue;
 				}
 				$rule = isset($validateProp['rule']) ? $validateProp['rule'] : false;
@@ -265,6 +263,34 @@ protected function _isRequiredField($validateProperties) {
 		return $required;
 	}
 
+/**
+ * Checks if the field is required by the 'on' key in validation properties.
+ * If no 'on' key is present in validation props, this method returns true.
+ *
+ * @param array $validateProp
+ * @return mixed. Boolean for required
+ */
+	protected function _isRequiredRule($validateProp) {
+		if (isset($validateProp['on'])) {
+			if (
+				($validateProp['on'] == 'create' && $this->requestType != 'post') ||
+				($validateProp['on'] == 'update' && $this->requestType != 'put')
+			) {
+				return false;
+			}
+		}
+		if (
+			isset($validateProp['allowEmpty']) &&
+			$validateProp['allowEmpty'] === true
+		) {
+			return false;
+		}
+		if (isset($validateProp['required']) && empty($validateProp['required'])) {
+			return false;
+		}
+		return true;
+	}
+
 /**
  * Returns false if given form field described by the current entity has no errors.
  * Otherwise it returns the validation message
@@ -2587,26 +2613,4 @@ protected function _initInputField($field, $options = array()) {
 		return $result;
 	}
 
-/**
- * Checks if the field is required by the 'on' key in validation properties.
- * If no 'on' key is present in validation props, this method returns true.
- *
- * @param array $validateProp
- * @return mixed. Boolean for required, null if create/update does not match
- */
-	protected function _parseRuleRequired($validateProp) {
-		if (isset($validateProp['on'])) {
-			if (
-				($validateProp['on'] == 'create' && $this->requestType != 'post') ||
-				($validateProp['on'] == 'update' && $this->requestType != 'put')
-			) {
-				return null;
-			}
-		}
-		if (isset($validateProp['allowEmpty']) && $validateProp['allowEmpty'] === true) {
-			return false;
-		}
-		return true;
-	}
-
 }