-
Notifications
You must be signed in to change notification settings - Fork 11.1k
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
[8.x] Fix getDirty method when using AsArrayObject
/ AsCollection
#38869
[8.x] Fix getDirty method when using AsArrayObject
/ AsCollection
#38869
Conversation
@gdebrauwer can you revert all of those unrelated "$value" changes in the other cast files? |
Those changes are necessary, otherwise, the wrong value will be cast. The castable class will just be casting the model's current value instead of the provided value |
@gdebrauwer In what situations would |
When you try to cast the original value, instead of the updated value |
@gdebrauwer is there a way to do this without recasting? Like just |
Yeah that would work. I'll have a look tomorrow and update the PR 👌 (regarding the attributes-array, honestly, I don't really know why that fourth parameter exists 😅) |
@gdebrauwer thanks |
@taylorotwell It now uses the existing |
Are the |
Forgot about those 😅 I reverted those changes. |
Do you know if these changes work with |
Those do not have the same issue, as the encrypted value will be saved in a string column instead of json column |
I think you have to provide a json-encoded string as default value: protected $attributes = [
'settings' => '{"is_public":true,"footer":true}'
]; The error is not caused by this change, because if you use the 'object' cast on that attribute for example, then you will have the same issue. protected $attributes = [
'settings' => [
'is_public' => true,
'footer' => true,
],
];
protected $casts = [
'settings' => 'object'
];
// this setup will also cause the json_decode error The |
@gdebrauwer hmm... Interesting. I can update it to that json string. Also I think the way it was working earlier seems right to me. |
Also as you can see that column name is |
@taylorotwell hey, what do think about my comments? Any thoughts? |
…laravel#38869) * Add failing tests * Fix casts Use $value instead of $attributes[$key] * Fix originalIsEquivalent method * Fix linting issue * Compare json values without recasting * Revert cast changes
A less impactful version of #38774.
I updated the
originalIsEquivalent
method to make a custom comparison when the attribute usesAsArrayObject
orAsCollection
cast. This way, Eloquent will not think there is a difference if there are just some extra spaces in the original json string.