Skip to content
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

Boolean always get an entry in history... #16

Closed
arenowebdev opened this issue Jun 14, 2013 · 7 comments
Closed

Boolean always get an entry in history... #16

arenowebdev opened this issue Jun 14, 2013 · 7 comments

Comments

@arenowebdev
Copy link

If I submit a form with a single string change on it, no matter what the boolean field value is set to (even if it's the same), I always gets an entry in the revisions table. See below. Am I just doing something wrong, or did I stumble across a bug? At first I thought it was a false vs. 0 issue, but even when I change the value to 0 when setting the boolean flag, I still see the result in the database as below. Thanks!!

id          revisionable_type  revisionable_id  user_id     key         old_value      new_value          created_at           updated_at         
----------  -----------------  ---------------  ----------  ----------  -------------  -----------------  -------------------  -------------------
1           Thing              1                2           name        String         Stringtest         2013-06-13 21:32:58  2013-06-13 21:32:58
2           Thing              1                2           disabled    0              0                  2013-06-13 21:32:58  2013-06-13 21:32:58
3           Thing              1                2           name        Strintgtest    String             2013-06-13 21:33:24  2013-06-13 21:33:24
4           Thing              1                2           disabled    0              0                  2013-06-13 21:33:24  2013-06-13 21:33:24
@duellsy
Copy link
Member

duellsy commented Jun 14, 2013

Hmmm interesting, if there's any issues in determining which fields have been updated and which are unchanged, it points to a possible bug in Eloquents getDirty() function: https://github.com/laravel/framework/blob/master/src/Illuminate/Database/Eloquent/Model.php#L2162

In particular, the following line is equating to true:

if ( ! array_key_exists($key, $this->original) or $value != $this->original[$key])

Since the key would exist in the original array, it seems as though $value != $this->original[$key] is true...

As a test, perhaps do a var_dump of the object as it currently exists, and then also directly before saving it, and compare the value on initial load and then before saving

@arenowebdev
Copy link
Author

Yep...that is the problem. http://paste.laravel.com/wEV

Looks like it's being cast to an integer where when it's in the database it is a string. I just confirmed that the disabled field is a boolean in the migration $table->boolean('disabled')->default(0);...

Not sure if you typo'd it or not, but in my version of Laravel that line is:
if ( ! array_key_exists($key, $this->original) or $value !== $this->original[$key])
(notice the double equal...). Just to be certain, I changed mine to a single equal and it still has the same problem. Hmmm...time to file a bug on the Laravel repo? Do you experience the same issue on boolean fields?

@duellsy
Copy link
Member

duellsy commented Jun 14, 2013

I copied that line from the Laravel project in it's latest form on github.

Might be worth opening an issue in Laravel to confirm that it is correct behaviour... if they feel that booleans should continue to be cast to an int when pulled from the database, then we'll have to adhere to that and make a fix to cater for it.

@duellsy
Copy link
Member

duellsy commented Jun 14, 2013

Oh actually... my apologies, the latest Laravel has !==, I was looking at an older version. This more explicit comparison wouldn't be helping, since in theory
true != 0 would return a fail

Seems it was updated in Laravel about a month back laravel/framework@e247073

@arenowebdev
Copy link
Author

Found the fix here after asking in IRC. :P

laravel/framework#1429 (comment)

@duellsy
Copy link
Member

duellsy commented Jun 14, 2013

Awesome, nice find. If enough other people have this issue, then we'll look at putting this in revisionable core

@arenowebdev
Copy link
Author

I can't seem to get this to work, unfortunately. Even when I have the getAttribute in my base model, it never gets called on my boolean fields if they are changing. Any ideas? I think I'm in need of tutoring... 😄

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

No branches or pull requests

2 participants