Skip to content

Commit

Permalink
Handle selects (etc) with numeric keys (#4688)
Browse files Browse the repository at this point in the history
  • Loading branch information
@jasonvarga
jasonvarga authored Nov 9, 2021
1 parent 4d0588a commit e5bd278
Show file tree
Hide file tree
Showing 3 changed files with 130 additions and 1 deletion.
21 changes: 20 additions & 1 deletion src/Fieldtypes/HasSelectOptions.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ public function augment($value)
if ($this->multiple()) {
return collect($value)->map(function ($value) {
return [
'key' => $value,
'key' => $value = $this->normalizeAugmentableValue($value),
'value' => $value,
'label' => $this->getLabel($value),
];
Expand All @@ -74,9 +74,28 @@ public function augment($value)

throw_if(is_array($value), new MultipleValuesEncounteredException($this));

$value = $this->normalizeAugmentableValue($value);

return new LabeledValue($value, $this->getLabel($value));
}

private function normalizeAugmentableValue($value)
{
if (! is_numeric($value)) {
return $value;
}

if ($value == (int) $value) {
return (int) $value;
}

if ($value == (float) $value) {
return (string) $value;
}

return $value;
}

private function getLabel($actualValue)
{
$value = $actualValue;
Expand Down
59 changes: 59 additions & 0 deletions tests/Fieldtypes/LabeledValueTests.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,65 @@ public function it_augments_to_a_LabeledValue_object_with_options_with_keys()
$this->assertEquals('missing', $augmented->label());
}

/** @test */
public function it_augments_to_a_LabeledValue_object_with_options_with_numeric_keys()
{
$field = $this->field([
'type' => 'select',
'options' => [
1 => 'Australia',
2 => 'Canada',
'2.5' => 'Canada and a half',
3 => 'USA',
],
]);

$augmented = $field->augment(2);
$this->assertInstanceOf(LabeledValue::class, $augmented);
$this->assertEquals(2, $augmented->value());
$this->assertEquals('Canada', $augmented->label());

// Javascript converts numeric keys to strings. Thanks.
// People will end up with string in their data. We should treat it like a number though.
$augmented = $field->augment('2');
$this->assertInstanceOf(LabeledValue::class, $augmented);
$this->assertEquals(2, $augmented->value());
$this->assertEquals('Canada', $augmented->label());

// Just a sanity check that floats aren't converted to ints.
// You can't have a float as a key in an array.
$augmented = $field->augment(2.5);
$this->assertInstanceOf(LabeledValue::class, $augmented);
$this->assertEquals('2.5', $augmented->value());
$this->assertEquals('Canada and a half', $augmented->label());

// and again for the string version
$augmented = $field->augment('2.5');
$this->assertInstanceOf(LabeledValue::class, $augmented);
$this->assertEquals('2.5', $augmented->value());
$this->assertEquals('Canada and a half', $augmented->label());

$augmented = $field->augment(null);
$this->assertInstanceOf(LabeledValue::class, $augmented);
$this->assertNull($augmented->value());
$this->assertNull($augmented->label());

$augmented = $field->augment(false);
$this->assertInstanceOf(LabeledValue::class, $augmented);
$this->assertFalse($augmented->value());
$this->assertFalse($augmented->label());

$augmented = $field->augment(true);
$this->assertInstanceOf(LabeledValue::class, $augmented);
$this->assertTrue($augmented->value());
$this->assertTrue($augmented->label());

$augmented = $field->augment('missing');
$this->assertInstanceOf(LabeledValue::class, $augmented);
$this->assertEquals('missing', $augmented->value());
$this->assertEquals('missing', $augmented->label());
}

/** @test */
public function it_augments_to_a_LabeledValue_object_with_options_without_keys()
{
Expand Down
51 changes: 51 additions & 0 deletions tests/Fieldtypes/MultipleLabeledValueTests.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,57 @@ public function it_augments_multiple_enabled_to_an_array_of_LabeledValue_equival
], $field->augment(['au', 'us', null, false, true, 'missing']));
}

/** @test */
public function it_augments_multiple_enabled_to_an_array_of_LabeledValue_equivalents_with_numeric_keys()
{
$field = $this->field([
'type' => 'select',
'multiple' => true,
'options' => [
1 => 'Australia',
2 => 'Canada',
3 => 'USA',
],
]);

$this->assertEquals([
['key' => 2, 'value' => 2, 'label' => 'Canada'],
['key' => null, 'value' => null, 'label' => null],
['key' => false, 'value' => false, 'label' => false],
['key' => true, 'value' => true, 'label' => true],
['key' => 'missing', 'value' => 'missing', 'label' => 'missing'],
], $field->augment([2, null, false, true, 'missing']));

$this->assertEquals([
['key' => 2, 'value' => 2, 'label' => 'Canada'],
], $field->augment(['2']));

$this->assertEquals([
['key' => '2.5', 'value' => '2.5', 'label' => '2.5'],
], $field->augment(['2.5']));
}

/** @test */
public function it_augments_multiple_enabled_to_an_array_of_LabeledValue_equivalents_with_floats_for_keys()
{
$field = $this->field([
'type' => 'select',
'multiple' => true,
'options' => [
'1.5' => 'One point five',
'2.5' => 'Two point five',
],
]);

$this->assertEquals([
['key' => '2.5', 'value' => '2.5', 'label' => 'Two point five'],
], $field->augment(['2.5']));

$this->assertEquals([
['key' => '2.5', 'value' => '2.5', 'label' => 'Two point five'],
], $field->augment([2.5]));
}

/** @test */
public function it_augments_multiple_enabled_to_an_array_of_LabeledValue_equivalents_with_boolean_casting()
{
Expand Down

2 comments on commit e5bd278

@ebeauchamps
Copy link
Contributor

Choose a reason for hiding this comment

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

Hello @jasonvarga .
In the test, shouldn't you test for "2,5" also, the comma being the common way of representing the digital separator in some/many countries?
Thanks.

@jasonvarga
Copy link
Member Author

Choose a reason for hiding this comment

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

Doesn't really matter, it's not going to be a number anyway. It's a string.

Please sign in to comment.