Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Possible bug with morphedByMany relations #15

Open
bert-w opened this issue Jul 25, 2019 · 3 comments
Open

Possible bug with morphedByMany relations #15

bert-w opened this issue Jul 25, 2019 · 3 comments

Comments

@bert-w
Copy link

bert-w commented Jul 25, 2019

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

I've tracked it down to

$join->where($relation->getMorphType(), '=', $parent->getMorphClass());

 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
Copy link
Owner

any chance for a PR with fix?

@bert-w
Copy link
Author

bert-w commented Apr 14, 2020

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

@marnickmenting
Copy link

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".

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants