ZF-9467: Zend_Form getErrors() incorrect handling when !$subform->isArray()
Description
Zend_Form getErrors() does not handle Form Structure correctly, when calling getErrors($name = null) without parameter, and having a Form or a SubForm set isArray(false);
foreach ($this->getSubForms() as $key => $subForm) {
$fErrors = $this->_attachToArray($subForm->getErrors(), $subForm->getElementsBelongTo());
// the result
if (!$subForm->isArray()) {
$fErrors === array('' => array('foo' => ...
}
// because
if (!$subForm->isArray()) {
null === $subForm->getElementsBelongTo()
}
}
I cleaned the method a bit as well, before Patch
public function getErrors($name = null)
{
$errors = array();
if ((null !== $name) && isset($this->_elements[$name])) {
$errors = $this->getElement($name)->getErrors();
} elseif ((null !== $name) && isset($this->_subForms[$name])) {
$errors = $this->getSubForm($name)->getErrors();
} else {
foreach ($this->_elements as $key => $element) {
$errors[$key] = $element->getErrors();
}
foreach ($this->getSubForms() as $key => $subForm) {
$fErrors = $this->_attachToArray($subForm->getErrors(), $subForm->getElementsBelongTo());
$errors = array_merge($errors, $fErrors);
}
}
return $errors;
}
// Update I have learned while fixing [ZF-9586] and from [ZF-5222], that interferencing SubForm Names and Segments of elementsBelongTo cause data loss, when using array_merge instead of array_merge_recursive, so i updated this issue to reflect that too.
And after Patch
public function getErrors($name = null, $suppressArrayNotation = false)
{
$errors = array();
if (null !== $name) {
if (isset($this->_elements[$name])) {
return $this->getElement($name)->getErrors();
} else if (isset($this->_subForms[$name])) {
return $this->getSubForm($name)->getErrors(null, true);
}
}
foreach ($this->_elements as $key => $element) {
$errors[$key] = $element->getErrors();
}
foreach ($this->getSubForms() as $key => $subForm) {
$merge = array();
if (!$subForm->isArray()) {
$merge[$key] = $subForm->getErrors();
} else {
$merge = $this->_attachToArray($subForm->getErrors(null, true),
$subForm->getElementsBelongTo());
}
$errors = array_merge_recursive($errors, $merge);
}
if (!$suppressArrayNotation && $this->isArray()) {
$errors = $this->_attachToArray($errors, $this->getElementsBelongTo());
}
return $errors;
}
The Test as a whole
public function _setup9350()
{
$this->form->addSubForm(new Zend_Form_SubForm(), 'foo')
->foo->setElementsBelongTo('foo[foo]') // foo[foo]
->addSubForm(new Zend_Form_SubForm(), 'foo') // foo[foo][foo]
->foo->setIsArray(false)
->addElement('text', 'foo') // foo[foo][foo][foo]
->foo->addValidator('Identical',
false,
array('foo Value'));
$this->form->foo->addSubForm(new Zend_Form_SubForm(), 'baz') // foo[foo][baz]
->baz->setIsArray(false)
->addSubForm(new Zend_Form_SubForm(), 'baz')
->baz->setElementsBelongTo('baz[baz]') // foo[foo][baz][baz][baz]
->addElement('text', 'baz') // foo[foo][baz][baz][baz][baz]
->baz->addValidator('Identical',
false,
array('baz Value'));
// This is appending a different named SubForm and setting
// elementsBelongTo to a !isArray() Subform name from same level
$this->form->foo->addSubForm(new Zend_Form_SubForm(), 'quo')
->quo->setElementsBelongTo('foo') // foo[foo][foo] !!!!
->addElement('text', 'quo') // foo[foo][foo][quo]
->quo->addValidator('Identical',
false,
array('quo Value'));
// This is setting elementsBelongTo point into the middle of
// a chain of another SubForms elementsBelongTo
$this->form->addSubForm(new Zend_Form_SubForm(), 'duh')
->duh->setElementsBelongTo('foo[zoo]') // foo[zoo] !!!!
->addElement('text', 'zoo') // foo[zoo][zoo]
->zoo->addValidator('Identical',
false,
array('zoo Value'));
// This is !isArray SubForms Name equal to the last segment
// of another SubForms elementsBelongTo
$this->form->addSubForm(new Zend_Form_SubForm(), 'iek')
->iek->setElementsBelongTo('foo') // foo !!!!
->addSubForm(new Zend_Form_SubForm(), 'zoo') // foo[zoo] !!!!
->zoo->setIsArray(false)
->addElement('text', 'iek') // foo[zoo][iek]
->iek->addValidator('Identical',
false,
array('iek Value'));
$data = array('valid' => array('foo' =>
array('foo' =>
array('foo' =>
array('foo' => 'foo Value',
'quo' => 'quo Value'),
'baz' =>
array('baz' =>
array('baz' =>
array('baz' => 'baz Value')))),
'zoo' =>
array('zoo' => 'zoo Value',
'iek' => 'iek Value'))),
'invalid' => array('foo' =>
array('foo' =>
array('foo' =>
array('foo' => 'foo Invalid',
'quo' => 'quo Value'),
'baz' =>
array('baz' =>
array('baz' =>
array('baz' => 'baz Value')))),
'zoo' =>
array('zoo' => 'zoo Value',
'iek' => 'iek Invalid'))),
'partial' => array('foo' =>
array('foo' =>
array('baz' =>
array('baz' =>
array('baz' =>
array('baz' => 'baz Value'))),
'foo' =>
array('quo' => 'quo Value')),
'zoo' =>
array('zoo' => 'zoo Value'))));
return $data;
}
public function testGetErrorsWithElementsBelongTo()
{
$data = $this->_setup9350();
$this->form->isValid($data['invalid']);
$errors = $this->form->getErrors();
$this->assertTrue(isset($errors['foo']['foo']['foo']['foo']));
$this->assertTrue(isset($errors['foo']['zoo']['iek']));
}
And finally the patch which fixes the issue, with Unit Test
Index: tests/Zend/Form/FormTest.php
===================================================================
--- tests/Zend/Form/FormTest.php (Revision 21732)
+++ tests/Zend/Form/FormTest.php (Arbeitskopie)
@@ -1579,7 +1621,17 @@
$this->assertSame($this->form->getValidValues($data['invalid']), $data['partial']);
}
+ public function testGetErrorsWithElementsBelongTo()
+ {
+ $data = $this->_setup9350();
+ $this->form->isValid($data['invalid']);
+ $errors = $this->form->getErrors();
+ $this->assertTrue(isset($errors['foo']['foo']['foo']['foo']));
+ $this->assertTrue(isset($errors['foo']['zoo']['iek']));
+ }
+
+
// Display groups
public function testCanAddAndRetrieveSingleDisplayGroups()
Index: library/Zend/Form.php
===================================================================
--- library/Zend/Form.php (Revision 21732)
+++ library/Zend/Form.php (Arbeitskopie)
@@ -2271,22 +2272,35 @@
* @param string $name
* @return array
*/
- public function getErrors($name = null)
+ public function getErrors($name = null, $suppressArrayNotation = false)
{
$errors = array();
- if ((null !== $name) && isset($this->_elements[$name])) {
- $errors = $this->getElement($name)->getErrors();
- } elseif ((null !== $name) && isset($this->_subForms[$name])) {
- $errors = $this->getSubForm($name)->getErrors();
- } else {
- foreach ($this->_elements as $key => $element) {
- $errors[$key] = $element->getErrors();
+ if (null !== $name) {
+ if (isset($this->_elements[$name])) {
+ return $this->getElement($name)->getErrors();
+ } else if (isset($this->_subForms[$name])) {
+ return $this->getSubForm($name)->getErrors(null, true);
}
- foreach ($this->getSubForms() as $key => $subForm) {
- $fErrors = $this->_attachToArray($subForm->getErrors(), $subForm->getElementsBelongTo());
- $errors = array_merge($errors, $fErrors);
+ }
+
+ foreach ($this->_elements as $key => $element) {
+ $errors[$key] = $element->getErrors();
+ }
+ foreach ($this->getSubForms() as $key => $subForm) {
+ $merge = array();
+ if (!$subForm->isArray()) {
+ $merge[$key] = $subForm->getErrors();
+ } else {
+ $merge = $this->_attachToArray($subForm->getErrors(null, true),
+ $subForm->getElementsBelongTo());
}
+ $errors = array_merge_recursive($errors, $merge);
}
+
+ if (!$suppressArrayNotation && $this->isArray()) {
+ $errors = $this->_attachToArray($errors, $this->getElementsBelongTo());
+ }
+
return $errors;
}
Comments
Posted by Christian Albrecht (alab) on 2010-04-01T04:31:09.000+0000
Ready for review and commit
Posted by Christian Albrecht (alab) on 2010-04-02T00:23:14.000+0000
Updated UnitTest
Posted by Matthew Weier O'Phinney (matthew) on 2010-04-16T12:46:13.000+0000
Patch applied to trunk and 1.10 release branch.