Collection Input Filter fix messages #5968
Conversation
|
Hey @fabiocarneiro, can you provide tests that back this new logic? I also expect failures from the current test suite, right? |
|
@Ocramius I'll work on that. The only problem is that the messages were not working. But since i had to change the behavior of that, i went further. |
| @@ -80,7 +80,6 @@ public function setInputFilter($inputFilter) | |||
| } | |||
|
|
|||
| $this->inputFilter = $inputFilter; | |||
| $this->inputs = $inputFilter->getInputs(); | |||
There was a problem hiding this comment.
This change breaks line 172?
There was a problem hiding this comment.
@fabiocarneiro Can you comment on this? I think @danizord raises a valid point...
There was a problem hiding this comment.
@weierophinney, He was talking about the old line 172, already removed. but since this line 83 was not modified, his note wasn't outdated.
|
@fabiocarneiro It seems that you're introducing a minor behavior change. So, you should probably reopen this PR against develop. |
|
Also, @fabiocarneiro. I realised that you moved a lot of logic from Having said that, I think you can also remove the |
| @@ -109,17 +109,6 @@ public function testSetInputFilter() | |||
| $this->assertInstanceOf('Zend\InputFilter\BaseInputFilter', $this->filter->getInputFilter()); | |||
There was a problem hiding this comment.
Please restore the chmod of this file
| @@ -223,22 +201,18 @@ public function isValid() | |||
| /** | |||
| * {@inheritdoc} | |||
| */ | |||
| public function setValidationGroup($name) | |||
| public function setValidationGroup($groups) | |||
There was a problem hiding this comment.
Please revert the change. It's better to have the same name on parent and childs
…Input instead of a InputFilter
| @@ -176,45 +175,24 @@ public function isValid() | |||
| return $valid; | |||
| } | |||
|
|
|||
| $inputs = $this->validationGroup ?: array_keys($this->inputs); | |||
| $inputs = $this->validationGroup ?: array_keys($this->inputFilter->getInputs()); | |||
| /* | ||
| * @var array | ||
| */ | ||
| protected $collectionInvalidInputs; | ||
|
|
|
|
||
| if ($this->validateInputs($inputs, $data)) { | ||
| $this->collectionValidInputs[$key] = $this->validInputs; | ||
| if ($this->inputFilter->isValid()) { |
| @@ -300,4 +250,5 @@ public function getMessages() | |||
| { | |||
| return $this->collectionMessages; | |||
| } | |||
|
|
|||
|
@Maks3w Can you review this one? Tell me what must be done to get it merged. |
|
Will someone review this ? I need it in my project. |
|
@fabiocarneiro I'll review after you answer the open question above. |
…ilter-messages Collection Input Filter fix messages
…o/hotfix-collection-input-filter-messages Collection Input Filter fix messages
All CollectionInputFilter messages are returning as a empty arrays instead of an array of messages.
Also modified the class behavior. Previously it was copying the inputs from its Inputfilter and validating the InputFilter in CollectionInputFilter, which is not necessary since it can already be done in the InputFilter itself.
To make sure this is necessary, I updated testGetDefaultInputFilter, so it can fail when there is a error in the nested CollectionInputFilter. It is failing in current master, but it will work with this hotfix.
The CollectionInputFilter should only get each element data and setting it to the InputFilter it contains, get back the validation and messages and return in its own methods.
Special thanks to @danizord, who is always helping me with those issues.