Skip to content

[Console] Fix first choice was invalid when using value #14648

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

Merged
merged 1 commit into from
May 20, 2015
Merged

[Console] Fix first choice was invalid when using value #14648

merged 1 commit into from
May 20, 2015

Conversation

ogizanagi
Copy link
Contributor

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

This PR solves the following issues encountered using question helper and choices questions:

  • First choice was not selectable by value.
  • ChoiceList with associative choices with mixed string and int keys has same issue with first choice.
  • Fix inconsistency by always returning values as strings.

First point exemple:
screenshot 2015-05-15 a 17 16 12

Last two points are mainly edge cases. Indeed, if a QuestionChoice has something like :

array(
    '0' => 'No environment',
    '1' => 'My environment 1',
    'env_2' => 'My environment 2',
    3 => 'My environment 3',
);

as choices, you will not be able to select the first choice and get an InvalidArgumentException:

There were 2 errors:

1) Symfony\Component\Console\Tests\Helper\QuestionHelperTest::testChoiceFromChoicelistWithMixedKeys with data set #0 ('0', '0')
InvalidArgumentException: Value "0" is invalid

2) Symfony\Component\Console\Tests\Helper\QuestionHelperTest::testChoiceFromChoicelistWithMixedKeys with data set #1 ('No environment', '0')
InvalidArgumentException: Value "No environment" is invalid

Moreover, even if you were able to select by value (No environment), you'll get an integer instead of a string:

Failed asserting that '0' is identical to 0.

For more consistency, the solution is to always return a string.

The issue does not exist in 2.6, as the QuestionChoice::getDefaultValidator handled things differently.

Using question helper and choices questions:
- first choice was not selectable by value.
- ChoiceList with associative choices with mixed string and int keys has same issue with first choice. Add test for all choices.
- Fix inconsistency by always returning values as strings
@fabpot
Copy link
Member

fabpot commented May 20, 2015

Thank you @ogizanagi.

@fabpot fabpot merged commit 03e4ab6 into symfony:2.7 May 20, 2015
fabpot added a commit that referenced this pull request May 20, 2015
…gizanagi)

This PR was merged into the 2.7 branch.

Discussion
----------

[Console] Fix first choice was invalid when using value

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

This PR solves the following issues encountered using question helper and choices questions:
- First choice was not selectable by value.
- ChoiceList with associative choices with mixed string and int keys has same issue with first choice.
- Fix inconsistency by always returning values as strings.

First point exemple:
![screenshot 2015-05-15 a 17 16 12](https://cloud.githubusercontent.com/assets/2211145/7655757/3344b39a-fb26-11e4-9fe7-0775616619bf.PNG)

Last two points are mainly edge cases. Indeed, if a QuestionChoice has something like :
```php
array(
    '0' => 'No environment',
    '1' => 'My environment 1',
    'env_2' => 'My environment 2',
    3 => 'My environment 3',
);
```
as choices, you will not be able to select the first choice and get an `InvalidArgumentException`:

```
There were 2 errors:

1) Symfony\Component\Console\Tests\Helper\QuestionHelperTest::testChoiceFromChoicelistWithMixedKeys with data set #0 ('0', '0')
InvalidArgumentException: Value "0" is invalid

2) Symfony\Component\Console\Tests\Helper\QuestionHelperTest::testChoiceFromChoicelistWithMixedKeys with data set #1 ('No environment', '0')
InvalidArgumentException: Value "No environment" is invalid
```

Moreover, even if you were able to select by value (`No environment`), you'll get an integer instead of a string:
```
Failed asserting that '0' is identical to 0.
```
For more consistency, the solution is to always return a string.

The issue does not exist in 2.6, as the `QuestionChoice::getDefaultValidator` handled things differently.

Commits
-------

03e4ab6 [Console] Fix first choice was invalid when using value
@ogizanagi ogizanagi deleted the console_choices_2_7 branch May 20, 2015 08:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants