Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

finish #13598 #18047

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 2 commits into from
Closed

finish #13598 #18047

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
0baa2e9
Fixed a very weird edge case where empty data may not be a string and…
guilhermeblanco Feb 4, 2015
85e8805
add tests for previous commit to avoid regressions
xabbuh Mar 7, 2016
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
11 changes: 11 additions & 0 deletions 11 src/Symfony/Component/Form/Form.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -612,6 +612,17 @@ public function submit($submittedData, $clearMissing = true)
$emptyData = $emptyData($this, $viewData);
}

// Treat false as NULL to support binding false to checkboxes.
// Don't convert NULL to a string here in order to determine later
// whether an empty value has been submitted or whether no value has
// been submitted at all. This is important for processing checkboxes
// and radio buttons with empty values.
if (false === $emptyData) {
$emptyData = null;
} elseif (is_scalar($emptyData)) {
$emptyData = (string) $emptyData;
}

$viewData = $emptyData;
}

Expand Down
27 changes: 27 additions & 0 deletions 27 src/Symfony/Component/Form/Tests/Extension/Core/Type/CheckboxTypeTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -170,4 +170,31 @@ public function provideCustomModelTransformerData()
array('unchecked', false),
);
}

public function testSubmitNullWithEmptyDataTrue()
{
$form = $this->factory->create('checkbox', null, array(
'empty_data' => true,
Copy link
Contributor

Choose a reason for hiding this comment

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

Actually, this code is invalid. empty_data expects data in view format, not in model format. The view format depends on the transformer that is used.

Imagine the field is configured with the following transformer

true <---> 'yes'
false <---> 'no'

Then this code will fail. The empty_data value true will be cast to the string '1'. Consequently, the transformer fails, since it doesn't know how to reverse transform '1'.

The correct test would be:

$form = $this->factory->create('checkbox', null, array(
    'empty_data' => '1',
));

I understand this is not very straight-forward nor intuitive, but we can't change that for the moment.

I believe that the use case that motivated this change was actually using the empty_data incorrectly, by passing data in model instead of view format (ping @guilhermeblanco).

Copy link
Contributor

Choose a reason for hiding this comment

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

But it's valid with default settings, right ?

In that test $this->assertSame('1', $viewData) would pass.

));

$form->submit(null);

$this->assertTrue($form->getData());
$this->assertSame('1', $form->getViewData());
}

public function testSubmitNullWithEmptyDataFalse()
{
$form = $this->factory->create('checkbox', null, array(
'empty_data' => false,
));

$form->submit(null);

$this->assertFalse($form->getData());

$view = $form->createView();

$this->assertFalse($view->vars['checked']);
}
}
22 changes: 22 additions & 0 deletions 22 src/Symfony/Component/Form/Tests/Extension/Core/Type/IntegerTypeTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,4 +32,26 @@ public function testSubmitCastsToInteger()
$this->assertSame(1, $form->getData());
$this->assertSame('1', $form->getViewData());
}

public function testSubmitNull()
{
$form = $this->factory->create('integer');

$form->submit(null);

$this->assertNull($form->getData());
$this->assertSame('', $form->getViewData());
}

public function testSubmitNullWithEmptyData()
{
$form = $this->factory->create('integer', null, array(
'empty_data' => 1,
));

$form->submit(null);

$this->assertSame(1, $form->getData());
$this->assertSame('1', $form->getViewData());
}
}
Morty Proxy This is a proxified and sanitized view of the page, visit original site.