Closed ahukkanen closed 1 year ago
:+1: It's so interesting
Do you have examples of other framework implementations of this? We might just want to implement something like this ourselves.
@KorvinSzanto There are a lot of implementations of similar concepts in different frameworks but with a slightly different mindset. This is mostly because the active record pattern is generally far more popular in these frameworks than the repository pattern provided by Doctrine (2).
To name a few popular ones:
I think in a problem scope as big as this one, it would be much easier to rely on existing implementations than bundle together something own. I think Symfony would be the natural choise for c5, it using already quite a few components from it.
The underlying benefit is much faster development process in my opinion. Building dashboard pages is quite a lot building different forms. This would allow us to automatically bundle those forms together just by passing in the object and the fields we want the user to fill in. The validations can also directly be defined for the entities as constraints: http://symfony.com/doc/current/cookbook/validation/custom_constraint.html
This would also produce less duplicate code e.g. if you want to update the same entity through a dashboard page and also through an API for instance.
+1 building and generating forms and crud interfaces is a sorely lacking manual feature if you come from a rails or cake php world. addons like block designer help in some ways but then you have another way of doing things not built into the core. I find myself spending a lot of time building basic crud form interfaces in the dashboard manually would be cool to be able to generate these faster.
Let's come back around to this once we have a functional beta of version 8. There are significant developments in it that are meant to address this specifically – so it may be that we won't need to introduce a third party library to get this functionality (which I agree, is lacking)
@aembler Have played a bit with 8.0.0 development branch today and I thought I want to share a couple of thoughts on this:
I'm not sure if there is more progress coming in the form building tools but this surely is not a drop-in replacement for symfony forms (or something similar).
Just repeating what I've already put out there but here's a simple example of creating a form with Symfony forms:
$object = new \My\Doctrine\Entity();
$builder = $formFactory->createBuilder('form', $object, array('action' => '/submit/this/form'))
->add('name', 'text', array('required' => true))
->add('description', 'textarea');
And then displaying it in the view with twig (similarly easy with PHP if you use the Symfony framework):
{{ form(form) }}
Validating and saving the data:
$request = new \Symfony\Component\HttpFoundation\Request::createFromGlobals();
$form->handleRequest($request);
if ($form->isValid()) {
// TODO: Return and redirect
}
// TODO: Display the form with errors
Adding custom validators to the form or creating these forms without attaching them into doctrine entities is just as simple, just few modifications above.
The validator constraints can be directly added into the doctrine entities when they are automatically applied to the forms: http://symfony.com/doc/current/reference/constraints/NotBlank.html http://symfony.com/doc/current/cookbook/validation/custom_constraint.html
Or validators can be defined manually for the form fields in case they should only apply to the form and not elsewhere:
$builder
->add('name', 'text', array(
'constraints' => array(
new \Symfony\Component\Validator\Constraints\NotBlank(),
new \Symfony\Component\Validator\Constraints\Length(array('min' => 3)),
)
)
);
Currently doesn't seem the core implementation would be even closly as flexible as Symfony forms.
I would also advice trying to create the form from scratch programmatically and compare the process to the examples given above.
I think this is reasonable. If you put together a pull request we'll take a look at it.
(Obviously that might just be updating composer but in case you had a little bit more integration with concrete5 in mind I wanted to give you room to explore it.)
@ahukkanen we are discussing this issue on the concretePHP mailing list, we might implement it in a package before attempting to get a pull request merged. Head over there and join in.
Any reason why we would move discussion away from this place where everyone had been contributing to elsewhere?
On Thu, Feb 11, 2016 at 11:12 AM, Korvin Szanto notifications@github.com wrote:
@ahukkanen https://github.com/ahukkanen we are discussing this issue on the concretePHP mailing list, we might implement it in a package before attempting to get a pull request merged. Head over there and join in.
— Reply to this email directly or view it on GitHub https://github.com/concrete5/concrete5/issues/2717#issuecomment-183014656 .
I think the concretePHP list discussion on forms should move here. On the concretePHP list it was a digression from discussion of a generic plugin mechanism, where making the core forms block extendable by pluggable field types was something speculated as an example. So moving the discussion here solves 2 problems.
What's the best way to handle that - just copy and paste relevant parts of the concretePHP discussion into a post here?
@JohntheFish I'd recommend a new issue like "Proposal: Form builder". Whether we use symfony forms or some other builder is still up for debate and this thread is pretty cluttered.
@aembler I wasn't suggesting that we move discussion, this thread is not exactly a 1-1 for what we were talking about. I was suggesting that he join in the conversation that already exists there.
Part of the reason for that is because I wouldn't want to just have @ahukkanen come up with the solution himself and submit a pull request, we're already talking about the issue and have some ideas to discuss. Your response was "If you put together a pull request we'll take a look at it.", I didn't want to undermine that with extra semi-off-topic discussion here when there is an active discussion on the mailing list.
@JohntheFish is right that we should probably just discuss it in a new proposal issue since you seem open to the idea.
Just a note for everyone, if someone is planning to build an integration. We've already built one and it works: https://github.com/mainio/c5pkg_symfony_forms
And an example of it in use: https://github.com/mainio/c5_symfony_forms_example
We're currently looking into updating the symfony packages since there are a couple of good updates coming there.
It's most probably not the most elegant solution architecturally/code-wise and it requires the twig templating engine for the view layer which would be cool if it was not necessary but it works. During the initial implementation I realized the symfony forms twig bridge is quite much easier to integrate than the PHP alternative as that is quite bound into the symfony framework itself.
@aembler I would be very happy to look into integrating this into the core.
But we can first check into whether it would be easily possible to make it twig-intependent (i.e. not requiring twig) so that it would be easier to approve into the core.
Yeah, that would be a good idea.
On Thu, Feb 11, 2016 at 11:51 AM, Antti Hukkanen notifications@github.com wrote:
@aembler https://github.com/aembler I would be very happy to look into integrating this into the core.
But we can first check into whether it would be easily possible to make it twig-intependent so that it would be easier to approve into the core.
— Reply to this email directly or view it on GitHub https://github.com/concrete5/concrete5/issues/2717#issuecomment-183035010 .
I did some research today and indeed the reason for using twig was that a lot of the components required by the symfony PHP rendering layer come from the symfony/framework-bundle
package which I don't think we want to include as it brings up a lot of additional required packages that are not necessarily needed.
However, I was able to get it working without the Symfony framework bundle by copying a couple of required components/classes from there:
Then, I also needed to implement own versions of these (or extend the originals):
Symfony\Component\Templating\Loader\FilesystemLoader
Symfony\Component\Templating\TemplateNameParser
Symfony\Component\Templating\TemplateReference
With these modifications, it works outside of the Symfony framework bundle and without the twig requirement.
We could distribute these requirements out of a separate composer package so that they would not become part of the core.
Just for reference (and for myself), here is the initialization code for binding all this together: https://gist.github.com/ahukkanen/bd9c4c337492677d9327
This approach still requires the initialization of the Symfony PHP templating engine (\Symfony\Component\Templating\PhpEngine
) which kind of sucks as it would add one more templating layer to c5 which is only used for this purpose. But I think getting rid of this requirement goes too deep and we would probably end up creating something similar anyways. Getting rid of that would require re-writing the whole rendering stuff for the forms which I think would not be that easy + would make the code more vulnerable to any changes in Symfony.
In addition, the form views need the translator helper which requires the initialization of the Symfony translator (Symfony\Component\Translation\Translator
) which of course requires loading the concrete5 translations into that as well, which might be a slight hit on performance. However, I think that:
@aembler What do you think of the addition of these two extra components (the PHP templating engine and the translator) as core requirements? It's kind of combining stuff from another framework but regarding the workload on this integration, I think it would be the best alternative at this point.
And by the way, here's the set of composer requirements to make it work:
And when they load their own requirements, it becomes into:
A couple of these are already required by the core.
+ Another point on requiring symfony/templating
:
I think at some point it would be cool if we could make the view layers easily swappable (e.g. for easier use of twig templates) and for rendering different formats of the output (e.g. html and text emails within their own templates), so the Symfony templating component might be useful also regarding that.
But that's really another issue, just adding the point here...
@ahukkanen I think being so married to symfony might burn us here. I would much rather discuss the implementation completely with everyone who is interested in benefiting and be sure that we are fully covered before we start rather than just half integrating symfony's form builder. I know that's what this thread is meant for, but this issue is specifically for implementing symfony's form builder rather than a general form builder that might be more tuned for concrete5. That's why I suggested a new issue be opened to discuss a general form builder for concrete5.
@KorvinSzanto I'm perfectly fine with that as long as this one is linked from there.
Just want to still re-emphasize my main points that support Symfony forms and why we like them so much:
So, I won't be changing my opinion even if this moves to another thread but I don't care where the discussion happens exactly if you want to move this.
While I'm not trying to bash or understate any great work you guys do on the core, from the history I can almost guarantee that implementing that solid framework for building forms would not happen anywhere near v9-v10 with the current development speed. And even if a homebrew solution is made, it will initially be a long way from the features that Symfony provides.
So obviously, I don't think the marriage with Symfony would be a bad thing either, them having a lot of high class developers who have already though these things through so that the core developers could focus on something else.
I have also brought up the couple of comments about the marriage that I don't like but even if the templating layer would be implemented specifically for c5, it doesn't get rid of the symfony/templating
requirement as the form renderer still needs an interface to refer to from there. And I also don't think this layer is adding much overhead overall since it's basically just a layer for finding and including PHP files (+ adding a couple of ready made variables available to those views).
Another question that I have for you @KorvinSzanto, would you be more open to include this if we check out how we can get rid of the templating layer inclusion which I think is the biggest thing causing the "marriage" here? As said, it wouldn't get us away from requiring symfony/templating
but anyways, if the forms were rendered through a homebrew solution, would you be more open to this?
The basic idea of the example above was just to prove it can be integrated without requiring symfony/framework-bundle
but if this needs further investigation, we are more than open to do that. I just need to know first whether we are doing it for nothing or if there is a possibility to get this forward with these efforts.
@ahukkanen I'm against moving forward with this because the implementation is happening in a black box without consideration of the end goals of the feature, I agree that waiting for any one individual to do this all on their own will cause this to take a long time to implement, but I don't see that as an excuse for moving forward without a list of things that it needs to support. We (meaning at least more than a few package and site developer using forms in concrete5) should spend some time talking about what this will be used for and why, that way we can be sure we come up with an implementation that solves the needs we have.
@KorvinSzanto Can you elaborate the "black box" part? I don't think I quite follow that point.
The basic need in any kind of web application is building forms and handling the data that user inputs. This is why most of the major framework ship with something very similar to this, the needs are always the same:
These are the basic needs, I don't think there is much more to this. I know it can be done in many ways but in short, Symfony forms just provides us a way to do this easily and quickly + with less bugs in the end-developer's code.
I'm not quite convinced why these needs would not be in line what other developers often need, as most of the major frameworks provide something very, very (and very) similar.
To be clear, I am not asking for the core team to dictate this feature, I'm asking to hold off until we have real community involvement and clear requirements.
The black box part is just that there's few voices here and it sounds like we plan to have you implement it on your own with limited input from the community. Doing it like this (which is how we've kind of always done it) leads to you being the only person who fully understands the wrapper and the implementation. Which leads to no one using the feature and limited documentation.
You listed the basic needs yes, but the Api is more important than you are letting on. We should discuss the needs of the implementation instead of just high level "it needs to be able to work" kinds of requirements. Things like how one could create a block edit interface without view files, or return a form for package configuration on install or any number of things I can't come up with right now.
Let's wait til more people like @johnthefish can tell us what would help them in their day to day before we move forward. As I said there was interest brought up by others in other places so this deserves a little more planning. On Tue, Feb 16, 2016 at 11:59 AM Antti Hukkanen notifications@github.com wrote:
@KorvinSzanto https://github.com/KorvinSzanto Can you elaborate the "black box" part? I don't think I quite follow that point.
The basic need in any kind of web application is building forms and handling the data that user inputs. This is why most of the major framework ship with something very similar to this, the needs are always the same:
- Print out form fields
- Analyze the validity of the input data
- Save the data to DB
These are the basic needs, I don't think there is much more to this. I know it can be done in many ways but in short, Symfony forms just provides us a way to do this easily and quickly + with less bugs in the end-developer's code.
I'm not quite convinced why these needs would not be in line what other developers often need, as most of the major frameworks provide something very, very (and very) similar.
— Reply to this email directly or view it on GitHub https://github.com/concrete5/concrete5/issues/2717#issuecomment-184854580 .
OK, sounds fair and I think more discussion is only positive.
And just to add to the points above, there's nothing stopping us using this in any possible view, including the block form views. And yes, that would probably need some changes in the core (especially if you want to get rid of the view file completely).
And I also want to point out that within this thread as in others, there are already a couple of other developers that have shown interest towards something this. I really think this is a real need and just a natural step after having the proper data entity definition format. So from the list above, "saving the data to DB" is covered but the two points still remain open (+ linking them together).
I dont know enough about the alternatives to have a preference on the solution. However, I do have some quick thoughts on requirements for a new forms interface:
Just adding here that I'm also willing to spend some time on documentation (or possibly someone else here at the office) in case Symfony forms is chosen as the go-to solution. It seems to be the biggest worry mentioned by @KorvinSzanto and also @JohntheFish above.
I believe most the listed points above are covered by it or the DB entities that it attaches to. And just to clarify for @JohntheFish, it is by no means any requirement to attach it to DB entities, you can attach it to also array of keys/values and then it returns an array filled with the same keys and user inputs upon submission that you can use to do anything you'd like.
For the "events" part, I don't think this is covered by the Symfony forms framework itself but I guess it's just a question on how to integrate it. With normal controllers it wouldn't become a problem as you can easily control every step of the form handling process but the form handling is hidden from the end-developer e.g. in the block implementation, some c5 events need to be added there to comply with this requirement.
For the view part, I'm not sure if it's exactly "easy" to attach jQuery events to it but Symfony forms prints out basic forms that you can target in the JS, so I think that would be easy enough. And you can of course set classes to the form fields and even customize the rendering layer if you feel like it (e.g. in case you're not using Bootstrap for your layout, since I guess the default implementation would obviously come with the bootstrap classes if it were to be easily integrable to the block views and dashboard views).
I also did a quick search for some PHP based alternatives:
The two first ones seem to offer the direct integration between data objects which is also included in Symfony forms. The other ones don't seem to be as rock solid. And I think none of these have the amount of docs available as Symfony forms.
I'm also not sure how attached these are to any rendering layers or other external components.
I actually thought Zend Form might be a better pick – its API looked like it might fit what we're doing a little more, and we certainly use as much from the Zend stack as we do from Symfony. But trying to set it up, it does require a tremendous amount of dependencies – and I couldn't really get it to work without the full Zend MVC setup as well, which i think makes it a non-starter.
How do you feel about rolling our own? I know that's a lot of work but here are some reasons I think it might be the way that we'd have to go:
For these reasons and more I think rolling our own makes the most sense. Thoughts? I like the API of Zend Form.
I'm perfectly fine with you rolling out your own system but I was just trying to mention above that it's a long road while we have perfectly good working components available that already provide a large set of features, are very well written and tested, and might provide a framework some part of the new developers finding c5 are already familiar with.
Regarding the points above, I don't think points 2.-5. would be a consern here since:
I don't actually see much difference between the Zend and Symfony APIs here regarding form the building, request data handling and validation but there are some differences regarding the rendering which I think is easier in Symfony.
If you will roll out your own implementation, I would only wish for it to be even near these libraries. And also hoping for you to take the Doctrine entities and their automatic constraint validations into account. And by that I mean something similar as in Symfony: http://symfony.com/doc/current/book/validation.html
Oh, let me be clear: we have no plans of rolling our own implementation any time soon – I just wanted to mention that it was an option, given some of the sticking points. I think having some kind of system like this in the core would be a nice addition, I'm just concerned with making sure that it doesn't require us adopting completely new systems for things. That said, if there were some kind of compatibility layer or some custom coding that could obviate the need for switching translation layers, that'd be great. Additionally, if we could swap out or unify our validation approach so that it isn't a completely different system from what we recently introduced, that would answer that question as well.
I think the requirement for using the homebrew validation layer with Symfony forms is kind of a showstopper for this since I don't think it would be possible easily. Or alternatively, we would end up rewriting some internal parts of the Symfony forms which would leave very little benefit in using it at all since I think the biggest advantage/difference it has over the other libraries is exactly the validation.
Also, I checked about the symfony/translation
requirement and it seems that these are not only used in the view layer but these are also a direct requirement by the symfony/form
component because the library is also used for the validator translations. The component also ships with the XMLs containing these translations.
The translator could of course be overridden (through implementing own version of the TranslatorInterface) to use the concrete5's own translation layer but it would not get rid of the symfony/translation
requirement.
So even if we rewrote the translator and the templating layer dependencies, we could not get rid of those packages as a couple of interfaces would still be required from each of them.
I think the other points might be possibly solvable but it's starting to seem it would be wasted effort to investigate those.
It's a shame since I really think this makes building dashboard interfaces really fast and enjoyable, avoiding writing the same code over and over again (view layer, validation and assigning the values to the data object). Shipping with these in packages is really tough since all packages need to be essentially using the same version of the libraries and it adds up quite a lot to the package size.
Seems like drupal has had the same discussion for quite some time: https://www.drupal.org/node/1447712
The main differences, though, are that 1) drupal already ships with their own form building layer (which they are discussing replacing there), 2) drupal ships with twig, so the integration would be easier, 3) drupal ships with symfony/translation
so the integration would be easier, and 4) drupal ships with symfony/validator
so the integration would be easier in that sense as well.
So the issues discussed here are kind of irrelevant for them, they are just discussing about the benefits on using symfony forms vs. continuing with their own layer.
But just one interesting comment I noticed from the thread from the original developer of the symfony/form
component:
After two years of intensive development, the main Form API becomes stable in Symfony 2.1
- Bernhard Schussek aka. webmozart Source: Original comment - May 30, 2012
And this was said 4 years ago...
Just trying to note here how long road it would be to ship with a homebrew component. And if this is the direction to be taken, it might also be worth discussing whether this component should be created as a composer dependency which would also work individually outside of the concrete5 context.
Does an improved form interface need to replace and be functionally compatible with the existing interface? Could a new interface sit alongside it and slowly be used by new code as developers get round to it? For example, 5.7 supports both XML and doctrine classes for data. So other than code size (and some technical hurdles), is there any fundamental reason for not providing two (or more) disparate form interfaces overlapping in capability?
With the above premise in mind, how many of the technical hurdles remain?
Going way off topic (but a similar underlying issue), should the core be looking for common resources and use-cases encountered by developers and aiming to provide such resources, even if they are not actually used within the core?
@JohntheFish The biggest technical hurdle regarding this issue is the marriage with the symfony framework and I think the core team is thinking this adds too many dependencies.
To your general question: I think yes, this is what a good framework does. But quite a central question regarding that would be the better handling of composer dependencies and I don't think there is a perfect way to do that yet as composer does not provide any parent/child dependency functionality. If that was possible, these components would not necessarily need to ship with the core.
@JohntheFish we would likely add it alongside the current forms implementations. I'm thinking this is something we should revisit once we have develop merged in and a few of the other big things done. At that time we can have a better discussion about this and composer dependencies.
This feature would be a massive improvement around! Twig as a templating engine, Symfony forms, the validator service. It so well maintained and easy to extend. I use these components a lot in other projects. Plus it means that UI doesn't really need to learn PHP as this is kinda handlebars syntax and its used across many other language templating styles. I hope this feature makes it ❤️
current forms, in my opinion at least, are bauked. Express forms is unwieldy, badly documented and extremely buggy at the time of writing. Not to mention being very confusing to the end user.
Using symfony forms is a natural step towards a slicker, more usable set of dev tools.
In what way are they badly documented and extremely buggy?
https://documentation.concrete5.org/developers/express/express-forms-controllers/overview
There certainly is a learning curve for express theming - and it was too bad that we had to change the api as much as we did between 8.1 and 8.2 (although the changes lead to much cleaner code going forward).
Express and symfony forms are completely orthogonal in purpose. One allows you to define attributes and associations through a GUI and the other is developer focused.
Symfony forms are perfectly fine for custom code. There’s nothing keeping you from defining them as a dependency on your composer.json and using them in your single pages.
On Thu, Sep 21, 2017 at 1:44 AM DevDonkey notifications@github.com wrote:
current forms, in my opinion at least, are bauked. Express forms is unwieldy, badly documented and extremely buggy at the time of writing. Not to mention being very confusing to the end user.
Using symfony forms is a natural step towards a slicker, more usable set of dev tools.
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/concrete5/concrete5/issues/2717#issuecomment-331092617, or mute the thread https://github.com/notifications/unsubscribe-auth/AAgNwZLA9Th62Gy6_j74qoMBFxfhHBlgks5skiIBgaJpZM4FUTmr .
@aembler Where I agree with your comment I feel that overlooking the use of something that is already there and extremely well maintained and documented such as Symfony forms or another so like component would not be such a good thing but highly beneficial to C5.
1) Many other projects use this component, meaning it's easy to pick up and transition :) 2) Validation is easy to extend, highly document and updated. 3) Doctrine bridge is already there, should be pretty straightforward to integrate with the express objects and or forms, potentially you may need a data transformer to acts as middleware between the two. 4) Reduce in-house responsibility on feature maintenance of forms, validators etc which frees up time to concentrate on core CMS functionality
I feel such things as Forms, Validators, Routing, Autoloading etc are all out there so why re-invent the wheel?
With the comment on tying yourself which is early up the thread, this isn't such a bad thing, it takes more time to maintain something, CVE patch, within house or a smaller CMS community than a large component provider such as Symfony or Zend. Thats, why I love our PHP Community, it ROCKS!
The comment regarding documentation - I personally feel finding the information can sometimes be a little hard. i.e. the symfony docs are very well layed out and clean, with anchor points to some of the basic question potentially found within a forum (frequently asked questions), I'd recommend that if something is frequently asked, perhaps creating a cookbook! Sometimes the technical jargan is cool but sometimes it just requires a clear laid out path.
Long time coming but happy to announce that Concrete CMS 10.0 will have integration with Symfony Forms, thanks to @gutig . There's no timeline for 10.0, and we're still working out exactly how we can make this integration a bit more user-facing, but it's great to have the ground work laid for the feature. Nice job.
@aembler that's great to hear look forward to seeing that in action in V10
I'd like to hear some thoughts on whether we could introduce Symfony forms into concrete5.
This would really make it easier to build forms on single pages and why not everywhere else as well. It would also generalize e.g. form validation, display of form errors specifically to some field, make the developers write code with less bugs (as we could use generalized components), handle automatically CSRF validation, and probably many other benefits as well. And of course, make the code more testable.
And now that we have the possibility to use Doctrine entities, they can be easily mapped into Symfony forms, so building CRUD interfaces for Doctrine entities would become quite much easier than it is today, almost automatic.
Basically all the bigger and widely used frameworks provide something similar to this, so this is really an important feature for most developers choosing a good framework to build something bigger.
As a reference implementation, here's an example I put together: https://github.com/mainio/c5_symfony_forms_example
It relies on a couple of composer packages that I also put together to make this all work within c5.
It also uses twig templates not necessarily because I'd want to use them but I realized the twig bridge for the form component is quite much lighter in terms of included packages than the alternative PHP version. If we wanted to use the PHP version, we would need to include quite many composer packages we don't really need or only need a very small portion of it. That's the reason for using the twig bridge there. Or other option is also writing our own bridge for the form component.
On the other hand, introducing twig as well into c5 might not be that bad idea either. It seems to have catched up quite well and it's already used by Drupal and it's the default template engine in Symfony, so it's not going anywhere any time soon. And the default engine in Symfony means that all the Symfony documentation provides twig examples as the primary examples of anything related to the views. Other advantages would include enforcing people to keep business logic out of the view layer and also cleaner views in general.
Any thoughts? Are you open to the idea? How about the twig issue then?