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

CollectionInputFilter should respect the keys of collectionData #4516

Closed
wants to merge 1 commit into from
Closed

CollectionInputFilter should respect the keys of collectionData #4516

wants to merge 1 commit into from

Conversation

zluiten
Copy link
Contributor

@zluiten zluiten commented May 21, 2013

Currently the CollectionInputFilter assumes that collectionData has always consecutive array keys which can be wrong.

Two changes

  • Iterating over collectionData instead of a composed inputCollection
  • When there are less items in collectionData than $this->count is set to, just mark the collection as invalid, since it doesn't make sense to add them to invalidInputs when there is no corresponding set of data.

@weierophinney
Copy link
Member

@davidwindell - As the CollectionInputFilter was your addition, mind weighing in?

@Netiul If David approves, I would consider this a bugfix; as such, do you mind if I cherry pick it into master for 2.2.1? Authorship is retained; the history changes, though, so you'll have to update your branch after I merge.

@davidwindell
Copy link
Contributor

@weierophinney sure thing, I'll run this through our app tomorrow which has additional extensive tests involving the component and provide any feedback.

@zluiten
Copy link
Contributor Author

zluiten commented May 23, 2013

@weierophinney, no problem

@davidwindell
Copy link
Contributor

Looks good to me 👍 thanks @Netiul

@zluiten
Copy link
Contributor Author

zluiten commented May 24, 2013

Thanks for reviewing @davidwindell

@ghost ghost assigned weierophinney May 24, 2013
weierophinney added a commit that referenced this pull request May 24, 2013
@zluiten zluiten deleted the hotfix/CollectionInputFilter branch May 24, 2013 19:05
weierophinney added a commit to zendframework/zend-inputfilter that referenced this pull request May 15, 2015
weierophinney added a commit to zendframework/zend-inputfilter that referenced this pull request May 15, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants