Issues

ZF-11402: Zend_Form_Element_MultiCheckbox does not accept empty array even when AllowEmpty flag is set

Issue Type: Bug Created: 2011-05-23T15:20:13.000+0000 Last Updated: 2011-11-04T00:15:35.000+0000 Status: Resolved Fix version(s): - 1.11.8 (07/Jul/11)

  • 1.11.12 (22/Jun/12)

Reporter: Olivier (oliviercuyp@hotmail.com) Assignee: Adam Lundrigan (adamlundrigan) Tags: - Zend_Form

Related issues: Attachments: - ZF-11402.patch

Description

Here is the code with the validator :

'New York', 2 => 'Los Angeles' 3 => 'Chicago' ); $element = new Zend\_Form\_Element\_MultiCheckbox('cities'); $element->setLabel('Cities') ->setMultiOptions($options) ->setRequired(true) ->addValidators(array( new Zend\_Validate\_NotEmpty(Zend\_Validate\_NotEmpty::PHP) )); $this->addElement($element); ... ?> Then I test it like this: 'someone', 'email' => 'someone@somewhere.com', 'cities' => array() ); Zend\_Debug::dump($form->isValid($user)); exit(); // return (bool) true ?> I would have expected the $form->isValid($user) to return false ...

In my real code I use a form to validate my model before saving it. The user model has an empty array as default value if data relationship is enable in my mapper other null. My user model should have at least 1 city but I can't check it with the form validation when data relationship is enable...

Comments

Posted by Adam Lundrigan (adamlundrigan) on 2011-05-24T01:29:54.000+0000

This is not an issue with Zend_Validate_NotEmpty, as it rejects empty array properly, as evidenced by this test:

<pre class="highlight">
/**
 * @group ZF-11402
 */
public function testPHPRejectsEmptyArray()
{
    $validator = new Zend_Validate_NotEmpty(
        Zend_Validate_NotEmpty::PHP
    );
    $this->assertFalse($validator->isValid(array()));        
}

And the result:

<pre class="highlight">
++ phpunit --verbose --group ZF-11402 AllTests
PHPUnit 3.5.13 by Sebastian Bergmann.
OK (1 test, 1 assertion)

I would check the documentation for Zend_Form_Element_MultiCheckbox to determine how to properly enforce the condition that one or more of the checkboxes in the group must be selected.

Posted by Olivier (oliviercuyp@hotmail.com) on 2011-05-24T11:40:45.000+0000

Hi Adam,

Sorry about this, I miss placed the issue. It is not on the Zend_Validator_NotEmpty element but on the Zend_Element_Multi. Correct me if I'm wrong but the validator for Zend_Element_Multi the validation only happens on each individudal value but not set of values itself if it is an array. Believe me, I have red the documentation but I couldn't find any way (expect ugly hacks) to validate the number of elements of a Zend_Element_Multi. I even looked in the code of Zend_Element_Multi and Zend_Element and to my unberstanding, today it is impossible to validate the number of values recieved.

Here is the code of the validation for Zend_Element (nothing interesting in Zend_Element_Multi just adding InArray) :

public function isValid($value, $context = null) { ... foreach ($this->getValidators() as $key => $validator) { ... if ($isArray && is_array($value)) { $messages = array(); $errors = array(); foreach ($value as $val) { if (!$validator->isValid($val, $context)) { $result = false; if ($this->_hasErrorMessages()) { $messages = $this->_getErrorMessages(); $errors = $messages; } else { $messages = array_merge($messages, $validator->getMessages()); $errors = array_merge($errors, $validator->getErrors()); } } } if ($result) { continue; } } .. } ... }

Validation only validate single value not value set so if you give a empty array it will still pass the validation. Not alternative method given by the documentation ...

Posted by Adam Lundrigan (adamlundrigan) on 2011-05-31T13:14:26.000+0000

Olivier,

Ah, I see the issue now. I can reproduce it with this code:

<pre class="highlight">
/**
 * @group ZF-11402
 */
public function testValidateShouldNotAcceptEmptyArray()
{
    $this->element->addMultiOptions(array(
        'foo' => 'Foo',
        'bar' => 'Bar',
        'baz' => 'Baz',
    ));
    $this->element->setRegisterInArrayValidator(true);

    $this->assertTrue($this->element->isValid(array('foo')));
    $this->assertTrue($this->element->isValid(array('foo','baz')));

    $this->element->setAllowEmpty(true);
    $this->assertTrue($this->element->isValid(array()));

    $this->element->setAllowEmpty(false);
    $this->assertFalse($this->element->isValid(array()));
}

This results in:

<pre class="highlight">
++ phpunit --verbose --group Zend_Form AllTests

There was 1 failure:

1) Zend_Form_Element_MultiCheckboxTest::testValidateShouldNotAcceptEmptyArray
Failed asserting that  is false.

/usr/local/apache2/htdocs/lib/zfdev/trunk/tests/Zend/Form/Element/MultiCheckboxTest.php:301

The last assert in my test - where isValid(array()) is tested - fails. When I make the following change to Zend_Form_Element:

