Skip to content
This repository has been archived by the owner on Jan 8, 2020. It is now read-only.

BaseInputFilter not populating InputFilters of Element\Collection #4294

Closed
danizord opened this issue Apr 22, 2013 · 17 comments
Closed

BaseInputFilter not populating InputFilters of Element\Collection #4294

danizord opened this issue Apr 22, 2013 · 17 comments
Assignees
Milestone

Comments

@danizord
Copy link
Contributor

I realized this after #4231 be merged, so my forms began to be invalid.
Zend\InputFilter\BaseInputFilter::populate() is not populating values to the InputFilters of the Element\Collection's children.

Sorry for the short description, I'm very busy now.
Tell me if that isn't enough, then I'll provide an example later.

@weierophinney
Copy link
Member

@danizord A short reproduce case would help me out a ton here -- then I know for sure what needs to be fixed.

@ghost ghost assigned weierophinney Apr 22, 2013
@danizord
Copy link
Contributor Author

@weierophinney Here's the reproduce case:

$myFieldset = new Fieldset;
$myFieldset->add([
    'name' => 'email',
    'type' => 'email', // Zend\Form\Element\Email implements InputProviderInterface
]);

$myForm = new Form;
$myForm->add([
    'name' => 'collection',
    'type' => 'collection',
    'options' => [
        'target_element' => $myFieldset,
    ],
]);

$data = [
    'collection' => [
        ['email' => 'test1@test1.com'],
        ['email' => 'test2@test2.com'],
        ['email' => 'test3@test3.com'],
    ]
];

$myForm->setData($data);

var_dump($myForm->isValid()); //False
var_dump($myForm->getMessages()); //Empty array

Before #4231 $myForm->isValid() was return true.

@bakura10
Copy link
Contributor

As a side note: 'type' => 'email' aliases to Zend\Form\Element\Email because of the FormElementManager (that was merged in 2.1), so all the elements are required. If you want an empty email field without the filter/validation mechanism provided by the element, you must do this:

$myFieldset->add([
    'name' => 'email',
    'type' => 'text',
    'attributes' => array('type' => 'email'))
]);

So getMessages returning empty is logical, but why isValid returns false... this is strange :/. Those Collections are driving me crazy lol. @davidwindell any idea ?

@danizord
Copy link
Contributor Author

@bakura10 I know this. I purposely used the Zend\Form\Element\Email because it provides a input specification.

I checked the whole process and the $myForm->setData() is setting values correctly in all elements, the problem here is at the time of setting the data in the input filters. (In BaseInputFilter::populate())

@bakura10
Copy link
Contributor

Mmhhh... This is really annoying if this is the case... :/. Can't you find the reason ? :(

@danizord
Copy link
Contributor Author

@bakura10 I checked again and realized that before the merge of #4231 everything was working like a charm.
$myForm->isValid(); returning true and all values filtered ​​and validated.

@davidwindell I'm not sure if I got the intention of #4231. Can you take a help here?

@davidwindell
Copy link
Contributor

I'm on holiday until Monday (29th) so I can't help until then. I gave it a quick test and by adding a validation group the form returns valid. Hopefully that will help someone trace this one down.

@danizord
Copy link
Contributor Author

@davidwindell 29th will be the code-freeze day for 2.2 release. We need to fix this before 29th.

If nobody find a solution for this, can I revert #4231?

@davidwindell
Copy link
Contributor

I wouldn't revert that as there's a security implication to doing so.
On 25 Apr 2013 23:29, "Daniel Gimenes" notifications@github.com wrote:

@davidwindell https://github.com/davidwindell 29th will be the *
code-freeze* day for 2.2 release. We need to fix this before 29th.

If nobody find a solution for this, can I revert #4231#4231
?


Reply to this email directly or view it on GitHubhttps://github.com//issues/4294#issuecomment-17044286
.

@danizord
Copy link
Contributor Author

@davidwindell But it stopped working =/

@bakura10
Copy link
Contributor

Will reproduce the bug today to solve it.

Envoyé de mon iPhone

Le 26 avr. 2013 à 00:41, Daniel Gimenes notifications@github.com a écrit :

@davidwindell But it stopped working =/


Reply to this email directly or view it on GitHub.

@bakura10
Copy link
Contributor

I tried to track the bug. The reason is that the input filter is not created correctly, hence failing. I'm not sure to still understand your CollectionInputFitler @davidwindell , I don't know why it's not used for such cases (collection).

There was indeed something wrong in your logic @davidwindell here: https://github.com/zendframework/zf2/pull/4231/files#L0L700 because it returns only one element instead of how many there are really in the collection. Trying to find a fix but if I can't we are going to need to revert your fix, it definitely broke something.

The code base of Form has become really messy... Hard to find where the error is...

@davidwindell
Copy link
Contributor

The CollectionInputFilter could be used with a Form, it's not directly related to the Element\Collection. The Collection term refers to a collection of data not the element.

Good luck finding the fix! If it's reverted we would still need to find a way to apply the filter defaults for collection elements.

@bakura10
Copy link
Contributor

@davidwindell , before your fix, this is what the input filter looks like: http://cl.ly/image/2p0j441R201U

After your fix: http://cl.ly/image/221j2E0O0308

Notice the email element, this is what makes fail.

@davidwindell
Copy link
Contributor

I see, so the fix will be changing that new code that it loops the collections fieldsets x number of times and then applies the defaults

@zluiten
Copy link
Contributor

zluiten commented Apr 26, 2013

Looking into it too, since I really don't want it to be reverted!

This was referenced Apr 26, 2013
weierophinney added a commit that referenced this issue Apr 26, 2013
@weierophinney
Copy link
Member

Fixed with #4328

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants