Skip to content

[Form][FrameworkBundle] Replace render() with new renderForm() in form documentation #15217

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
merged 3 commits into from
Sep 23, 2021

Conversation

javiereguiluz
Copy link
Member

@javiereguiluz javiereguiluz commented Apr 9, 2021

This adds some missing changes to #15105 and fixes #14844.

@javiereguiluz javiereguiluz added this to the 5.3 milestone Apr 9, 2021
@javiereguiluz javiereguiluz requested a review from xabbuh as a code owner April 9, 2021 15:18
@carsonbot carsonbot changed the title [FrameworkBundle] Replace render() with new renderForm() in form documentation [Form][FrameworkBundle] Replace render() with new renderForm() in form documentation Apr 9, 2021
@javiereguiluz
Copy link
Member Author

Don't merge yet. Let's wait for symfony/symfony#40799 to decide the best way of solving this.

nicolas-grekas added a commit to symfony/symfony 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
@javiereguiluz
Copy link
Member Author

After the latest changes made on handleForm(), I propose to not use it to document form handling. The reason is that the resulting code looks a bit messy.

Traditional form handling:

#[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);
    if ($form->isSubmitted() && $form->isValid()) {
        $this->getDoctrine()->getManager()->flush();

        return $this->redirectToRoute('conference_index', [], Response::HTTP_SEE_OTHER);
    }

    return $this->render('conference/edit.html.twig', [
        'conference' => $conference,
        'form' => $form->createView(),
    ]);
}

Form handling with handleForm():

#[Route('/{id}/edit', name: 'conference_edit', methods: ['GET', 'POST'])]
public function edit(Request $request, Conference $conference): Response
{
    return $this->handleForm(
        $request,
        $this->createForm(ConferenceType::class, $conference),
        function (FormInterface $form) use ($conference) {
            $this->getDoctrine()->getManager()->flush();

            return $this->redirectToRoute(
                'conference_show',
                ['id' => $conference->getId()],
                Response::HTTP_SEE_OTHER
            );
        },
        function (FormInterface $form) {
            return $this->render('conference/edit.html.twig', [
                'form' => $form->createView(),
            ]);
        }
    );
}

However, since this handleForm() method does some additional tasks needed when using Stimulus ... we could document it in the docs related to Stimulus. What do you think?

@nicolas-grekas
Copy link
Member

renderForm() is back, so that this PR might be good to merge now :)

@nicolas-grekas
Copy link
Member

Should be updated for symfony/symfony#41190

@javiereguiluz javiereguiluz changed the base branch from 5.4 to 5.3 September 23, 2021 09:57
@javiereguiluz javiereguiluz merged commit 29f610f into symfony:5.3 Sep 23, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[FrameworkBundle] Add renderForm() helper setting the appropriate HTTP …
5 participants