Feature/unfiltered input data #176
Conversation
|
And how would I use this? as I understand I could override |
|
As what was attempted with the previous setup, it was to make all of the unfiltered data available, not to already implement it in existing functionality. That's why this only adds the property on the InputFilter class and sets it. That makes it available for each to implement/use it at will. To make it implemented by default, e.g. in the |
There was a problem hiding this comment.
This looks good. I've made a number of comments, but I can address them (and push them back to your branch) during merge. I'll also document the new interface and method when I do.
Thanks, @rkeet!
.gitignore
Outdated
| @@ -5,3 +5,4 @@ clover.xml | |||
| coveralls-upload.json | |||
| phpunit.xml | |||
| zf-mkdoc-theme.tgz | |||
| .idea/ | |||
There was a problem hiding this comment.
Please remove this; this should be in your global gitignore file.
| */ | ||
| // TODO replace functions when upgrading to > PHP 7.2 as minimum requirement | ||
| // public function getUnfilteredData() : array; | ||
| public function getUnfilteredData() |
There was a problem hiding this comment.
TODO items should be done as annotations:
@todo Replace signature ...That said: this one is unnecessary; when we move to 7.1 or 7.2 as the minimum supported version, we will be adding return type hints and scalar type hints everywhere we can, as that's a primary motivation for adopting the newer PHP version.
| */ | ||
| // TODO replace functions when upgrading to > PHP 7.2 as minimum requirement | ||
| // public function setUnfilteredData(array $data) : UnfilteredDataInterface; | ||
| public function setUnfilteredData($data) |
| * @link http://github.com/zendframework/zf2 for the canonical source repository | ||
| * @copyright Copyright (c) 2005-2019 Zend Technologies USA Inc. (http://www.zend.com) | ||
| * @license http://framework.zend.com/license/new-bsd New BSD License | ||
| */ |
There was a problem hiding this comment.
-
Use the short-version of this annotation, please (can omit the summary line and the blank line following).
-
Use https links, please, and link to the
LICENSE.mdin the repository itself. -
Since this is a new file, only list 2019 for the copyright date.
src/UnfilteredDataInterface.php
Outdated
|
|
||
| // TODO replace functions when upgrading to > PHP 7.2 as minimum requirement | ||
| // public function getUnfilteredData() : array; | ||
| // public function setUnfilteredData(array $data) : UnfilteredDataInterface; |
There was a problem hiding this comment.
Remove this comment please.
…implements UnfilteredDataInterface.
…nd TODO for when PHP 7.2 is minimum requirement.
…ter for applying UnfilteredDataInterface on BaseInputFilter.
…Interface in BaseInputFilter.
…unfiltered data in BaseInputFilter
…r. Added property on BaseInputFilter - unfilteredData. Tests pass again. Changed setter return type for PHP 7.2 todo's from array to UnfilteredDataInterface.
… returning same via getUnfiteredData
…getUnfilteredData) - works out-of-the-box
…result for getValues and getRawValues as opposed to getUnfilteredData.
… the same after running isValid method.
Should be in global gitignore, not project
000ba4c to
2fc2d79
Compare
|
Thanks, @rkeet! |
Based on old PR (deprecated) discussion here and Issue description here
Link to last comment with action points (copied below)
@froschdesign @svycka @weierophinney
I haven't forgotten this, just didn't have time this weekend. I hope to get to it this week either during work, an evening or maybe during the coming weekend. However, very busy at the moment.
What I'll try to do the coming days:
Setup up a skeleton application demonstrating issue with plain modules, comments, etc.
Summarize this thread & #168 into a coherent whole
Quick re-read though:
getRawValue(s)() should not be altered from existing functionality to prevent bc-break
Agreed with that getRawValue(s)() returns only known input values, UnfilteredDataInterface should provide solution next to these functions
UnfilteredDataInterface adds functionality instead of modifying existing ones
I'll get into that more by providing the code I mentioned with unit testing - I hope this can wait a few more days (possibly weeks). Like I said, currently very busy, also next to my job. Though holidays soon, so might get something done then...
So I think this might be it
I've done this one TDD (pats own back). I'm however running stretched for time, so I'm hoping to get some feedback for now.
Also, starting a new job next month in Symfony development (my own projects run ZF3/Doctrine), so as I'll have to do some study to get up to speed, ZF3 contributions will regrettably have to go on a back-burner.