Simplify the form submission #27638
Comments
|
How to separate not submitted forms with not valid in that case? |
|
We not remove existing method just create a new one « helper » |
|
What about providing a callback to a form, that would be called when it is submitted, valid, etc.? |
|
Like a listener to |
|
Not sure if it would be worth it to add some kind of helper method to the public function handleFormSubmissionRequest(FormInterface $form, Request $request, callable $onSuccess = null, callable $onFailure = null): void
{
$form->handleRequest($request);
if ($form->isSubmitted()) {
if (null !== $onSuccess && $form->isValid()) {
$onSuccess($form);
}
if (null !== $onFailure && !$form->isValid()) {
$onFailure($form);
}
}
} |
|
It's always interesting to see how others solved the same problems you have. So here is for example how do they process forms in Django: def get_name(request):
# method is POST -> process the submitted form
if request.method == 'POST':
form = NameForm(request.POST)
if form.is_valid():
# process the data in form.cleaned_data as required...
return HttpResponseRedirect('/thanks/')
# method is GET -> display the initial form
else:
form = NameForm()
return render(request, 'name.html', {'form': form}) |
|
I'm not a fan of using callbacks, we will end up with anonymous functions in controllers instead of an explicit API. But I do like the idea of using the controller trait to get this helper, what about: // trait
public function isFormValidForRequest(FromInterface $form, Request $request): bool
{
return $form->handleRequest($request)->isSubmitted() && $form->isValid();
}
// controller
public function action(Request $request): Response
{
$form = $this->createForm(MyType::class);
if ($this-> isFormValidForRequest($form, $request)) {
// do stuff and redirect
}
// render
} |
|
Something like that @xabbuh yes. By doing this kind of logic in the Cache component, we made it much easier to use, and in fact it became much more than a helper as this is now backed by a first class interface and interesting features have been built on top. Maybe applicable here also? |
|
Based on the idea of @xabbuh: public function handleForm(string $formType, callable $preSubmit, callable $onSuccess)
{
$form = $this->createForm($formType);
// Try to get the $request automatically (autowiring, container...)
$form->handleRequest($request);
if ($form->isSubmitted() && $form->isValid()) {
return $onSuccess($form);
}
return $preSubmit($form);
}In controller, there is only: public function myAction()
{
return $this->handleForm(MyFormType::class, function (FormInterface $form) {
return $this->render('form.html.twig', ['form' => $form->createView()]);
}, function (FormInterface $form) {
return $this->redirectToRoute('success');
});
} |
|
I'm afraid that I agree with @HeahDude ... the proposals don't look much better than the current situation (and considering that it would obsolete all our docs, tutorials, screencasts, etc.) I'm not sure it's worth it. |
|
Why not simply |
|
The problem I see with I'm not sure if there is really something to do here but if there was I would be more in favor of combining the |
@Pierstoval because it would make the shortcut be a form interface contract, keeping it in the framework bundle trait is better IMO, since the request object is not tied to the form. |
|
I am happy to see all proposals, IMO the @HeahDude solution (add a simple method in the |
|
Then |
|
yes yes yes, a shortcut in the trait. Thus the PR already exist: |
|
Good point @lyrixx ! |
|
A lot of points from this discussion are leading towards the form handler bundle I've worked on, perhaps it's worth checking it out? https://github.com/hostnet/form-handler-bundle It lets you create thin controllers by extracting any logic to the handler, which can have an "onSuccess" flow and an "onFailure" flow (configurable). // example taken from the readme
class YourController extends Controller
{
public function formAction(Request $request, MyEntityUser $user)
{
$handler = $this->get('hostnet.form_handler.factory')->create(MyFormHandler::class);
if (($response = $handler->handle($request, new MyFormData())) instanceof RedirectResponse) {
return $response;
}
// regular or in-valid flow
return $this->render->renderView('/your/form.html.twig', [
'form' => $handler->getForm()->createView()
]);
}
} |
|
@iltar In the readme, there are no mention of binding a form to an existing object. Is it intended? $handler = $this->get('hostnet.form_handler.factory')->create(MyFormHandler::class, $myObject); |
|
@lyrixx your form is bound to a specific object, you configure which form this is in your handler. At the moment of creation, you might not yet (want to) have your data object: https://github.com/hostnet/form-handler-component/blob/87cbc8444ec20a0c66bc8e05e79a102fb7cf4b0d/src/FormHandler/Handler.php#L92-L104 I guess it could be possible that way, but it was written like this because you handle a specific data set rather than just handling a handler. Edit: |
|
@lyrixx I missed that one too, what about reopening? :) |
|
On a related note, how about reporting an error in |
|
I rebased #24576 and addressed your comments. No more error prone and super easy. |
|
I reopen the issue because the code was revert in 2be1987 |
|
This issue was opened more than a year ago. We had a lot of ideas, but none of them was convincing enough. Therefore I suggest that we close here as we weren't able to agree on a solution. We could still reconsider if someone comes up with a great new idea that receives enough support. |
Uh oh!
There was an error while loading. Please reload this page.
Description
At the moment when you want to submit a form in a controller most of the time you should call 3 methods
handleRequest($request),isSubmitted()andisValid()see documentation and if you callisValid()without callingisSubmitted()before it will throw an ExceptionI know it's little magic but the simple idea is to aggregate the 3 methods in 1.
the method will handle the request, check if the form isSubmitted and if it's valid.
Example
The method name is a simple example. I'am ready to work on if you're OK
I think the community have already spoken about that, but I'm not sure, WDYT ?
The text was updated successfully, but these errors were encountered: