laravel icon indicating copy to clipboard operation
laravel copied to clipboard

Allow morph relationships to define conditional $with attributes

Open efmjackhardcastle opened this issue 2 years ago • 3 comments

I have the following:

    protected array $with = [
        'eventRelationshipUpdates.fromModel.ancestors'
    ];

fromModel is morphable, some of these model types may have an ancestors relationship that needs loading, some may not, this doesn't work due to not every morphable model having this relationship defined.

For morphable relationships any $with inclusions, as well as anything present in the include query, should conditionally include if the resolving model has that relationship available and defined.

efmjackhardcastle avatar Oct 30 '23 13:10 efmjackhardcastle

I have found a workaround I will use in the meantime, on the relationship definition:

    public function fromModel(): MorphTo
    {
        return $this->morphTo()
            ->morphWith([
                MaintainableEntity::class => ['ancestorsAndSelf']
            ]);
    }

efmjackhardcastle avatar Oct 30 '23 13:10 efmjackhardcastle

Thanks for raising. I think you're going to have to stick with the workaround for now, as the eager loading needs revamping to unlock several features on the development roadmap. I'll leave this issue open though as something to look into, either as part of that revamp or once the revamp has been done.

lindyhopchris avatar Oct 30 '23 16:10 lindyhopchris

No problem @lindyhopchris - on reflection, this workaround doesn't as much feel like one. There would be limitations if you only wanted to do this contextually under one server, however not an issue right now.

Would be nice to have the same behaviour in the future on the Schema definition.

class EventSchema extends Schema
{
    protected array $morphWith = [
        'eventRelationshipUpdates.fromModel' => [
            MaintainableEntity::class => ['ancestorsAndSelf'],
        ],
    ];
    
    ....
}

Don't have a semantic option to offer for the includes field I'm afraid!

But appreciate what you're saying in the meantime.

Thanks for the package, it's great!

efmjackhardcastle avatar Oct 30 '23 19:10 efmjackhardcastle