Skip to content

[Form] compound forms without children should be considered rendered implicitly #20108

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.

Sign up for GitHub

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 1 commit into from
Closed

[Form] compound forms without children should be considered rendered implicitly #20108

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
8 changes: 6 additions & 2 deletions src/Symfony/Component/Form/FormView.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,8 +65,12 @@ public function __construct(FormView $parent = null)
*/
public function isRendered()
{
if (true === $this->rendered || 0 === count($this->children)) {
return $this->rendered;
if ($this->rendered) {
return true;
}

if (isset($this->vars['compound']) ? !$this->vars['compound'] : 0 === count($this->children)) {
return false;
}

foreach ($this->children as $child) {
Expand Down
2 changes: 1 addition & 1 deletion src/Symfony/Component/Form/Tests/AbstractDivLayoutTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -453,7 +453,7 @@ public function testNestedFormError()
$form = $this->factory->createNamedBuilder('name', 'form')
->add($this->factory
->createNamedBuilder('child', 'form', null, array('error_bubbling' => false))
->add('grandChild', 'form')
->add('grandChild', 'text')
Copy link
Contributor

Choose a reason for hiding this comment

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

Why this change?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

$form->createView()->isRendered() now returns true, when the whole form tree is compound and would result in this test case failing.

The real change is for $grandChild->createView()->isRendered() which now returns true in case grandChild is compound. This is because compound forms with no children are considered rendered now, where before each leaf form view (that includes compound forms without children), that doesnt have its rendered flag explicitly set, would not be considered rendered.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok it was reverted when I tried on master because of the conflict resolution. So I think you should not make that change and fix the layout test in this case because it does impact form_rest and this should be tested too.

)
->getForm();

Expand Down
2 changes: 1 addition & 1 deletion src/Symfony/Component/Form/Tests/AbstractTableLayoutTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -318,7 +318,7 @@ public function testNestedFormError()
$form = $this->factory->createNamedBuilder('name', 'form')
->add($this->factory
->createNamedBuilder('child', 'form', null, array('error_bubbling' => false))
->add('grandChild', 'form')
->add('grandChild', 'text')
)
->getForm();

Expand Down
59 changes: 55 additions & 4 deletions src/Symfony/Component/Form/Tests/Extension/Core/Type/FormTypeTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -488,16 +488,67 @@ public function testPassMultipartTrueIfAnyChildIsMultipartToView()
$this->assertTrue($view->vars['multipart']);
}

public function testViewIsNotRenderedByDefault()
public function testViewIsConsideredRenderedForRenderedNonCompoundForms()
{
$view = $this->factory->createBuilder('form', null, array(
'compound' => false,
))
->getForm()
->createView();

$view->setRendered();

$this->assertTrue($view->isRendered());
}

public function testViewIsNotConsideredRenderedImplicitlyForNonCompoundForms()
{
$view = $this->factory->createBuilder('form', null, array(
'compound' => false,
))
->getForm()
->createView();

$this->assertFalse($view->isRendered());
}

public function testViewIsNotConsideredRenderedImplicitlyForCompoundFormsWithNonCompoundChildren()
{
$view = $this->factory->createBuilder('form')
->add('foo', 'form')
->getForm()
->createView();
->add('foo', 'form', array(
'compound' => false,
))
->getForm()
->createView();

$this->assertFalse($view->isRendered());
}

public function testViewIsConsideredRenderedImplicitlyForCompoundFormsWithRenderedNonCompoundChildren()
{
$view = $this->factory->createBuilder('form')
->add('foo', 'form', array(
'compound' => false,
))
->getForm()
->createView();

foreach ($view as $child) {
$child->setRendered();
}

$this->assertTrue($view->isRendered());
}

public function testViewIsConsideredRenderedImplicitlyForCompoundFormsWithoutChildren()
{
$view = $this->factory->createBuilder('form')
->getForm()
->createView();

$this->assertTrue($view->isRendered());
}

public function testErrorBubblingIfCompound()
{
$form = $this->factory->create('form', null, array(
Expand Down