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

Model's newCollection method runs twice #16174

Closed
iamzozo opened this issue Oct 29, 2016 · 5 comments
Closed

Model's newCollection method runs twice #16174

iamzozo opened this issue Oct 29, 2016 · 5 comments

Comments

@iamzozo
Copy link

iamzozo commented Oct 29, 2016

  • Laravel Version: 5.3.2
  • PHP Version: 5.3
  • Database Driver & Version: MySql 5

Reference in the docs

Description:

When overriding base model with newCollection method, it runs twice.

  • Database\Eloquent\Builder\get > getModels > hydrate > newCollection
  • Database\Eloquent\Builder\get > newCollection

Steps To Reproduce:

  1. Create a newCollection method in a model
@GrahamCampbell
Copy link
Member

Please could you provide a small code sample. Thanks. :)

@iamzozo
Copy link
Author

iamzozo commented Oct 30, 2016

Here is my model, with the newCollection method, which get called twice, when calling: Post::get()

class Post extends Model
{
    protected $guarded = ['id'];
    protected $with = ['author'];

    public function newCollection(array $models = [])
    {
        return collect($models);
    }
}

Basically, I would like to alter the items of a collection in my models.

@jhoff
Copy link
Contributor

jhoff commented Oct 31, 2016

This is expected behavior and doesn't have anything explicitly to do with the fact that you're overriding the method.

When you call the get method the builder executes the query and collects all of the results using the getModels method. getModels is using Model::hydrate to build the first collection ( using newCollection ) without eager loads but then needs to unwrap it because getModels needs to return an array. After eager loading is complete ( if necessary ) then newCollection is called again to return the newly eager loaded dataset.

In order for the builder to not call newCollection twice, it would have to start passing Collections around under the hood. It seems intentional to me that passing around arrays is likely more efficient, and in the case of calling get, making a collection twice is the trade off.

I'm a little fuzzy as to what you're trying to do as it really shouldn't matter that this is being called multiple times. The purpose of the method, as stated in the docs is to allow you to use a custom collection class over the Illuminate Collection class. It seems to me that attempting to modify the contents of the collection in this method is probably bad idea.

Can you provide some more context as to what you're trying to do? It seems there is likely a better place for you to transform your collection than in this method.

@iamzozo
Copy link
Author

iamzozo commented Oct 31, 2016

I wanted to add user related fields to the collection models. So adding fields to the post, like is the user marked that post as favorite or not, which data comes from not an exact relation. So when I used this, I realized it duplicate the queries. I had a workaround with adding another method, just tought it's the right way to modify collections

@jhoff
Copy link
Contributor

jhoff commented Oct 31, 2016

Yeah, newCollection definitely isn't the right place for that...

It seems that you could maybe use an attribute getter for something like that:

public function getIsFavoriteAttribute()
{
    return $this->favoritedBy(Auth::user());
}

public function favoritedBy(App\User $user)
{
    return /* Has $user favorited $this ? */;
}

Then any model that you use in your blade files or whatever will check when you try to access the $post->isFavorite attribute.

If you want that attribute to automatically be added to the model for json and array serialization ( like in an API response ), you can add it to the appends property:

class Post extends Model
{
    ...
    protected $appends = [
        'isFavorite'
    ];
    ...
}

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

3 participants