laravel-json-api / laravel

JSON:API for Laravel applications
MIT License
523 stars 43 forks source link

Allow morph relationships to define conditional $with attributes #261

Open efmjackhardcastle opened 8 months ago

efmjackhardcastle commented 8 months ago

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 commented 8 months ago

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']
            ]);
    }
lindyhopchris commented 8 months ago

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.

efmjackhardcastle commented 8 months ago

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!