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

Changing logic of isDirty to match with Laravel 5.5 and prevent from always returning false #18

Merged
merged 3 commits into from
Nov 17, 2017

Conversation

EricTendian
Copy link
Contributor

@EricTendian EricTendian commented Oct 16, 2017

This fixes #15.

This PR changes the if statement in isDirty so that an empty array will be passed in when no arguments are specified, so hasChanges works properly. I have tested the changes and they fix the issue and allow model update operations to function again.

See laravel/framework#20130 for what was changed in Laravel 5.5 as it relates to isDirty. We now need to pass in an array instead of allowing null as hasChanges() in Illuminate\Database\Eloquent\Concerns\HasAttributes is checking for an empty array instead of null. This package's isDirty was effectively doing parent::isDirty(null); with the default argument, so func_get_args() inside Laravel's isDirty was returning [null] and screwing everything up.

…properly

See laravel/framework#20130 for what was
changed in Laravel 5.5. We now need to use an empty array instead
of null since the method hasChanges() in Illuminate\Database\Eloquent\Concerns\HasAttributes
is checking for an empty array instead of null.
@daverdalas
Copy link

daverdalas commented Oct 19, 2017

@EricTendian Would not it be better to leave default value like it is in HasAttributes trait and and use that code instead?

   /**
     * Register hook for isDirty.
     *
     * @param null $attributes
     * @return bool
     */
    public function isDirty($attributes = null)
    {
        if (! is_array($attributes)) {
            $attributes = func_get_args();
        }

        $hooks       = $this->boundHooks(__FUNCTION__);
        $params      = compact('attributes');
        $payload     = $attributes;
        $destination = function ($attributes) {
            return parent::isDirty($attributes);
        };

        return $this->pipe($hooks, $payload, $params, $destination);
    }

Note the change in the first if statment

@EricTendian
Copy link
Contributor Author

@daverdalas thanks for the feedback! You're right, that would be better. Updated the PR accordingly.

@EricTendian EricTendian changed the title Changing default value of isDirty to prevent from always returning false Changing logic of isDirty to match with Laravel 5.5 and prevent from always returning false Oct 19, 2017
@spamoom
Copy link

spamoom commented Oct 21, 2017

Would be good to get this merged and tagged so that other packages can inherit the bugfix

@jarektkaczyk jarektkaczyk merged commit 7eb58b5 into jarektkaczyk:5.5 Nov 17, 2017
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