-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
cast integer to string in case date time format is „U“ #50272
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
In case the date time format is „U“, the data comes as a integer (timestamp in seconds). To keep the „data is string“-condition, the integer is casted to a string.
Hey! I see that this is your first PR. That is great! Welcome! Symfony has a contribution guide which I suggest you to read. In short:
Review the GitHub status checks of your pull request and try to solve the reported issues. If some tests are failing, try to see if they are failing because of this change. When two Symfony core team members approve this change, it will be merged and you will become an official Symfony contributor! I am going to sit back now and wait for the reviews. Cheers! Carsonbot |
Hey! Thanks for your PR. You are targeting branch "6.3" but it seems your PR description refers to branch "6.3 (?)". Cheers! Carsonbot |
I'd rather change the type check to also allow integers when using |
And this needs to have tests covering it to prevent regressions |
We might want to account for |
Thank you for the feedback, @stof and @nicolas-grekas. Is it the better solution to allow integers and floats for
With PHP 8.1 following code works just fine (does not return false)
but could this behavior change in the future? |
@PercyDwn My guess is that this code would not work fine with strict types. |
IMO, yes |
$data can come as an integer, when date time format is „U“. Updated type check, so in that case no exception is thrown.
Okay so I updated the type check to allow integers when dateTimeFormat is "U". Some of the github tests are failing, but I am not quite sure why. |
Fixed by #50251 |
In case the date time format is „U“, the data comes as a integer (timestamp in seconds). To keep the „data is string“-condition, the integer is casted to a string.
Fixes DateTimeNormalizer::denormalize throwing a NotNormalizableValueException when DateTimeFormat is "U" and data is passed as an integer.
The Exception should not be thrown, as data being an integer is valid for a timestamp.
My solution is to check if $dateTImeFormat is "U" and $data is an integer. In that case the integer is casted to a string.
This way, the condition that $data is a string does not need to be changed.