Zend Framework

Zend_Form getErrors() incorrect handling when !$subform->isArray()

Details

  • Type: Sub-task Sub-task
  • Status: Resolved Resolved
  • Priority: Major Major
  • Resolution: Fixed
  • Affects Version/s: None
  • Fix Version/s: 1.10.4
  • Component/s: Zend_Form
  • Labels:
    None

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;
     }

Issue Links

This issue is related to:
ZF-5222 Zend_Form::getValues() mishandles SubForms data when elementsBelongTo is set Major Resolved
ZF-3917 Zend_Form_Element::isValid() losing error messages with arrays Major Resolved

Activity

Ascending order - Click to sort in descending order
Hide
Permalink
Christian Albrecht added a comment -

Ready for review and commit

Show
Christian Albrecht added a comment - Ready for review and commit
Hide
Permalink
Christian Albrecht added a comment -

Updated UnitTest

Show
Christian Albrecht added a comment - Updated UnitTest
Hide
Permalink
Matthew Weier O'Phinney added a comment -

Patch applied to trunk and 1.10 release branch.

Show
Matthew Weier O'Phinney added a comment - Patch applied to trunk and 1.10 release branch.

People

Vote (0)
Watch (0)

Dates

  • Created:
    Updated:
    Resolved:
Atlassian JIRA (v4.4.3#663-r165197) | Report a problem
Powered by a free Atlassian JIRA open source license for Zend Framework. Try JIRA - bug tracking software for your team.