[Live] Fix nullable DateTime hydration with empty value #1901
Conversation
| if ('' === $value) { | ||
| return $allowsNull ? null : throw new BadRequestHttpException(sprintf('The model path "%s" was sent invalid date data "%s". Setting empty date value is not allowed, send a non-empty value or make the property nullable.', $propertyPathForError, $value, $dateFormat)) ; | ||
| } | ||
|
|
There was a problem hiding this comment.
We can't do that this is a BC break. If we want to implement this fix we have to add an option to activate or not your fix and to have it not activate by default.
|
I'm pretty sure this would have been fixed with the LiveArg implementation we almost sent in 2.18. But as we realized it has a minor bug, we had to revert it. So I'd prefer that we find a more generic way to fix these cases... or i fear every specific type issue will need specific patches, with less coherence and a lot of similar code in various places. Would you like to have a look at it ? |
|
@1ed did you have time to read the other PR i'd be curious to have your opinion... They may not but they are really due to the same thing Maybe you would have a fresh look / opinion on that matter ? |
|
@smnandre instead of reverting #1694 , we could look at implementing this:
|
|
We had no choice but to revert it, sadly, as it created bugs in rare occasion. i’m 100% open to any solution :) |
are the bugs documented somewhere or could we add tests that cover current behaviour (other than the ones we already had)? I could take out the bugs but I’d have to be aware of the current situation 😅 |
|
@jannes-io No, still this problem with Doctrine / other valueresolvers, but this was enough to not release it like that 😅 |
Kocal
added
Status: Needs Work
Currently when we send an empty string as value, the
new DateTime('')callcreates an object with the current time, which I think is not correct and the
object should be set to null instead.