[Form] Fixed set "empty_data" option with empty string to return empty string not null #10126
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| @@ -43,10 +43,22 @@ public function testSubmitAddsNoDefaultProtocolIfEmpty() | |||
|
|
|||
| $form->submit(''); | |||
|
|
|||
| $this->assertNull($form->getData()); | |||
| $this->assertSame('',$form->getData()); // the data given was empty string so we should compare it to empty | |||
There was a problem hiding this comment.
assertEmpty will allow a lot of values, while we really want to make sure here that the outcome is an empty string
|
Thank you for your work on this! I added the new test for UrlType to #10647. I'm afraid we can't change the return type of the UrlType when an empty string is submitted. Currently, the convention is to always convert empty strings to null. The solution is to implement #5906 properly, but that's not that trivial. :( |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I see no reason to check here the empty string value and return this with null value. This is place where the param was manualy requested by the user with the "empty_value" option so when he given it, he must have a reason to force string not null, so if he want to return empty string there is no reason to convert it to null value.
I see also no changes that will be done in the future flow with the methods normToModel and normToView.
So I'm pretty sure that resolves that issue.