Story #3714
Add advanced validation of ViewHelper arguments
| Status: | Rejected | Start date: | 2009-06-21 | |
|---|---|---|---|---|
| Priority: | Could have | Due date: | ||
| Assigned To: | Christian Müller | % Done: | 100% |
|
| Category: | Core | |||
| Target version: | - | |||
| Has patch: |
Description
We should add a feature to support the @validate annotation inside the ViewHelper render() method comment.
See the ActionController for the code which needs to be included in AbstractViewHelper, and make sure to refactor it to a central place.
History
#1 Updated by Sebastian Kurfuerst about 6 years ago
- Assigned To set to Christian Müller
#2 Updated by Sebastian Kurfuerst about 6 years ago
- Status changed from New to Accepted
#3 Updated by Christian Müller about 6 years ago
- File fluid_3714_viewhelper_validation.diff
added
The attached diff changes some files (mostly extBase) and tries to put most of the code into central place.
#4 Updated by Christian Müller about 6 years ago
- Status changed from Accepted to Needs Feedback
As this is my first bigger patch, would be great if someone could look it over and comment...
#5 Updated by Sebastian Kurfuerst about 6 years ago
Hey Christian,
first of all - thanks for your nice work :-)
I have some comments:
- in the ValidatorResolver you introduced a new method "initializeMethodValidators" - I would rather remove this again, as it is quite difficult to gasp, and you expect a certain callback method which needs to be there but is never documented explicitly.
Rather, I would instead loop through the $validatorConjunctions by hand inside the AbstractViewHelper.
Concerning general development practice (I should have said this earlier, sorry!): All changes in Fluid core must be made in FLOW3 (including unit tests) and then Fluid Core is automatically backported to TYPO3 v4.
Do you have a FLOW3 installation at hand? If yes, it would be really awesome if you could port the code with the above comments to fluid v5 and cover it with a unit test. Then, we can backport it automatically using the "Backporter" scripts.
If there are any questions, don't hesitate to contact me!
Thanks again for your work and greets,
Sebastian
#6 Updated by Christian Müller about 6 years ago
- Status changed from Needs Feedback to Accepted
I am working on FLOW3 port an cleanup, patch next week.
#7 Updated by Christian Müller about 6 years ago
- File fluid_3714_viewhelper_validation2.diff added
- % Done changed from 0 to 50
New patch for FLOW3, just with the change to the AbstractViewHelper and a call to it in ViewHelperNode, but I am not sure if it is called at the right point. Testcase follows with additional patch.
#8 Updated by Christian Müller about 6 years ago
- Status changed from Accepted to Resolved
- % Done changed from 50 to 100
Patch v2 working good with some basic testing. Think it could be integrated, what do I have to do now with the patch?
#9 Updated by Sebastian Kurfuerst about 6 years ago
- Status changed from Resolved to Needs Feedback
Hey Christian,
I just had a quick look at the patch, and I still have two questions:
1) How can initializeRenderMethodValidators() be protected if you call it from AbstractViewHelper?
2) I did not find the unit test covering it :-) Did you forget to update it, or do you need some help in writing it?
Greets and thanks a bunch for your work,
Sebastian
#10 Updated by Christian Müller about 6 years ago
- File fluid_3714_viewhelper_validation3.diff added
So sorry, this is a mix up. Of course it can't be protected, the v3 patch changes this to public as it is on my other machine ;-)
For the unit test, well then there is a misunderstanding, because I thought that this is already covered by another test. No problem, will look into the tests again and ask for help if needed (possibly is).
#11 Updated by Robert Lemke about 6 years ago
- Target version deleted (
1.0)
#12 Updated by Sebastian Kurfuerst over 5 years ago
- Priority changed from Must have to Could have
#13 Updated by Sebastian Kurfuerst about 4 years ago
- Tracker changed from Feature to Story
- Status changed from Needs Feedback to Rejected