8000 [Form] ChoiceType - empty choices array not allowed · Issue #3298 · symfony/symfony · GitHub
[go: up one dir, main page]

Skip to content
8000

[Form] ChoiceType - empty choices array not allowed #3298

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
chmielot opened this issue Feb 7, 2012 · 0 comments
Closed

[Form] ChoiceType - empty choices array not allowed #3298

chmielot opened this issue Feb 7, 2012 · 0 comments

Comments

@chmielot
Copy link
chmielot commented Feb 7, 2012

The documentation about ChoiceType sais, that default for 'choices' is array().
This is not allowed because an empty array evaluates to false:

if (!$options['choice_list'] && !$options['choices']) {

Use case: choices are empty and items are added dynamically.

@chmielot chmielot closed this as completed Feb 7, 2012
@chmielot chmielot reopened this Feb 7, 2012
@chmielot chmielot closed this as completed Feb 7, 2012
Gregwar pushed a commit to Gregwar/symfony that referenced this issue Nov 29, 2013
Commits
-------

dbaddbb [Form] Allow empty choices array for ChoiceType

Discussion
----------

[Form] Allow empty choices array for ChoiceType

Bug fix: yes
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes

The documentation about ChoiceType says, that default for 'choices' is array().
This is not allowed because an empty array evaluates to false:

if (!$options['choice_list'] && !$options['choices']) {

Use case: choices are empty and items are added dynamically.

---------------------------------------------------------------------------

by chmielot at 2012-02-07T21:03:03Z

Sorry, I messed up with the tickets. Didn't know a pull request opens a ticket.

---------------------------------------------------------------------------

by bschussek at 2012-02-08T08:23:29Z

This ticket depends on symfony#3290 for being merged.

Apart from this, a test case is missing. Add this to ChoiceTypeTest:

    // symfony#3298
    public function testInitializeWithEmptyChoices()
    {
        $this->factory->createNamed('choice', 'name', null, array(
            'choices' => array(),
        ));
    }

---------------------------------------------------------------------------

by craue at 2012-02-10T20:32:44Z

@bschussek: Why does it depend on symfony#3290? I have issues with the `!$options['choices']` check even without symfony#3290 applied.

---------------------------------------------------------------------------

by chmielot at 2012-02-10T21:24:28Z

Ok, I updated the branch with the test case and craue's suggestion on explicitly checking valid values.

---------------------------------------------------------------------------

by chmielot at 2012-02-11T09:07:05Z

Should be fine now.

---------------------------------------------------------------------------

by bschussek at 2012-02-11T09:15:10Z

Good. I see that you added a check for \Traversable too - can you add a test for this too? It should be fine to pass an empty \ArrayObject().

---------------------------------------------------------------------------

by craue at 2012-02-11T10:05:36Z

@bschussek: But even if there's passed something different than an array, the c'tor of `SimpleChoiceList` (which is called in line 45) is type hinted with `array` and won't allow passing a `Traversable` anyway, right?

---------------------------------------------------------------------------

by bschussek at 2012-02-11T10:16:36Z

@craue Yes. But in EntityType the choices option is reused and passed to EntityChoiceList, which extends ChoiceList and accepts any array or Traversable.

You're right though that it's not possible to add a test covering this in ChoiceTypeTest, and in EntityTypeTest this is already covered.

@fabpot Ready to merge.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant
0