bug #14648 [Console] Fix first choice was invalid when using value (ogizanagi)
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
This commit is contained in:
commit
efd68e6b71
@ -149,19 +149,19 @@ class ChoiceQuestion extends Question
|
|||||||
$result = array_search($value, $choices);
|
$result = array_search($value, $choices);
|
||||||
|
|
||||||
if (!$isAssoc) {
|
if (!$isAssoc) {
|
||||||
if (!empty($result)) {
|
if (false !== $result) {
|
||||||
$result = $choices[$result];
|
$result = $choices[$result];
|
||||||
} elseif (isset($choices[$value])) {
|
} elseif (isset($choices[$value])) {
|
||||||
$result = $choices[$value];
|
$result = $choices[$value];
|
||||||
}
|
}
|
||||||
} elseif (empty($result) && array_key_exists($value, $choices)) {
|
} elseif (false === $result && isset($choices[$value])) {
|
||||||
$result = $value;
|
$result = $value;
|
||||||
}
|
}
|
||||||
|
|
||||||
if (empty($result)) {
|
if (false === $result) {
|
||||||
throw new \InvalidArgumentException(sprintf($errorMessage, $value));
|
throw new \InvalidArgumentException(sprintf($errorMessage, $value));
|
||||||
}
|
}
|
||||||
array_push($multiselectChoices, $result);
|
array_push($multiselectChoices, (string) $result);
|
||||||
}
|
}
|
||||||
|
|
||||||
if ($multiselect) {
|
if ($multiselect) {
|
||||||
|
@ -210,6 +210,76 @@ class QuestionHelperTest extends \PHPUnit_Framework_TestCase
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* @dataProvider simpleAnswerProvider
|
||||||
|
*/
|
||||||
|
public function testSelectChoiceFromSimpleChoices($providedAnswer, $expectedValue)
|
||||||
|
{
|
||||||
|
$possibleChoices = array(
|
||||||
|
'My environment 1',
|
||||||
|
'My environment 2',
|
||||||
|
'My environment 3',
|
||||||
|
);
|
||||||
|
|
||||||
|
$dialog = new QuestionHelper();
|
||||||
|
$dialog->setInputStream($this->getInputStream($providedAnswer."\n"));
|
||||||
|
$helperSet = new HelperSet(array(new FormatterHelper()));
|
||||||
|
$dialog->setHelperSet($helperSet);
|
||||||
|
|
||||||
|
$question = new ChoiceQuestion('Please select the environment to load', $possibleChoices);
|
||||||
|
$answer = $dialog->ask($this->createInputInterfaceMock(), $this->createOutputInterface(), $question);
|
||||||
|
|
||||||
|
$this->assertSame($expectedValue, $answer);
|
||||||
|
}
|
||||||
|
|
||||||
|
public function simpleAnswerProvider()
|
||||||
|
{
|
||||||
|
return array(
|
||||||
|
array(0, 'My environment 1'),
|
||||||
|
array(1, 'My environment 2'),
|
||||||
|
array(2, 'My environment 3'),
|
||||||
|
array('My environment 1', 'My environment 1'),
|
||||||
|
array('My environment 2', 'My environment 2'),
|
||||||
|
array('My environment 3', 'My environment 3'),
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* @dataProvider mixedKeysChoiceListAnswerProvider
|
||||||
|
*/
|
||||||
|
public function testChoiceFromChoicelistWithMixedKeys($providedAnswer, $expectedValue)
|
||||||
|
{
|
||||||
|
$possibleChoices = array(
|
||||||
|
'0' => 'No environment',
|
||||||
|
'1' => 'My environment 1',
|
||||||
|
'env_2' => 'My environment 2',
|
||||||
|
3 => 'My environment 3',
|
||||||
|
);
|
||||||
|
|
||||||
|
$dialog = new QuestionHelper();
|
||||||
|
$dialog->setInputStream($this->getInputStream($providedAnswer."\n"));
|
||||||
|
$helperSet = new HelperSet(array(new FormatterHelper()));
|
||||||
|
$dialog->setHelperSet($helperSet);
|
||||||
|
|
||||||
|
$question = new ChoiceQuestion('Please select the environment to load', $possibleChoices);
|
||||||
|
$question->setMaxAttempts(1);
|
||||||
|
$answer = $dialog->ask($this->createInputInterfaceMock(), $this->createOutputInterface(), $question);
|
||||||
|
|
||||||
|
$this->assertSame($expectedValue, $answer);
|
||||||
|
}
|
||||||
|
|
||||||
|
public function mixedKeysChoiceListAnswerProvider()
|
||||||
|
{
|
||||||
|
return array(
|
||||||
|
array('0', '0'),
|
||||||
|
array('No environment', '0'),
|
||||||
|
array('1', '1'),
|
||||||
|
array('env_2', 'env_2'),
|
||||||
|
array(3, '3'),
|
||||||
|
array('My environment 1', '1'),
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* @dataProvider answerProvider
|
* @dataProvider answerProvider
|
||||||
*/
|
*/
|
||||||
|
Reference in New Issue
Block a user