[5.0] Fix More Bugs With "push" In Eloquent Models

[patrickcarlohickman]

For extra reference, bugs with the push() method have been mentioned in issues #6139, #6066, and PR #6067. Issues still remain.

Illuminate\Database\Eloquent\Model::push() has the following code snippet:

foreach ($this->relations as $models)
{
    $models = is_array($models) ? $models : array($models);
    foreach ($models as $model)
    {
        if ( ! $model->push()) return false;
    }
}

As far as I can tell, a relationship can either be a Collection (with 0+ items), a Model, or NULL. There are a couple issues with the current push() implementation:

Relationship is a Collection

If the relationship is a Collection, the is_array($models) check returns false, so the Collection is wrapped in an array. Therefore, $models is now an array containing one entry: the Collection. The code then iterates the array, and calls push() on the Collection, which is not what was intended. This causes the following error: Missing argument 1 for Illuminate\Support\Collection::push().

Relationship is NULL

If a hasOne relationship is loaded but the related object doesn't exist, the relationship will be set to NULL. In this case, in the push() method, is_array(NULL) returns false, and the NULL value is wrapped in an array. Therefore, $models is now an array containing one entry: NULL. The code then iterates the array, and calls push() on the NULL value, generating the following error: PHP Fatal error: Call to a member function push() on null.

Relationship is a Model

This looks to work fine.

This Pull Request attempts to correctly convert $models into an array of Models, filtering out NULL values as well. Tests have been included as well, which seem to work, but I don't have much experience writing tests, so they should be scrutinized.

[taylorotwell]

Thanks for your work on this ❤️

[patrickcarlohickman]

Not a problem. Glad I could help!