-
Notifications
You must be signed in to change notification settings - Fork 11.2k
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
result of serializeDate is being parsed but shouldn't be #20116
Comments
This only happens if "updated_at" / "created_at" are listed in $dates -- shouldn't this be allowed? |
Because the function expects the value being passed should be an instance of |
That doesn't make a lot of sense -- if it takes a DateTime and returns a DateTime then what is the function supposed to do? Also, why does it work fine if updated_at and created_at are NOT listed in $dates? |
By adding an attribute to the $dates array, you're telling Eloquent that you want it to be mutated to a Carbon date. You must start with a format that can be converted to Carbon and you end with a Carbon instance. If you want to have something other than a datish format, you shouldn't add it to the $dates array. |
Sorry, I must be missing something. I thought "serializeDate" would control how a DateTimeInterface would get converted into JSON or array format. If that is correct, then I don't understand how its return value should matter. If that is incorrect, then what exactly is serializeDate meant to do? |
Seems like Laravel should have two functions, one that handles input from an arbitrary object and converts to a DateTimeInterface object, and another that takes a DateTimeInterface and converts to an arbitrary object for inclusion in serialized arrays/JSON. No? |
Return a correct date format and you're good to go. Closing since it's not a bug, but feel free to continue the discussion if you want. |
Yes, please help me understand. I thought the purpose of that function was only to determine how a date looks in an array or in JSON. If not that, what is the purpose? I think there's a bug here -- either the function purpose is improperly documented, or the function is being invoked incorrectly, or something is amiss with how created_at and updated_at are handled. |
Please explain why returning "zar" causes no problem if updated_at and created_at are not in $dates but other variables are? If the result of the function must be a DateTime or date-ish, then why is there no problem with it returning an arbitrary string in most cases? |
Best way to see what is happening is go to Illuminate\Database\Eloquent\Concerns\HasAttributes and see exactly how it works. Using $dates, calls "asDate" which calls "asDateTime". These are located in the same class(HasAttributes) as "serializeDate". |
I'm sure the code explains the trouble I'm experiencing. But it doesn't tell me if the code does what is intended. Can someone please explain what "serializeDate" is supposed to do if it's not to format a DateTime to the user's preferred JSON/array serialized format? |
I looked at the code and am pretty sure there's a bug here. Here's what I think is happening: If you add updated_at or created_at into $dates, then it ends up being there twice because array_merge will allow duplicates in. Now that it's in $dates twice, it will be processed twice. The first time serializeDate is called, it will replace the Carbon with whatever it returns (such as a string). The second time it runs, there'll be an error because the string can't be converted to a DateTime. So I believe I am correct that the ouput of serializeDate isn't supposed to matter (and according to the source-code, the result is supposed to be a string). The result is only mattering because it's being processed twice. |
In short, it's an easy fix if we just make sure we don't allow duplicates when merging created_at and updated_at. Specifically, the problem is here:
|
Can this please be re-opened? I I found a bug and detailed the cause of it. |
Don't include them, they're handled by default. |
It's extremely counter-intuitive behavior that if you put created_at or updated_at in $dates that serializeDate will stop working. Why is there so much resistance to designating this a bug? |
@dsandber because I don't see a bug, it's just how it works, can you please share a real life scenario that you aren't able to accomplish because of the current behaviour? What are you trying to do? |
@themsaid If a patient went to a doctor and told them "When I stand-up, I have chest pains" and the doctor said "Then don't stand up" I would say the doctor is treating the symptom rather than the root cause of the issue. Similarly, the root cause of this issue is that updated_at and created_at are merged twice in the getDates() function, and I am trying to help fix that. I am not having any trouble with my code, but rather am trying to improve Laravel so that the next person doesn't have to spend hours on this issue. As an example, here's another person who had to wrestle with this thing that can be fixed in five minutes: https://laracasts.com/discuss/channels/laravel/formatting-dates-in-jsonapi-response-iso-8601-format If putting 'created_at' or 'updated_at' in $dates is an error, then Laravel should give an error. If it is not an error, then things should work properly. The currently behavior of serializeDate breaking when $dates included 'created_at' is counter-intuitive behavior and as such qualifies as a bug. |
Here you go #20193 |
Wonderful, thank you!!! |
mssql - github -- microsoft/mssql-server-linux
Description:
My understanding is that I can introduce my own way of serializing data by overriding serializeDate like this in a model:
However, when I do this the "foo" tries to get parsed as a date:
Steps To Reproduce:
Just override serializeDate as above. Why is the result of my serializeDate trying to be parsed as a date/time?
The text was updated successfully, but these errors were encountered: