8000 [Form] Add ability to clear form errors by TerjeBr · Pull Request #27571 · symfony/symfony · GitHub
[go: up one dir, main page]

Skip to content

[Form] Add ability to clear form errors #27571

8000
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

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions UPGRADE-5.0.md
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,11 @@ EventDispatcher

* The `TraceableEventDispatcherInterface` has been removed.

Form
----

* The method `clearErrors()` has been added to `FormInterface`.

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

Expand Down
8 changes: 8 additions & 0 deletions src/Symfony/Component/Form/Button.php
Original file line number Diff line number Diff line change
Expand Up @@ -193,6 +193,14 @@ public function getErrors($deep = false, $flatten = true)
return new FormErrorIterator($this, array());
}

/**
* {@inheritdoc}
*/
public function clearErrors($deep = false)
{
return $this;
}

/**
* Unsupported method.
*
Expand Down
1 change: 1 addition & 0 deletions src/Symfony/Component/Form/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ CHANGELOG
* deprecated the `ChoiceLoaderInterface` implementation in `CountryType`, `LanguageType`, `LocaleType` and `CurrencyType`
* added `input=datetime_immutable` to DateType, TimeType, DateTimeType
* added `rounding_mode` option to MoneyType
* added `FormInterface::clearErrors()` method
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we cannot do this in a minor release as it is a BC break


4.0.0
-----
Expand Down
16 changes: 16 additions & 0 deletions src/Symfony/Component/Form/Form.php
Original file line number Diff line number Diff line change
Expand Up @@ -795,6 +795,22 @@ public function getErrors($deep = false, $flatten = true)
return new FormErrorIterator($this, $errors);
}

/**
* {@inheritdoc}
*/
public function clearErrors($deep = false)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

public function clearErrors(bool $deep = false): void

{
$this->errors = array();
// Clear also the errors of nested forms
if ($deep) {
foreach ($this as $child) {
$child->clearErrors(true);
}
}

return $this;
}

/**
* {@inheritdoc}
*/
Expand Down
9 changes: 9 additions & 0 deletions src/Symfony/Component/Form/FormInterface.php
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,15 @@ public function all();
*/
public function getErrors($deep = false, $flatten = true);

/**
* Removes all the errors of this form.
*
* @param bool $deep Whether to remove errors of child forms as well
*
* @return FormInterface The form instance
*/
public function clearErrors($deep = false);
Copy link
Contributor
@linaori linaori Jun 11, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this will be possible. You'll have to make this work for 4.x as well and as is, it's a BC break. Probably requires a new interface, which can be merged in this interface in 5.0.

Copy link
Author
@TerjeBr TerjeBr Jun 11, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you mean I should create something like FormClearErrorsInterface and make Form implement that?

If we want to merge it into FormInterface in 5.0, does that mean the FormClearErrorsInterface should be marked as deprecated right away, so that people will be aware that it goes away in version 5.0?

I have not done something like this before, so that is why I would apreciate some guideance in how to do it. Thanks.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also @dosten wrote in #14233 (comment) that the BC break would not be a problem.

Copy link
Contributor

Also @dosten wrote in #14233 (comment) that the BC break would not be a problem.

The BC promise states that adding a new method is not acceptable http://symfony.com/doc/current/contributing/code/bc.html#changing-interfaces

Copy link
Contributor
@jvasseur jvasseur Jun 11, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@TerjeBr The comment you linked dates from 2015, since then the concept of @api interface was removed and we now consider every class as part of the public API (#15977, symfony/symfony-docs#5735). This means we can't add methods to any interface.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a BC break that needs to be reverted.


/**
* Updates the form with default data.
*
Expand Down
32 changes: 32 additions & 0 deletions src/Symfony/Component/Form/Tests/CompoundFormTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -879,6 +879,38 @@ public function testGetErrorsDeepRecursive()
$this->assertSame($nestedError, $nestedErrorsAsArray[0]);
}

public function testClearErrors()
{
$this->form->addError(new FormError('Error 1'));
$this->form->addError(new FormError('Error 2'));
$this->form->clearErrors();
$this->assertCount(0, $this->form->getErrors());
}

public function testClearErrorsShallow()
{
$this->form->addError($error1 = new FormError('Error 1'));
$this->form->addError($error2 = new FormError('Error 2'));
$childForm = $this->getBuilder('Child')->getForm();
$childForm->addError(new FormError('Nested Error'));
$this->form->add($childForm);
$this->form->clearErrors(false);
$this->assertCount(0, $this->form->getErrors(false));
$this->assertCount(1, $this->form->getErrors(true));
}

public function testClearErrorsDeep()
{
$this->form->addError($error1 = new FormError('Error 1'));
$this->form->addError($error2 = new FormError('Error 2'));
$childForm = $this->getBuilder('Child')->getForm();
$childForm->addError($nestedError = new FormError('Nested Error'));
$this->form->add($childForm);
$this->form->clearErrors(true);
$this->assertCount(0, $this->form->getErrors(false));
$this->assertCount(0, $this->form->getErrors(true));
}

// Basic cases are covered in SimpleFormTest
public function testCreateViewWithChildren()
{
Expand Down
7 changes: 7 additions & 0 deletions src/Symfony/Component/Form/Tests/SimpleFormTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -334,6 +334,13 @@ public function testHasNoErrors()
$this->assertCount(0, $this->form->getErrors());
}

public function testClearErrors()
{
$this->form->addError(new FormError('Error!'));
$this->form->clearErrors();
$this->assertCount(0, $this->form->getErrors());
}

/**
* @expectedException \Symfony\Component\Form\Exception\AlreadySubmittedException
*/
Expand Down
0