Skip to content
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

[Form] ArrayChoiceList can now deal with a null in choices #17511

Merged
merged 1 commit into from Jan 27, 2016

Conversation

issei-m
Copy link
Contributor

@issei-m issei-m commented Jan 24, 2016

re-create for mistaken #17502


Q A
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets -
License MIT
Doc PR -

Documentation says, since 2.7, choice value is treated as corresponding item value (if choices_as_values option is true). Null as well.

$builder->add('attending', 'choice', array(
    'choices' => array(
        'yes' => true,
        'no' => false,
        'maybe' => null,
    ),
    'choices_as_values' => true,
));

But actually null doesn't work as expected since ArrayChoiceList::getChoicesForValues() uses isset to check whether given value exists in its choices.

It should use array_key_exists instead to do so here.

@fabpot
Copy link
Member

fabpot commented Jan 25, 2016

👍

@fabpot
Copy link
Member

fabpot commented Jan 27, 2016

Thank you @issei-m.

@fabpot fabpot merged commit 68292bb into symfony:2.7 Jan 27, 2016
fabpot added a commit that referenced this pull request Jan 27, 2016
… (issei-m)

This PR was merged into the 2.7 branch.

Discussion
----------

[Form] ArrayChoiceList can now deal with a null in choices

re-create for mistaken #17502

---

| Q             | A
| ------------- | ---
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | -
| License       | MIT
| Doc PR        | -

Documentation says, since 2.7, choice value is treated as corresponding item value (if `choices_as_values` option is true). Null as well.

```php
$builder->add('attending', 'choice', array(
    'choices' => array(
        'yes' => true,
        'no' => false,
        'maybe' => null,
    ),
    'choices_as_values' => true,
));
```

But actually null doesn't work as expected since `ArrayChoiceList::getChoicesForValues()` uses `isset` to check whether given value exists in its choices.

It should use `array_key_exists` instead to do so here.

Commits
-------

68292bb [Form] ArrayChoiceList can now deal with a null in choices
@issei-m issei-m deleted the fix-choice-bug branch January 27, 2016 09:50
@fabpot fabpot mentioned this pull request Feb 3, 2016
fabpot added a commit that referenced this pull request Feb 18, 2016
This PR was merged into the 2.7 branch.

Discussion
----------

[Form] add test for ArrayChoiceList handling null

| Q             | A
| ------------- | ---
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | -
| License       | MIT
| Doc PR        | -

I found this bug but then realised that it was already fixed in #17511.
So this just adds a test.

Commits
-------

00e3819 [Form] add test for ArrayChoiceList handling null
This was referenced Feb 28, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants