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

Handle regular form elements as target elements of Zend\Form\Element\Collection. #6298

Closed
wants to merge 3 commits into from

Conversation

erispre
Copy link

@erispre erispre commented May 20, 2014

The changes below allow for setting regular form elements (i.e. not a Zend\Form\Fieldset) as the target element of a form collection. Before this patch, this would create problems when binding a domain model. Now, a new input (Zend\InputFilter\CollectionInput -- basically a ArrayInput with some minor changes to message handling) is set for the collection when no input filter configuration is provided. This input will take the validators and filters of the target element and loop over an array of values. This way, domain models can be hydrated from form data. (Before this, there would be no inputs nor input filters attached to the Collection's default input filter, so all values would be lost after Form::isValid() was called).

This pull request addresses issue #6263.

weierophinney and others added 3 commits May 12, 2014 17:11
Fielset, then add a CollectionInput instead of a regular input filter,
respecting the target's input specification.

Added Zend\InputFilter\CollectionInput and associated test case.

Added tests to ZendTest/Form/Element/CollectionTest
@Ocramius Ocramius added Form and removed enhancement labels May 20, 2014
}
return $this->messages;
}
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

EOF EOL

@SteveTalbot
Copy link

I don't think you need to create a new class \Zend\InputFilter\CollectionInput as there is already a Zend\InputFilter\CollectionInputFilter which can handle this for you.

Have a look at #6497 and #6498 for a way of instantiating a CollectionInputFilter automatically when a collection is encountered.

@weierophinney
Copy link
Member

@erispre Does the suggestion by @SteveTalbot address the use case you have? If so, we should close this. If not, can you explain how the use cases differ, so we can message when to use which type?

@adamlundrigan
Copy link
Contributor

@erispre #6880 should fix this issue, could you pull it down and try it out? (https://github.com/adamlundrigan/zf2/tree/zf6518)

@Ocramius
Copy link
Member

#6880 was merged, so this just needs to be verified.

@Ocramius
Copy link
Member

I'll just merge the tests from this patch.

@Ocramius Ocramius self-assigned this Dec 17, 2014
@Ocramius Ocramius modified the milestones: 2.4.0, 2.3.4 Dec 17, 2014
Ocramius added a commit that referenced this pull request Dec 28, 2014
Note that attribution will be given only as a comment to the commit, since rebasing/squashing
the commits to avoid changes to unaffected code-paths is problematic.

If this doesn't match expectations, then please ask for a revert and send the test code in a separate
pull request.
Ocramius added a commit that referenced this pull request Dec 28, 2014
@Ocramius Ocramius closed this in 4b496e9 Dec 28, 2014
@Ocramius
Copy link
Member

@erispre I've manually merged the newly introduced test case into master and develop. Note that the git commit is not having you as an author (because of the complexity in rebasing/squashing it to make it look like that), so if you have any problems with that please ask for a revert and send a new PR with just the test.

Thanks!

master: 4b496e9
develop: f352b36

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

Successfully merging this pull request may close these issues.

5 participants