<pre class="highlight">
Index: library/Zend/Form/Element.php
===================================================================
--- library/Zend/Form/Element.php       (revision 24092)
+++ library/Zend/Form/Element.php       (working copy)
@@ -1377,15 +1377,23 @@
             if ($isArray && is_array($value)) {
                 $messages = array();
                 $errors   = array();
-                foreach ($value as $val) {
-                    if (!$validator->isValid($val, $context)) {
+                if (empty($value)) {
+                    if ($this->isRequired()
+                        || (!$this->isRequired() && !$this->getAllowEmpty())
+                    ) {
                         $result = false;
-                        if ($this->_hasErrorMessages()) {
-                            $messages = $this->_getErrorMessages();
-                            $errors   = $messages;
-                        } else {
-                            $messages = array_merge($messages, $validator->getMessages());
-                            $errors   = array_merge($errors,   $validator->getErrors());
+                    }
+                } else {
+                    foreach ($value as $val) {
+                        if (!$validator->isValid($val, $context)) {
+                            $result = false;
+                            if ($this->_hasErrorMessages()) {
+                                $messages = $this->_getErrorMessages();
+                                $errors   = $messages;
+                            } else {
+                                $messages = array_merge($messages, $validator->getMessages());
+                                $errors   = array_merge($errors,   $validator->getErrors());
+                            }
                         }
                     }
                 }

The heart of the change is this:

<pre class="highlight">
if (empty($value)) {
    if ($this->isRequired()
        || (!$this->isRequired() && !$this->getAllowEmpty())
    ) {
        $result = false;
    }
}

Which sets the result to false if the value is empty and the field is either required or is not required and does not allow empty values. The latter is done to maintain the fact that Required supercedes AllowEmpty, as by defition marking a field as required does not allow it to be empty.

After making the proposed change to Zend_Form_Element, all unit tests in Zend_Form group (including my test testValidateShouldNotAcceptEmptyArray()) now pass.

I've attached the full patch to this issue.

This, however, only handles the empty case. I haven't looked at your other question on how to validate the number of values provided (ie: X values must be provided)

Posted by Olivier (oliviercuyp@hotmail.com) on 2011-05-31T19:44:00.000+0000

Adam,

Thanks for the patch. For the x values required validation, I just feel that it should happen in Zend_Element_Multi and not in Zend_Element. I guess other validation could be useful at this higher level like SumGreaterThan, SumLessThan, Average,... It would be nice to have some room for specific validation on Zend_Element_Multi.

Posted by Thomas Weidner (thomas) on 2011-06-26T11:26:34.000+0000

Assigned Zend_Form as component as requested Removed Zend_Validate as component Reassigned maintainer

Posted by Matthew Weier O'Phinney (matthew) on 2011-07-05T16:05:04.000+0000

Reviewed and applied patches from Adam to trunk and 1.11 release branch.

Posted by Justis Durkee (justis.durkee) on 2011-09-28T22:00:42.000+0000

What about the validation messages? isValid will return false, but there will be no error message(s) associated with the element.

Since the NotEmpty validator is being added to validation stack when the element value is required, we can allow it to run on the known empty value and thus create the error messages.

Library File:

<pre class="highlight">
Index: library/Zend/Form/Element.php
===================================================================
--- library/Zend/Form/Element.php   (revision 24480)
+++ library/Zend/Form/Element.php   (working copy)
@@ -1377,13 +1379,13 @@
             if ($isArray && is_array($value)) {
                 $messages = array();
                 $errors   = array();
+
+                // If the array is empty, then throw an empty element in the
+                // values array to allow the validators to run their course.
                 if (empty($value)) {
-                    if ($this->isRequired()
-                        || (!$this->isRequired() && !$this->getAllowEmpty())
-                    ) {
-                        $result = false;
+                    array_push($value, '');
                     }
-                } else {
+
                     foreach ($value as $val) {
                         if (!$validator->isValid($val, $context)) {
                             $result = false;

Unit Test:

<pre class="highlight">
Index: tests/Zend/Form/Element/MultiCheckboxTest.php
===================================================================
--- tests/Zend/Form/Element/MultiCheckboxTest.php   (revision 24480)
+++ tests/Zend/Form/Element/MultiCheckboxTest.php   (working copy)
@@ -299,6 +299,15 @@
     
         $this->element->setAllowEmpty(false);
         $this->assertFalse($this->element->isValid(array()));
+
+        $errors = $this->element->getErrors();
+        $this->assertArrayHasKey(0, $errors);
+        $this->assertEquals(1, count($errors));
+
+        $messages = $this->element->getMessages();
+        $this->assertArrayHasKey($errors[0], $messages);
+        $this->assertEquals(1, count($messages));
+        $this->assert($this->element->get
     }
 }

Posted by Adam Lundrigan (adamlundrigan) on 2011-10-19T23:10:55.000+0000

Justis: Thanks for pointing that out! I've taken your suggestion and implemented a revised version of my original fix which allows the validators to run so that the error messages come through properly. Could you try it out and let me know if it fixes the problem you raised?

Posted by Justis Durkee (justis.durkee) on 2011-10-24T18:49:15.000+0000

Adam,

Yes, your most recent patch fixes the issue. Thank you.

Posted by Adam Lundrigan (adamlundrigan) on 2011-11-03T23:48:20.000+0000

Additional fix committed to trunk in r24552 Merged to release-1.11 in r24553

Posted by Adam Lundrigan (adamlundrigan) on 2011-11-04T00:15:13.000+0000

ZF2 Pull Request: https://github.com/zendframework/zf2/pull/562

Have you found an issue?

See the Overview section for more details.

Copyright

© 2006-2016 by Zend, a Rogue Wave Company. Made with by awesome contributors.

This website is built using zend-expressive and it runs on PHP 7.

Contacts