-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[FrameworkBundle] Add renderForm() helper setting the appropriate HTTP status code #39843
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
weaverryan
approved these changes
Jan 14, 2021
chalasr
approved these changes
Jan 14, 2021
yceruto
approved these changes
Jan 14, 2021
fabpot
approved these changes
Jan 17, 2021
0369ae2
to
4c77e50
Compare
Thank you @dunglas. |
What if I have 2+ forms? /**
* Renders a collection of forms.
*
* An array of forms is passed to the template using the key as form name.
* If any form is invalid, a 422 status code is returned.
*/
public function renderFormCollection(string $view, array $forms, array $parameters = [], Response $response = null): Response
{
$formCollection = [];
foreach ($forms as $formName => $form) {
if ($form->isSubmitted() && !$form->isValid()) {
if ($response === null) {
$response = new Response();
}
$response->setStatusCode(Response::HTTP_UNPROCESSABLE_ENTITY);
}
$formCollection[$formName] = $form->createView();
}
return $this->render($view, array_merge($formCollection, $parameters), $response);
} |
1 task
nicolas-grekas
added a commit
that referenced
this pull request
Apr 16, 2021
… 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
symfony-splitter
pushed a commit
to symfony/framework-bundle
that referenced
this pull request
Apr 16, 2021
… 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 symfony/symfony#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 ------- 5228546066 [FrameworkBundle] Add AbstractController::handleForm() helper
Merged
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
A 422 HTTP status code should be returned after the submission of an invalid form. Some libraries including Turbo rely on this behavior and will not display the updated form (containing errors) unless this status code is present.
Rails also recently switched to this behavior by default for the same reason.
I propose to introduce a new helper method rendering the form and setting the appropriate status code. It makes the code cleaner: