Missing required field with fallback does not return input in getValidInput #22
Conversation
|
If it was not present in the submitted data, and is considered optional, it should not be in the returned values, unless it has a fallback value. In the case of the "valid inputs" set, even though it is considered valid, it was never actually validated as it was not submitted. Further, changing the behavior now could lead to a new category of upgrade issues, as developers have been currently working on the assumption that the valid input set will not return inputs that were not submitted. |
|
Evidently, what I stated above was the actual intent of the PR. |
| @@ -1069,6 +1069,8 @@ public function testMissingRequiredThatAllowsEmptyWithFallbackShouldMarkInputVal | |||
| $data = $filter->getValues(); | |||
| $this->assertArrayHasKey('bar', $data); | |||
| $this->assertEquals($bar->getFallbackValue(), $data['bar']); | |||
| $this->assertArrayHasKey('bar', $filter->getValidInput()); | |||
| $this->assertArrayNotHasKey('bar', $filter->getInvalidInput()); | |||
There was a problem hiding this comment.
both of these should be asserting that the key is not present.
There was a problem hiding this comment.
But bar has a fallback value. If not then assertTrue(isValid) should be false
|
I've pulled this locally and made the change as noted above (in the first test, indicating that the input is in neither the list of valid or invalid inputs); all tests pass. I'll merge with that change. |
…mark-input-as-valid Missing required field with fallback does not return input in getValidInput
Maks3w
deleted the
hotfix/required-fallback-does-not-mark-input-as-valid
branch
Test method says
testMissingRequiredThatAllowsEmptyWithFallbackShouldMarkInputValidbut don't assert input is returned ingetValidInputShould the input to be returned in
getValidInputcollection?IMO it should because has a valid status.
Test was introduced in #10