Lomkit / laravel-rest-api

Generate Api in seconds
https://laravel-rest-api.lomkit.com/
MIT License
444 stars 25 forks source link

morphTo relations with multiple types only ever return data from the related Model from fields defined in the first resource type #86

Closed Orrison closed 9 months ago

Orrison commented 9 months ago

Laravel Rest Api Version

2.3.3

Laravel Version

10.35.0

PHP Version

8.2.8

Database Driver & Version

Postgres

Description

When you define a MorphTo relation in a Resource, and you provide multiple types when performing a search and "including" the morphTo relation, the relationship data that is passed back via the API is returned only taking into account the first type Resource passed into the MorphTo::make method.

For example. If you have a MorphTo relationship called subscribable with two possible types. This is how you would register it:

public function relations(RestRequest $request): array
    {
        return [
            MorphTo::make('subscribable', [
                ProspectResource::class,
                StudentResource::class,
            ]),
        ];
    }

If you call the search endpoint and include the subscribable relationship like so: URL: http://localhost/api/v1/subscriptions/search Body:

{
    "search": {
        "includes": [
            {
                "relation": "subscribable"
            }
        ]
    }
}

The attributes that are returned for the subscribable relationship are the fields described in the first relationship. In this case ProspectResource.

If both Models/Resources have common columns that you will see data coming back for both relations. But you will see fields defined in the first Resource type even if it is not present in the second Resource type. Any fields defined in the second resource type are ignored and not returned.

This probably has something to do with Lomkit\Rest\Query\Traits\PerformSearch@include and that on line 195 it only ever returns one Resource. And Lomkit\Rest\Relations\Relation@resource explicitly calls $this->types[0]

The expected behavior should probably be that what is returned is based on the fields defined in the Resource of what type the Model is.

Steps To Reproduce

  1. Create a MorphTo relation with multiple possible related Models
  2. Ensure that the related Models have slightly different or completely different columns
  3. Create API Resources for these Models and define the MorphTo relationship and provide both related Model Resources and types.
  4. Perform a search and request that the MorphTo relation be included.
  5. You should see only fields described in the first Relation type returned.

You can see an example of us doing so here: https://github.com/canyongbs/advisingapp/commit/6cb8e8d4ee00e55b996951f14c3381248de13bbf

GautierDele commented 9 months ago

Hello @Orrison,

Thanks for your feedback, i'll need a bit more time to investigate this, feel free to submit a PR if you have time 😄

Orrison commented 9 months ago

Hey @GautierDele, I dug into it a little bit before submitting. I'll give it another look. Let me know if there is anything particular you think I should be looking at / looking out for.

GautierDele commented 9 months ago

Hi @Orrison,

I have a test reproducing your case, should be fixed by the end of the day, depending on the complexity 😄

Orrison commented 9 months ago

Hi @Orrison,

I have a test reproducing your case, should be fixed by the end of the day, depending on the complexity 😄

Awesome! I will say there is no rush on my end for a fix.

Unfortunately, we did pivot away from using this package. But it will still be an issue for folks using it going forward.

Thank you!

GautierDele commented 9 months ago

Unfortunately I won't be able to provide dynamic morph to because this is too specific and hard to handle. For now, morph to relation must specify the specific resource pointed, this allows uniformisation with all other relations If there are multiple resource, one relation for each must be declared