8000 [Form] fix problems when the field values are Boolean by qlombat · Pull Request #17755 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

[Form] fix problems when the field values are Boolean #17755

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

Jump to bottom
Closed
wants to merge 1 commit into from
Closed

[Form] fix problems when the field values are Boolean #17755

wants to merge 1 commit into from

Conversation

qlombat
Copy link
@qlombat qlombat commented Feb 10, 2016
Q A
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets -
License MIT
Doc PR -

Problem

When the field values are Boolean as below, false value is not shown.
class RadioYesNoType extends AbstractType

{
    public function setDefaultOptions(OptionsResolverInterface $resolver)
    {
        $resolver->setDefaults(array(
            'choices' => array(
                false => 'Yes',
                true => 'No',
            ),
            'multiple'  => false,
            'expanded'  => true,
        ));
    }

    public function getParent()
    {
        return 'choice';
    }

    public function getName()
    {
        return 'Radio_Yes_No';
    }
}

@javiereguiluz
Copy link
Member

@be-bewweb please, update the description of your pull request to add the checklist as explained here: http://symfony.com/doc/current/contributing/code/patches.html#make-a-pull-request Thanks!

@qlombat qlombat changed the title Update Form.php [form] fix problems when the field values are Boolean Feb 10, 2016
@xabbuh
Copy link
Member
xabbuh commented Feb 10, 2016

If this fixes a bug, please also add a test to avoid future regressions.

@qlombat qlombat changed the title [form] fix problems when the field values are Boolean [Form] fix problems when the field values are Boolean Feb 10, 2016
@qlombat
Copy link
Author
qlombat commented Feb 10, 2016

Where can I add the test ? Sorry but this is the first time I do a pull request. Thanks for your help

@fabpot
Copy link
Member
fabpot commented Mar 3, 2016

@be-bewweb The tests for the Form component are stored under the src/Symfony/Component/Form/Tests directory. The tests for the Form class are in the src/Symfony/Component/Form/Tests/SimpleFormTest.php, which is regular PHPUnit test class. You should add a new test in this file to cover your fix so that we can ensure that we won't introduce regressions in the future.

@stof
Copy link
Member
stof commented Mar 3, 2016

@be-bewweb the issue is that choices in your case are not true and false but '1' and '', because booleans are not valid array keys and so PHP casts them to strings when building the array.

So IMO, this PR is invalid.
The issue is in your own code, where the actual value is not the one you think.

HeahDude
HeahDude reviewed Mar 3, 2016
View reviewed changes
src/Symfony/Component/Form/Form.php
@@ -344,7 +344,7 @@ public function setData($modelData)
}

// Treat data as strings unless a value transformer exists
if (!$this->config->getViewTransformers() && !$this->config->getModelTransformers() && is_scalar($modelData)) {
if (!$this->config->getViewTransformers() && !$this->config->getModelTransformers() && !is_bool($modelData) && is_scalar($modelData)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

You should replace the scalar check by a string check instead of testing a non-bool.

wrong forget it.

@HeahDude
Copy link
Contributor
HeahDude commented Mar 3, 2016

Hi @be-bewweb, could you please update to 2.7.10 and confirm your issue before moving on with this PR ?

This bug may have been fixed by #17760.

@xabbuh
Copy link
Member
xabbuh commented Mar 31, 2016

ping @be-bewweb :)

@stof
Copy link
Member
stof commented Apr 8, 2016

Closing this as the bug is in the code written by the user. Booleans cannot be used as array keys in PHP, as I said above.

@stof stof closed this Apr 8, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
Bug Form Status: Needs Review Unconfirmed
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@qlombat @javiereguiluz @xabbuh @fabpot @stof @HeahDude @carsonbot
0