jarektkaczyk / eloquence-base

base for the Eloquence extensions + Searchable
https://softonsofa.com
MIT License
77 stars 68 forks source link

Possible bug with `morphedByMany` relations #15

Open bert-w opened 5 years ago

bert-w commented 5 years ago

It appears that I cant search properly in my morphedByMany relations.

I've tracked it down to https://github.com/jarektkaczyk/eloquence-base/blob/46f885cde97704bb263faa237fba6760b838bb5a/src/Relations/Joiner.php#L147

 if ($relation instanceof MorphOneOrMany) {
            $join->where($relation->getQualifiedMorphType(), '=', $parent->getMorphClass());
        } elseif ($relation instanceof MorphToMany || $relation instanceof MorphByMany) {
            $join->where($relation->getMorphType(), '=', $parent->getMorphClass());
        }

The last $parent->getMorphClass() should be $relation->getMorphClass(), in particular when the relation is a morphedByMany. So this elseif statement needs to be split.

I dont have a good testcase but here's the main setup that causes the bug:

table: "assets"
columns: id, name

table: "asset_relations"
columns: asset_id, morphable_id, morphable_type

table: "user"
columns: id, username
// App\Models\Asset.php (Eloquent model)

protected $searchableColumns = ['users.username'];

 public function users()
    {
        return $this->morphedByMany(User::class, 'morphable', 'asset_relations');
    }
// App\Models\User.php (Eloquent model)

public function assets()
{
    return $this->morphToMany(Asset::class, 'morphable', 'asset_relations');
}

Lastly, try to make a query like this: Asset::with(['user'])->search('somename')->get();

The SQL itself will generate the wrong query since asset_relations will be joined on morphable_type = App\Models\Asset instead of the correct App\Models\User.

jarektkaczyk commented 4 years ago

any chance for a PR with fix?

bert-w commented 4 years ago

I would PR it but I wasnt sure if this was the correct solution.

marnickmenting commented 3 years ago

I added another fix to the pull request, because when searching in more than one morphable relation I got SQL errors "'morphable_type' in on clause is ambiguous".