8000 feature #40799 [FrameworkBundle] Add AbstractController::handleForm()… · symfony/symfony@549c097 · GitHub
[go: up one dir, main page]
Skip to content

Commit 549c097

Browse files
nicolas-grekasnicolas-grekas
committed
feature #40799 [FrameworkBundle] Add AbstractController::handleForm() helper (dunglas)
This PR was squashed before being merged into the 5.3-dev branch. Discussion ---------- [FrameworkBundle] Add AbstractController::handleForm() helper | Q | A | ------------- | --- | Branch? | 5.x | Bug fix? | no | New feature? | yes <!-- please update src/**/CHANGELOG.md files --> | Deprecations? | no <!-- please update UPGRADE-*.md and src/**/CHANGELOG.md files --> | Tickets | n/a | License | MIT | Doc PR | symfony/symfony-docs#15217 Some libraries such as Turbo require to strictly follow the HTTP specification (and especially to use proper status codes) to deal with forms. In #39843, I introduced a new `renderForm()` helper for this purpose. But I'm not very satisfied by it. The current approach has several problems: 1. It calls `$form->isValid()` two times, which may hurt performance 2. It sets the proper status code in case of validation error (422), but not for the redirection when the entity is created or updated (306). The user must do this manually (and so be aware of these HTTP subtleties). 3. It hides the verbosity of the Form component a bit, but I'm a sure that we can reduce it more This PR proposes an alternative helper, `handleForm()`, which handles automatically 80% of the use cases, provide an extension point for the other 20%, and can also serve as a quick example for users to handle form in a custom way (by control-clicking on the function to see the code and copy/paste/adapt it). * if the form is not submitted, the Twig template passed in $view is rendered and a 200 HTTP status code is set * if the form is submitted but invalid, the Twig template passed in $view is rendered and 422 HTTP status code is set * if the form is submitted and valid, the entity is saved (only if it is managed by Doctrine ORM), a 306 HTTP status code is set and the Location HTTP header is set to the value of $redirectUrl Before (standard case): ```php #[Route('/{id}/edit', name: 'conference_edit', methods: ['GET', 'POST'])] public function edit(Request $request, Conference $conference): Response { $form = $this->createForm(ConferenceType::class, $conference); $form->handleRequest($request); $submitted = $form->isSubmitted(); $valid = $submitted && $form->isValid(); if ($valid) { $this->getDoctrine()->getManager()->flush(); return $this->redirectToRoute('conference_index', [], Response::HTTP_SEE_OTHER); } $response = $this->render('conference/edit.html.twig', [ 'conference' => $conference, 'form' => $form->createView(), ]); if ($submitted && !$valid) { $response->setStatusCode(Response::HTTP_UNPROCESSABLE_ENTITY); } return $response; } ``` With the new helper: ```php #[Route('/{id}/edit', name: 'conference_edit', methods: ['GET', 'POST'])] public function edit(Request $request, Conference $conference): Response { $form = $this->createForm(ConferenceType::class, $conference); return $this->handleForm( $request, $form, view: 'conference/edit.html.twig', redirectUrl: $this->generateUrl('conference_index') ); } ``` Before (more advanced use case): ```php #[Route('/{id}/edit', name: 'conference_edit', methods: ['GET', 'POST'])] public function edit(Request $request, Conference $conference, HubInterface $hub): Response { $form = $this->createForm(ConferenceType::class, $conference); $form->handleRequest($request); $submitted = $form->isSubmitted(); $valid = $submitted && $form->isValid(); if ($valid) { $this->getDoctrine()->getManager()->flush(); $hub->publish( new Update( 'conference:'.$conference->getId(), $this->renderView('conference/edit.stream.html.twig', ['conference' => $conference]) ) ); return $this->redirectToRoute('conference_index', [], Response::HTTP_SEE_OTHER); } $response = $this->render('conference/edit.html.twig', [ 'conference' => $conference, 'form' => $form->createView(), ]); if ($submitted && !$valid) { $response->setStatusCode(Response::HTTP_UNPROCESSABLE_ENTITY); } return $response; } ``` With the new helper (more advanced case): ```php #[Route('/{id}/edit', name: 'conference_edit', methods: ['GET', 'POST'])] public function edit(Request $request, Conference $conference, HubInterface $hub): Response { $form = $this->createForm(ConferenceType::class, $conference); $response = $this->handleForm( $request, $form, view: 'conference/edit.html.twig', redirectUrl: $this->generateUrl('conference_index') ); if ($response->isRedirection()) { $hub->publish( new Update( 'conference:' . $conference->getId(), $this->renderView('conference/edit.stream.html.twig', ['conference' => $conference]) ) ); } return $response; } ``` This also works without named parameters. I also considered passing a callback to be executed on success, but I'm happier with the current solution. WDYT? TODO: * [x] update tests Commits ------- 5228546 [FrameworkBundle] Add AbstractController::handleForm() helper
2 parents d7007d7 + 5228546 commit 549c097

