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

HasAttributes::hasChanges null attributes parameter detection #23924

Closed
eidng8 opened this issue Apr 18, 2018 · 5 comments
Closed

HasAttributes::hasChanges null attributes parameter detection #23924

eidng8 opened this issue Apr 18, 2018 · 5 comments

Comments

@eidng8
Copy link

eidng8 commented Apr 18, 2018

  • Laravel Version: 5.6.17
  • PHP Version: 7.1

Description:

The hasChanges() method checks for empty $attributes at entrance.

        // If no specific attributes were provided, we will just see if the dirty array
        // already contains any attributes. If it does we will just return that this
        // count is greater than zero. Else, we need to check specific attributes.
        if (empty($attributes)) {
            return count($changes) > 0;
        }

However, functions like isDirty() and wasChanged() calls the method like these:

    public function isDirty($attributes = null)
    {
        return $this->hasChanges(
            $this->getDirty(), is_array($attributes) ? $attributes : func_get_args()
        );
    }
    public function wasChanged($attributes = null)
    {
        return $this->hasChanges(
            $this->getChanges(), is_array($attributes) ? $attributes : func_get_args()
        );
    }

When called through isDirty() like $this->isDirty(null);, hasChanges() receives the $attributes parameter as an array of [null] due to is_array($attributes) ? $attributes : func_get_args(). Which in turn skips right through the empty check and returns a faulty false.

Steps To Reproduce:

Just call $this->isDirty(null); on any modified model instance.

@staudenmeir
Copy link
Contributor

I'll fix it.

@Miguel-Serejo
Copy link

What is there to fix here? Calling isDirty(null) is asking "Is the null property of my Model dirty?" to which the answer should always be "No, because it doesn't exist".

What's your use case for calling isDirty(null)?

@staudenmeir
Copy link
Contributor

Since the default value of the $attributes parameter is NULL, explicitly calling isDirty(null) should work:

$model->isDirty($if ? ['foo'] : null);

@Miguel-Serejo
Copy link

If anything, that's an argument for changing the default to [] rather than null just to avoid people thinking null is a valid parameter value.

@staudenmeir
Copy link
Contributor

staudenmeir commented Apr 18, 2018

The result of isDirty([]) on a modified model was (accidentally) changed in #20130. Before, it returned false, now it returns true. So originally null was different from [].

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

4 participants