Skip to content

Commit

Permalink
feature #17644 Deprecate using Form::isValid() with an unsubmitted fo…
Browse files Browse the repository at this point in the history
…rm (Ener-Getick)

This PR was merged into the 3.2-dev branch.

Discussion
----------

Deprecate using Form::isValid() with an unsubmitted form

| Q             | A
| ------------- | ---
| Bug fix?      | no
| New feature?  | yes
| BC breaks?    | no
| Deprecations? | yes
| Tests pass?   | yes
| Fixed tickets | #7737
| License       | MIT
| Doc PR        | not yet

> I'm calling out for you to decide how to resolve the inconsistency between the Form::isValid() method and the ``valid`` variable available in the template:
>
> - ``isValid()`` returns ``false`` if a form was not submitted. This way it is possible to write concise controller code:
>
> ```php
> $form = $this->createForm(...);
> $form->handleRequest($request);
> if ($form->isValid()) {
>     // only executed if the form is submitted AND valid
> }
> ```
>
> - ``valid`` contains ``true`` if a form was not submitted. This way it is possible to rely on this variable for error styling of a form.
>
> ```twig
> <div{% if not form.vars.valid %} class="error"{% endif %}>
> ```
>
> We have two alternatives for resolving this problem:
>
> 1. Leave the inconsistency as is.
> 2. Make ``isValid()`` return ``true`` if a form was not submitted (consistent with ``valid``)
> 3. Revert to the 2.2 behavior of throwing an exception if ``isValid()`` is called on a non-submitted form (not consistent with ``valid``).
>
> Both 2. and 3. will require additional code in the controller:
>
> ```php
> $form = $this->createForm(...);
> $form->handleRequest($request);
> if ($form->isSubmitted() && $form->isValid()) {
>     // only executed if the form is submitted AND valid
> }
> ```
>
> What do you think?

This PR implements the option 3 as it was the most chosen in #7737

Commits
-------

2c3a7cc Deprecate using Form::isValid() with an unsubmitted form
  • Loading branch information
fabpot committed Jun 17, 2016
2 parents 22f7ed7 + 2c3a7cc commit 7ccfdb4
Show file tree
Hide file tree
Showing 5 changed files with 48 additions and 2 deletions.
22 changes: 22 additions & 0 deletions UPGRADE-3.2.md
Expand Up @@ -7,6 +7,28 @@ DependencyInjection
* Calling `get()` on a `ContainerBuilder` instance before compiling the
container is deprecated and will throw an exception in Symfony 4.0.

Form
----

* Calling `isValid()` on a `Form` instance before submitting it
is deprecated and will throw an exception in Symfony 4.0.

Before:

```php
if ($form->isValid()) {
// ...
}
```

After:

```php
if ($form->isSubmitted() && $form->isValid()) {
// ...
}
```

Validator
---------

Expand Down
19 changes: 19 additions & 0 deletions UPGRADE-4.0.md
Expand Up @@ -42,6 +42,25 @@ Form
* Caching of the loaded `ChoiceListInterface` in the `LazyChoiceList` has been removed,
it must be cached in the `ChoiceLoaderInterface` implementation instead.

* Calling `isValid()` on a `Form` instance before submitting it is not supported
anymore and raises an exception.

Before:

```php
if ($form->isValid()) {
// ...
}
```

After:

```php
if ($form->isSubmitted() && $form->isValid()) {
// ...
}
```

FrameworkBundle
---------------

Expand Down
2 changes: 2 additions & 0 deletions src/Symfony/Component/Form/Form.php
Expand Up @@ -724,6 +724,8 @@ public function isEmpty()
public function isValid()
{
if (!$this->submitted) {
@trigger_error('Call Form::isValid() with an unsubmitted form is deprecated since version 3.2 and will throw an exception in 4.0. Use Form::isSubmitted() before Form::isValid() instead.', E_USER_DEPRECATED);

return false;
}

Expand Down
2 changes: 1 addition & 1 deletion src/Symfony/Component/Form/FormInterface.php
Expand Up @@ -192,7 +192,7 @@ public function addError(FormError $error);
/**
* Returns whether the form and all children are valid.
*
* If the form is not submitted, this method always returns false.
* If the form is not submitted, this method always returns false (but will throw an exception in 4.0).
*
* @return bool
*/
Expand Down
5 changes: 4 additions & 1 deletion src/Symfony/Component/Form/Tests/SimpleFormTest.php
Expand Up @@ -11,6 +11,7 @@

namespace Symfony\Component\Form\Tests;

use Symfony\Bridge\PhpUnit\ErrorAssert;
use Symfony\Component\Form\Form;
use Symfony\Component\Form\FormEvent;
use Symfony\Component\Form\FormEvents;
Expand Down Expand Up @@ -315,7 +316,9 @@ public function testValidIfSubmittedAndDisabled()

public function testNotValidIfNotSubmitted()
{
$this->assertFalse($this->form->isValid());
ErrorAssert::assertDeprecationsAreTriggered(array('Call Form::isValid() with an unsubmitted form'), function () {
$this->assertFalse($this->form->isValid());
});
}

public function testNotValidIfErrors()
Expand Down

0 comments on commit 7ccfdb4

Please sign in to comment.