Input fallback_value option limitation/feature suggestion

[svycka]

Would be nice if possible to use fallback value on optional fields only when input not set. Current behavior is when input is not set or validation fails, but I would like to fail validation if input is not valid and use fallback only when input is not set at all. With current behavior is impossible to check if data is invalid.

example:

// create $inputFilter with 'optional_field' input and 'fallback' as fallback value

// when input is not set
$inputFilter->setData([]);
$inputFilter->isValid(); // return true
$inputFilter->getValue('optional_field'); // return 'fallback'

// when input is set but not valid
$inputFilter->setData(['optional_field' => 'invalid']);
$inputFilter->isValid(); // return false

This is not possible at the moment, but very important scenario for me. I think after #25 when required option works as 'is set' and not as 'not empty' this would make more sense. no?

I think having fallback on invalid data is bad idea. Also if data should be modified somehow to be valid there is filters for this.

[larsnystrom]

A fallback_if_invalid setting would fix this. If it defaulted to true there would be no BC break. It would also be pretty easy to implement.

[weierophinney]

It would also be pretty easy to implement.

Considering the dozens of hours spent with the most recent releases to build a test matrix for the current combination of options, I beg to differ. Any new options introduced at this point need to be very carefully evaluated, as they add more permutations to the test matrix: we already are testing something like 32 combinations of options; adding another would double that to 64.

[larsnystrom]

Considering the dozens of hours spent with the most recent releases to build a test matrix for the current combination of options, I beg to differ. Any new options introduced at this point need to be very carefully evaluated, as they add more permutations to the test matrix: we already are testing something like 32 combinations of options; adding another would double that to 64.

In that case, maybe we could remove the "fallback if invalid" behaviour in 3.0?

[svycka]

I think the name is not correct for this behavior I am thinking about default_value or something this should be used only if value not set, otherwise should be used fallback if set. this also would solve BC problem but also have big test matrix problem. But well if this is good feature and we want it in 3.0 we can do it now and remove whole test matrix in 3.0 when other options will be removed.

[Maks3w]

The behavior can be do with very little effort by extending Input with your custom class.

Personally I prefer don't add implementations are not required by the Input interface and just provide stable and tested implementations.

Anyway, feel free to send a PR with the desired implementation and I'll reconsider my opinion if the implementation is simple and bugfree

Hint:

zend-inputfilter/test/InputTest.php

Lines 581 to 587 in a05fa0b

	return [
	// Description => [$inputIsRequired, $fallbackValue, $originalValue, $isValid, $expectedValue]
	'Required: T, Input: Invalid. getValue: fallback' => [ $required, $fallbackValue, $originalValue, !$isValid, $fallbackValue],
	'Required: T, Input: Valid. getValue: original' => [ $required, $fallbackValue, $originalValue, $isValid, $originalValue],
	'Required: F, Input: Invalid. getValue: fallback' => [!$required, $fallbackValue, $originalValue, !$isValid, $fallbackValue],
	'Required: F, Input: Valid. getValue: original' => [!$required, $fallbackValue, $originalValue, $isValid, $originalValue],
	];

zend-inputfilter/test/BaseInputFilterTest.php

Lines 473 to 506 in a05fa0b

	public function testValidationSkipsFieldsMarkedNotRequiredWhenNoDataPresent()
	{
	$filter = $this->inputFilter;
	$optionalInputName = 'fooOptionalInput';
	/** @var InputInterface|MockObject $optionalInput */
	$optionalInput = $this->getMock(InputInterface::class);
	$optionalInput->method('getName')
	->willReturn($optionalInputName)
	;
	$optionalInput->expects($this->never())
	->method('isValid')
	;
	$data = [];
	$filter->add($optionalInput);
	$filter->setData($data);
	$this->assertTrue(
	$filter->isValid(),
	'isValid() value not match. Detail . ' . json_encode($filter->getMessages())
	);
	$this->assertArrayNotHasKey(
	$optionalInputName,
	$filter->getValidInput(),
	'Missing optional fields must not appear as valid input neither invalid input'
	);
	$this->assertArrayNotHasKey(
	$optionalInputName,
	$filter->getInvalidInput(),
	'Missing optional fields must not appear as valid input neither invalid input'
	);
	}

[svycka]

@Maks3w what do you think if we make this default behavior in ZF3?
I think it is more logical to add fallback(default value) if value isn't set, but not when value is invalid(I would want error here). Or support both, but maybe that would be to complicated.

This would be BC but for 3.0 this is acceptable.

[vaclavvanik]

@svycka +1 I use same behavior especially in grid controllers

[Maks3w]

@svycka I don't consider replace a behavior an option. Instead I suggest send a PR where two behaviors could be used (one at time) probably setting a second parameter to setFallbackValue with an optional second argument where the default value is the current behavior.

[Maks3w]

Closed due inactivity

[Erikvv]

Ran into this limitation today made me pull my hair out. Fallback values which just work in the empty case would be very much appreciated.

W.r.t. complexity in the combination of flags: I think the solution is to split the Input class to multiple classes. Some combinations of options don't make sense or are very niche and you avoid that entirely that way.