Form::getMessages() on plain array collection - fix for issue #135

[vaclavvanik]

I used this fix. If anyone has better idea let me know!

[froschdesign]

Please test here the Fieldset not the Form class or the Collectionelement.

[froschdesign]

What is bar and why do you test it?

[froschdesign]

I think, the class is not needed. Please have a look at ZendTest\Form\Element\CollectionTest.

[vaclavvanik]

@froschdesign I think about the test again and again. Basically this patch allows storing messages for elements which are not founds. It does not break any other test. I would rewrite test to:

    public function testSetAndGetErrorMessagesForNonExistentElements()
    {
        $messages = [
            'foo' => [
                'foo_message_key' => 'foo_message_val',
            ],
            'bar' => [
                'bar_message_key' => 'bar_message_val',
            ],
        ];

        $fieldset = new Fieldset();
        $fieldset->setMessages($messages);

        $this->assertEquals($messages, $fieldset->getMessages());
    }

agreed?

[froschdesign]

@vaclavvanik
I have written one more test.

This works with your changes:

// ZendTest\Form\Element\CollectionTest

public function testGetErrorMessagesForInvalidCollectionElements()
{
    // Configure InputFilter
    $inputFilter = $this->form->getInputFilter();
    $inputFilter->add(
        [
            'name'     => 'colors',
            'type'     => ArrayInput::class,
            'required' => true,
        ]
    );
    $inputFilter->add(
        [
            'name'     => 'fieldsets',
            'type'     => ArrayInput::class,
            'required' => true,
        ]
    );


    $this->form->setData([]);
    $this->form->isValid();

    $this->assertEquals(
        [
            'colors'     => [
                'isEmpty' => "Value is required and can't be empty",
            ],
            'fieldsets' => [
                'isEmpty' => "Value is required and can't be empty",
            ],
        ],
        $this->form->getMessages()
    );
}

[vaclavvanik]

First look at testValidationForInvalidChildElements() => its problem in InputFilter.

Code from Form::isValid()

        $this->isValid = $result = $filter->isValid();
        ...
        if (!$result) {
            $this->setMessages($filter->getMessages());
        }

Error messages are set only when validation fails. This test is imho unrelated to my patch.

[froschdesign]

@vaclavvanik

First look at testValidationForInvalidChildElements() => its problem in InputFilter.

Right, the InputFilter was wrong.

My original intention: we need another test for the Collection element. (based on ZendTest\Form\TestAsset\FormCollection)

• both Collection elements are required
• all target elements are required

All error messages received?

[froschdesign]

@vaclavvanik
👍

[vaclavvanik]

@froschdesign tests updated. I hope I did not miss anything.

[vaclavvanik]

@froschdesign can you merge if don't have any other ideas?

[akrabat]

Why is this change on the Fieldset class and not the Collection class?

[froschdesign]

@akrabat
The Fieldset class resets all previous added messages. Look at this unit test: https://github.com/vaclavvanik/zend-form/blob/b5c442dbab50d9817e3c965aaf083c6b6bf3fed0/test/FieldsetTest.php#L579

[akrabat]

I mean, why isn't this fixed as new methods setMessages and getMessages on Collection?

i.e. does this bug affect Fieldset as well as Collection?

[froschdesign]

i.e. does this bug affect Fieldset as well as Collection?

Right!

[akrabat]

i.e. does this bug affect Fieldset as well as Collection?
Right!

That's a "yes it does" I assume. Next question. Will this have any side-effects for people using normal elements inside Fieldsets?

[froschdesign]

@akrabat
Sorry, for the late response.

Will this have any side-effects for people using normal elements inside Fieldsets?

No and the unit tests for these scenarios says the same.

[weierophinney]

Thanks, @vaclavvanik