File tree

3 files changed

+67
-35
lines changed

3 files changed

+67
-35
lines changed

‎src/Symfony/Bundle/FrameworkBundle/CHANGELOG.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ CHANGELOG
77
* Deprecate the `session.storage` alias and `session.storage.*` services, use the `session.storage.factory` alias and `session.storage.factory.*` services instead
88
* Deprecate the `framework.session.storage_id` configuration option, use the `framework.session.storage_factory_id` configuration option instead
99
* Deprecate the `session` service and the `SessionInterface` alias, use the `Request::getSession()` or the new `RequestStack::getSession()` methods instead
10-
* Added `AbstractController::renderForm()` to render a form and set the appropriate HTTP status code
10+
* Added `AbstractController::handleForm()` to handle a form and set the appropriate HTTP status code
1111
* Added support for configuring PHP error level to log levels
1212
* Added the `dispatcher` option to `debug:event-dispatcher`
1313
* Added the `event_dispatcher.dispatcher` tag

‎src/Symfony/Bundle/FrameworkBundle/Controller/AbstractController.php‎

Lines changed: 19 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -291,15 +291,28 @@ protected function stream(string $view, array $parameters = [], StreamedResponse
291291
}
292292

293293
/**
294-
* Renders a form.
294+
* Handles a form.
295295
*
296-
* The FormView instance is passed to the template in a variable named "form".
297-
* If the form is invalid, a 422 status code is returned.
296+
* * if the form is not submitted, $render is called
297+
* * if the form is submitted but invalid, $render is called and a 422 HTTP status code is set if the current status hasn't been customized
298+
* * if the form is submitted and valid, 7440 $onSuccess is called, usually this method saves the data and returns a 303 HTTP redirection
299+
*
300+
* @param callable(FormInterface, mixed): Response $onSuccess
301+
* @param callable(FormInterface, mixed): Response $render
298302
*/
299-
public function renderForm(string $view, FormInterface $form, array $parameters = [], Response $response = null): Response
303+
public function handleForm(FormInterface $form, Request $request, callable $onSuccess, callable $render): Response
300304
{
301-
$response = $this->render($view, ['form' => $form->createView()] + $parameters, $response);
302-
if ($form->isSubmitted() && !$form->isValid()) {
305+
$form->handleRequest($request);
306+
307+
$submitted = $form->isSubmitted();
308+
309+
$data = $form->getData();
310+
if ($submitted && $form->isValid()) {
311+
return $onSuccess($form, $data);
312+
}
313+
314+
$response = $render($form, $data);
315+
if ($submitted && 200 === $response->getStatusCode()) {
303316
$response->setStatusCode(Response::HTTP_UNPROCESSABLE_ENTITY);
304317
}
305318

‎src/Symfony/Bundle/FrameworkBundle/Tests/Controller/AbstractControllerTest.php‎

Lines changed: 47 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,6 @@
2323
use Symfony\Component\Form\FormConfigInterface;
2424
use Symfony\Component\Form\FormFactoryInterface;
2525
use Symfony\Component\Form\FormInterface;
26-
use Symfony\Component\Form\FormView;
2726
use Symfony\Component\HttpFoundation\BinaryFileResponse;
2827
use Symfony\Component\HttpFoundation\File\Exception\FileNotFoundException;
2928
use Symfony\Component\HttpFoundation\File\File;
@@ -425,50 +424,70 @@ public function testStreamTwig()
425424
$this->assertInstanceOf(StreamedResponse::class, $controller->stream('foo'));
426425
}
427426

428-
public function testRenderFormTwig()
427+
public function testHandleFormNotSubmitted()
429428
{
430-
$formView = new FormView();
431-
432429
$form = $this->createMock(FormInterface::class);
433-
$form->expects($this->once())->method('createView')->willReturn($formView);
434-
435-
$twig = $this->createMock(Environment::class);
436-
$twig->expects($this->once())->method('render')->with('foo', ['form' => $formView, 'bar' => 'bar'])->willReturn('bar');
437-
438-
$container = new Container();
439-
$container->set('twig', $twig);
430+
$form->expects($this->once())->method('isSubmitted')->willReturn(false);
440431

441432
$controller = $this->createController();
442-
$controller->setContainer($container);
443-
444-
$response = $controller->renderForm('foo', $form, ['bar' => 'bar']);
433+
$response = $controller->handleForm(
434+
$form,
435+
Request::create('https://example.com'),
436+
function (FormInterface $form, $data): Re ED4F sponse {
437+
return new RedirectResponse('https://example.com/redir', Response::HTTP_SEE_OTHER);
438+
},
439+
function (FormInterface $form, $data): Response {
440+
return new Response('rendered');
441+
}
442+
);
445443

446444
$this->assertTrue($response->isSuccessful());
447-
$this->assertSame('bar', $response->getContent());
445+
$this->assertSame('rendered', $response->getContent());
448446
}
449447

450-
public function testRenderInvalidFormTwig()
448+
public function testHandleFormInvalid()
451449
{
452-
$formView = new FormView();
453-
454450
$form = $this->createMock(FormInterface::class);
455-
$form->expects($this->once())->method('createView')->willReturn($formView);
456451
$form->expects($this->once())->method('isSubmitted')->willReturn(true);
457452
$form->expects($this->once())->method('isValid')->willReturn(false);
458453

459-
$twig = $this->createMock(Environment::class);
460-
$twig->expects($this->once())->method('render')->with('foo', ['form' => $formView, 'bar' => 'bar'])->willReturn('bar');
454+
$controller = $this->createController();
455+
$response = $controller->handleForm(
456+
$form,
457+
Request::create('https://example.com'),
458+
function (FormInterface $form): Response {
459+
return new RedirectResponse('https://example.com/redir', Response::HTTP_SEE_OTHER);
460+
},
461+
function (FormInterface $form): Response {
462+
return new Response('rendered');
463+
}
464+
);
461465

462-
$container = new Container();
463-
$container->set('twig', $twig);
466+
$this->assertSame(Response::HTTP_UNPROCESSABLE_ENTITY, $response->getStatusCode());
467+
$this->assertSame('rendered', $response->getContent());
468+
}
464469

465-
$controller = $this->createController();
466-
$controller->setContainer($container);
470+
public function testHandleFormValid()
471+
{
472+
$form = $this->createMock(FormInterface::class);
473+
$form->expects($this->once())->method('isSubmitted')->willReturn(true);
474+
$form->expects($this->once())->method('isValid')->willReturn(true);
467475

468-
$response = $controller->renderForm('foo', $form, ['bar' => 'bar']);
476+
$controller = $this->createController();
477+
$response = $controller->handleForm(
478+
$form,
479+
Request::create('https://example.com'),
480+
function (FormInterface $form): Response {
481+
return new RedirectResponse('https://example.com/redir', Response::HTTP_SEE_OTHER);
482+
},
483+
function (FormInterface $form): Response {
484+
return new Response('rendered');
485+
}
486+
);
469487

470-
$this->assertSame(Response::HTTP_UNPROCESSABLE_ENTITY, $response->getStatusCode());
471-
$this->assertSame('bar', $response->getContent());
488+
$this->assertInstanceOf(RedirectResponse::class, $response);
489+
$this->assertSame(Response::HTTP_SEE_OTHER, $response->getStatusCode());
490+
$this->assertSame('https://example.com/redir', $response->getTargetUrl());
472491
}
473492

474493
public function testRedirectToRoute()

0 commit comments

Comments
 (0)
0