[5.6] Fix isDirty(null) and wasChanged(null)

[staudenmeir]

HasAttributes::isDirty() is defined like this:

public function isDirty($attributes = null)

Calling isDirty() on a modified model returns true. But explicitly using the default value returns false:

$model->isDirty(); // true
$model->isDirty(null); // false

So something like $model->isDirty($if ? ['foo'] : null) doesn't work.

The same applies to HasAttributes::wasChanged().

Fixes #23924.

[devcircus]

I agree with the comment from @36864 here concerning this and think it should go to master.

[Miguel-Serejo]

Thinking it over a bit more, I'm not sure if I maintain my earlier position.

On one hand, it's weird that isDirty([]) and isDirty(null) behave differently. On the other hand, currently this lets you have different behaviors between multiple optional filters and a single optional filter.

$filters = $filterFields ?? [];
$model->isDirty($filters); // Check if a model's fields are dirty. If not using filters, check all fields.

$filters = $filterFields ?? null;
$model->isDirty($filters); // Check if a model's fields are dirty, if filters weren't defined, ignore all changes

I'd decidedly undecided in this matter. It definitely needs to target 5.7 though.

[devcircus]

I feel like we're overlooking something with the proposed implementation. Haven't looked it over thoroughly though. Seems almost impossible that it won't affect current apps though, so I'd send to master.

[staudenmeir]

The behavior of null vs [] is a separate discussion. This PR is just about isDirty() and isDirty(null) behaving the same way when the default parameter value is null.

[taylorotwell]

No plans to change this right now.

[tomlankhorst]

This makes no sense... passing the default parameter explicitly should yield exactly the same behaviour. Why else provide a default? If the current behaviour is intended, we should use a variable-length argument list and func_num_args() imo.