Eloquent relation child -> parent

First: If I were you, I would call my Offer model "Offer" and OfferBids "Bid"

This will clean up all your code signifficantly, because you can use all laravel conventions.

Then an offer can have many bids, and a bid can belong to an offer.

Right, that's cool, but that's really just the naming convention. Is there any workaround for my problem though?

A similar post is going on right now read https://laracasts.com/discuss/channels/laravel/need-help-with-ticket-system

@jlrdw I don't think its actually related to my problem

one of the dangers of using protected $with=['relation'] is that you load the relation when it might not be necessary

if you don't have it then you can just do $offers = Offers::with('bids')->get();

@peterstarling Where possibly would you require that, you know the parent and the children. Why would you again ask the children who their parent is ?

@Snapey: If you're referring to using:

protected $with = ['childModel'];

in the parent Model, I couldn't agree more with the "dangerous" sentiment. I have created quite the mess using that on models in the past. I now refuse to ever, what would you call it? pre-eager-load? or automatically-eager-load? any hasMany or hasManyThrough models in the parent models. I just get them ->with() when I need to. When I first learned about using protected $with = ['childModel']; I thought it was the best thing I had ever heard of. It's now on my "never do" list.

@goatshark yes, thats what I mean, I updated my reply to try and indicate which I am talking about.

I still occasionally use it but only for enumeration scenarios, ie System belongs to SystemType where SystemType is a simple lookup table.

@goatshark

have created quite the mess using that on models in the past. I now refuse to ever, what would you call it? pre-eager-load? or automatically-eager-load? any hasMany or hasManyThrough models in the parent models. I just get them ->with() when I need to. When I first learned about using protected $with = ['childModel']; I thought it was the best thing I had ever heard of. It's now on my "never do" list.

Moderation is the key my friend ... :)

Right, that's cool, but that's really just the naming convention. Is there any workaround for my problem though?

Maybe I don't understand your problem, because it's kinda confusing for me with how you named things right now.

If one offer has many bids, and your relations are setup correctly, you can do

$offer->bids

to get all the bids for an offer

or

$bid->offer

To get the offer for a bid.

But again, I probably don't understand the problem :)

This basically means that Offers contains multiple OffersBids, but the OffersBids then refer to the parent to define if they are bids made by the user who created the offer or other user.

Why do you check the bid ownership and not the offer ownership (as offer contains the user_id) ? The problem is probably just your app badly structured, not eloquent related.

I don't think it's badly structured, a main offer 'thread' can be made by one user, but then bids can then be made by others, the idea is to see if any of the bids was actually made by the user who created the offer 'thread'.

So far we've established that the $with property is a bad practice, and I agree.

Anyway just to show you a better example of what I am thinking about: model Listing has a hasMany relation with ListingImage and naturally ListingImage has then a relation belongsTo linking it with its parent: Listing model. Every ListingImage has an attribute full_path, which is defined as follows:

public function getFullPathAttribute()
{
    return 'images/' . $this->listing->user_id . `/` . $this->filename;
}

We refer to the parent model to get the user_id, this means we essentially make another db call (I'm not too worried about that, all my models are cached anyway) and what makes matters worse is that its appended to the model so that after $image->toArray() the image array will contain Listing data.

This does not look to me like a bad structure (I might be wrong though), more like a typical db schema. My question is, perhaps in the relation between Listing and ListingImage we could pass a parameter that would then be used in the child model? Or somehow pass the parent? That would basically solve the problem.

a main offer 'thread' can be made by one user, but then bids can then be made by others

So just $offer->user_id == $bid->user_id. I think you should not check this at the bid model level. I would create a sameOwner method in the Offer model which takes a Bid as parameter and return if the same user own them then use it like : $offer->sameOwner($bid).

Anyway, about using parent model in model method, as other said don't use the $with attribute it is a source of problem. Then use the with method correctly :

$offers = Offer::with('bids.offer')->get() // if you want to load the parent for each child

$offers = Offer::with('bids')->get() // if you dont need the parent

You can also store the offer user id in the bid. I dont think the offer user will change so it can be a reliable solution.

@pmall you're probably right, but that means I'd have to loop through the results (possibly in the controller or some other library) and append this property to every $bid model comparing its user with $offer user.

the use of $with is not even a case here, even without that any reference to other model will essentially append it to the model you're referring from.

Why do you want to know if they have the same user, for displaying something ?

Do you know what - it's not even that important here, take a look at the other example I gave - an image model that belongs to listing model. Image generates its full_path attribute that asks for parent's (so listing's) user_id (as this is required in the full path).

See what I mean? It seems perfectly reasonable to refer to parent's model property. And you're right, we could simply take that logic out of the model, but as I then said - this would lead to looping through the child models and appending attribute manually, which seems a bit cumbersome.

I still don't get the problem here.

//Offer.php
class Offer extends Model
{   
    public function bids()
    {
        return $this->hasMany(Bid::class);
    }
    public function owner() 
    {
        return $this->belongsTo(User::class);
    }    
}

//Bid.php
class Bid extends Model 
{
    public function offer()
    {
        return $this->belongsTo(Offer::class);
    }
    
    public function bidder() 
    {
        return $this->belongsTo(User::class);
    }

    public function bidderIsOfferOwner() {
        return $this->bidder->id == $this->offer->owner->id;  //you can probably just compare users here, don't know for sure, so id's will work.
// so maybe even better: return $this->user_id == $this->offer->user_id;
    }

}

I don't even think the extra method is nessecary. If you want to show the offer with all the bids, you already HAVE the offer, so:

@foreach($offer->bids as $bid) 
@if($bid->user_id == $offer->user_id)
this bid is made by the owner...
@endif 
@endforeach

Image generates its full_path attribute that asks for parent's (so listing's) user_id (as this is required in the full path).

I cant figure out why you think about looping over a collection to append an attribute. I guess it is in a view so :

@include('partials.image', ['listing' => $listing, 'image' => $image'])

# partials/image.blade.php
<img src="images/{{ $listing->id }}/{{ $image->id }}.jpg">

Yes the above works.

And I agree - if we leave out the $with property it will never lead to an infinite nesting (actually we don't even have to do that, we can simple do return $this->user_id !== $this->offer()->setEagerLoads([])->first()->user_id; which will refer to the parent and will tell Eloquent not to eager load the relation again)

However the result of the above and the use of Offers::with('bids')->find($offer_id)->toArray() will be an array of all the Offer properties andbids array of all the bids, which will then again contain parent offer (so $offer['bids'][0]['offer'] will be a copy of $offer). Naturally we could just ignore that but what if every Offer model contains multiple attributes and so on, this will not only drastically increase the amount of data returned but also certainly affect execution speed.

which will then again contain parent offer

Not if you are not using the model $with attribute.

Also you should use something like the fractal package to have better control over your models serialization to array/json.

@pmall Perhaps I should have mentioned that this is a RESTful API, so it needs to return all the data and to do that I would have to loop over a collection.

Sure that will work but in this thread I am looking for a way to avoid unnecessary calls from parent to child model and back.

If you're building a rest api, if someone goes to the endpoint of an offer, you can just return (some serialized/sanitized/whatever version of:

$offer = Offer::with('bids')->find($id);

no need to include the offer again within the bids, because they will always be the same offer you already return

And if someone goes to the endpoint for a bid:

$bid = Bid::with('offer')->find($id);

Again: Maybe I don't get what the problem is, but that leads me to believe there is no problem. Just trying to help here :)

Perhaps I should have mentioned that this is a RESTful API, so it needs to return all the data and to do that I would have to loop over a collection.

Try the fractal package.

avoid unnecessary calls from parent to child model and back.

This is three db calls :

$offers = Offer::with('bids.offer')->get();

Hard to do better. If you just want to $offers = Offer::with('bids')->get() with two queries you must store all the data needed for the bids in the bids table. IMHO don't worry about 2 or 3 request.

If you return only one particular model each time then @Prullenbak is right. If sometime you return a list of offers with their list of associated bids then consider this answer.

Even if you want to return a list of offers, with their bids:

Offer::with('bids')->all();

This will result a list of offers with bids attached. Why should those bids contain those offers again?

For bids, same thing:

Bid::with('offer')->all();

Why should those bids contain those offers again?

Because he want to check if the offer owner and the bid owner are the same.

First of all - thanks for the help guys, I'm only trying to make a point but I agree that the source of my problem is that I made the problem up myself. However, as @pmall noted:

If you just whant to $offers = Offer::with('bids')->get() with two queries you must store all the data needed for the bids in the bids table.

Yep, that makes sense, as I said I don't even mind making another request from the child back to the parent but this will essentially load the entire parent model again and append it to the child model.

Ideally in the parent model we could do something like

public function bids()
{
    return $this->hasMany(OffersBids::class, 'offer_id', 'offer_id')->withProperty('parent_user_id', $this->user_id);
}

the withProperty part is what I made up, of course. This would give me the ability to use $this->parent_user_id inside OffersBids. Does that make sense?

You should perhaps look at other example I pasted in this thread with Listings and Images, it makes the need of adding the above functionality much more reasonable.

Oh and looks like I could really use fractal package, thanks.

Because he want to check if the offer owner and the bid owner are the same.

And why would he want to do that? And where? when?

He's building a REST api, right? So would't that be up to the consumer of the api to check? And they can easily do that since they will have an order (with a userid) and a list of bids (all with a owner id).

And why would he want to do that? And where? when?

I guess in order to add a special flag attribute if it is the case.

but this will essentially load the entire parent model again and append it to the child model.

You are thinking too much about this, this is not a problem at all. Load all your models, use the fractal package to format the json output and it is fine.

ooo then isn't it as simple as this?

//in Bid model

protected $appends = ['by_owner'];

getByOwnerAttribute() {
    return $this->user_id == $this->offer->user_id;
}

ooo then isn't it as simple as this?

Yes but OP is for some reason worrying about loading the full parent offer object just for this :-)