[5.4] allow BelongsTo relations to return defaults

[browner12]

continuation of #19696

I was going to try and do all of the relationships right away but it is a little more confusing than I thought, so I'm going to keep it broken out with a PR for each relationship.

code only required minor tweaks from the HasOne implementation.

I've tested the code on an actual application of mine, and it mostly works. The place I run into an issue is if I have an eager loaded relationship on the model via the $with property. If I try to eager load the parent model, all the properties are set to null. If I don't eager load, everything works just fine.

Anyone have any ideas on why this is?

BTW, it is incredible how much cleaner and more readable my code becomes with this. really excited to get this in!

[browner12]

ping @decadence and @sileence

[decadence]

Thanks for starting this.
I can't test it now but is eager load working ok with HasOne and withDefault? If so maybe @sileence can turn you into right direction. If not we need to find solution for both of cases.

[taylorotwell]

@decadence can you give an example of what you are concerned may not be working so we can write a test for it?

[decadence]

@taylorotwell I just commented initial PR message:

I've tested the code on an actual application of mine, and it mostly works. The place I run into an issue is if I have an eager loaded relationship on the model via the $with property. If I try to eager load the parent model, all the properties are set to null.

So I wonder if HasOne works properly with with.
If so I suggest investigate it together with @sileence

[sileence]

Maybe just merge it for now so I can pull, test and apply the fix?

[decadence]

You can pull branch of the @browner12 fork in your fork without merging it in Laravel :)
git pull git@github.com:browner12/framework.git belongto-default

[browner12]

I'll try and explain a little better what I'm seeing.

blogs table

• id
• title
• body
• user_id
• name
• email

class Blog
{
    protected $with = ['user'];

    public function user()
    {
        return $this->belongsTo(User::class)->withDefault([
            'name' => $this->name,
            'email' => $this->email,
        ]);
    }
}

Then let's say I pull a blog that does not have a user_id, but has a name and email.

$blog = Blog::find(1);

If I dump the user from this I am getting null values for both name and email.

dd($blog->user);

However, if I remove the eager loading, it works fine and has the name and email.

I'm not sure what is happening from a code standpoint yet, because I don't have a great grasp on the eager loading yet. My guess is maybe the eager load is taking precedence with something.

[taylorotwell]

This works fine for me:

What is broken?

[sileence]

@taylorotwell I believe the problem is with eager load, I'm cloning to have a look now.

[taylorotwell]

I have that too, updated screenshot, still working as expected.

[browner12]

yup, @taylorotwell if you hard code values in it works fine, but if you try to access a property on the Post class it will be null.

return $this->belongsTo(User::class)->withDefault([
    'name' => $this->name,
    'email' => $this->email,
]);

[sileence]

sorry @browner12 but what do you mean with hardcoding values?

[taylorotwell]

In @browner12's example I do see null on $this->name from within the user() function, but that seems to happen with or without this feature, so I'm not sure it's really related or relevant to this particular PR.

[sileence]

Yes, when Eloquent is executing the queries the attributes from the main model are not there yet.

[browner12]

@sileence in Taylor's example he hardcoded the 'name' and 'email' values in with strings, which works fine

[taylorotwell]

Went ahead and merged this one since I don't think it causes the other "anomaly" we are seeing. If anyone wants to explore that I would definitely appreciate it :)

[browner12]

@sileence why does the presence or absence of $with change if the query is executed earlier or later?

if we have to go with this for now, that's fine with me. It allows us to use null models, but now my pseudo model case won't work with eager loading, so I'll just turn the eager loading off for the time being.

I'd love to see if I can get it working with eager loading as well. any thoughts on a path for me to start down?

[sileence]

@browner12 because eager load means things are loaded before you start interacting with the model. I'm still having a look to see if I can fix that issue... By the way when I finally decide to clone the fork, Taylor comes and merge it to the framework 🙃

[browner12]

hah, that's okay. now we've got about 5 other relations we can see if we can try and tack this onto.

I'll keep looking into it as well, let me know if you make any progress. I'm on the Slack channel or on here.

let me know if you decide to tackle any of the other relations too 😄

[sileence]

@browner12 the related models are not eager loaded in the context of a specific model instance, that's why this doesn't work:

return $this->belongsTo(User::class)->withDefault([
    'name' => $this->name,
    'email' => $this->email,
]);

I don't see an easy fix for this, because one user could be eager loaded for 10 different posts (i.e. one user is the author of 10 posts) and therefore we face a typical javascript problem: "What this refers to?"

Maybe use dynamic attributes instead?

[decadence]

Thanks for adding this

[FatBoyXPC]

Okay, I love the change that @taylorotwell and @browner12 have made! Is there any chance we could get this expanded past belongsTo, though? I'm thinking for something like hasOne it would work the same, but for empty collections, first() will act like firstOrNew() (alternatively, to prevent BC issues, add firstOrNew() to collections).

[sileence]

@FatBoyXPC it's already there, hasOne, morphOne, belongsTo, morphTo they all support the withDefault option:

#16198

[jasonvarga]

I just ran into the issue regarding eager loading, and may have found a fix.

I'll show our usage as an example. Admins can set up products on behalf of sellers. Until a seller creates their account and links the product, we have a column on the product that has the seller's name.

class Product extends Model
{
    public function seller()
    {
        return $this->belongsTo('App\Seller')->withDefault(function ($seller) {
            $seller->name = $this->temporary_seller_name;
        });
    }
}

When eager loading, $this->temporary_seller_name is null. (There are simply no attributes on $this). When not eager loading, $this->temporary_seller_name shows the expected value.

To fix this, I changed this line:

framework/src/Illuminate/Database/Eloquent/Relations/Concerns/SupportsDefaultModels.php

Line 54 in 281038e

return call_user_func($this->withDefault, $instance) ?: $instance;

To this:

$closure = \Closure::bind($this->withDefault, $parent);
return call_user_func($closure, $instance) ?: $instance;

I don't completely understand what's going on, that's why I'm putting this as a comment instead of a PR. Maybe someone else can make better sense of what's going on.

It seemed like when eager loading, the $this in the closure is a new instance of the parent model, which doesn't have any $attributes yet. Re-binding the $parent as $this in the closure seems to fix it.

Tests continue to pass. However, this only fixes it if you use a closure in withDefault. Using an array continues to have the problem.