Skip to content

Fixed a problem with Eloquent::get_dirty #1201

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

Merged
merged 2 commits into from
Sep 26, 2012
Merged

Conversation

JoostK
Copy link
Contributor

@JoostK JoostK commented Sep 9, 2012

When you had a synched Eloquent model (e.g. without changed values) but
one of those values is null, then that value would be considered as
dirty. Eloquent::changed returns false, but the value is present in
Eloquent::get_dirty.

This fix makes sure that a null value in $attributes is only
present in get_dirty when it wasn't at all set in $original.

When you had a synched Eloquent model (e.g. without changed values) but
one of those values is `null`, then that value would be considered as
dirty. `Eloquent::changed` returns false, but the value is present in
`Eloquent::get_dirty`.

This fix makes sure that a `null` value in `$attributes` is only
present in `get_dirty` when it wasn't at all *set* in `$original`.
@franzliedke
Copy link
Contributor

Indeed. I had already fixed this in pull request #901, but Taylor reverted a little bit too much in v3.2.5. :)

@JoostK
Copy link
Contributor Author

JoostK commented Sep 9, 2012

I remember having seen that pull request before, but didn't think of it when I just experienced the problem.

Here's the revert: 1203473

@JoostK
Copy link
Contributor Author

JoostK commented Sep 9, 2012

I'll spend the evening (I live in The Netherlands) writing some Eloquent unit tests to avoid these issues

Basic Eloquent unit tests.

Lacks database operations, relationship testing and some magic method
calls.
@taylorotwell
Copy link
Member

Thanks!

taylorotwell added a commit that referenced this pull request Sep 26, 2012
Fixed a problem with `Eloquent::get_dirty`
@taylorotwell taylorotwell merged commit fc9b0e1 into laravel:develop Sep 26, 2012
@RuslanGetmansky
Copy link

It seems the problem was fixed not completely. If some tinyint DB field has NULL as default value and then it has been changed to 0 then Model::get_dirty() doesn't consider that as a change. It thinks that null == 0 and the change isn't saved by Model::save().

@JoostK
Copy link
Contributor Author

JoostK commented Apr 3, 2013

I guess you're right that !== should be used.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants