Skip to content

Use the parent's morph class if parent returns children#33

Merged
calebporzio merged 3 commits intotighten:masterfrom
wfeller:morphable
Jan 31, 2019
Merged

Use the parent's morph class if parent returns children#33
calebporzio merged 3 commits intotighten:masterfrom
wfeller:morphable

Conversation

@wfeller
Copy link
Contributor

@wfeller wfeller commented Jan 23, 2019

If the parent is designed to return children, then the children will use the parent's morph class.

Probably solves #10

Without this change, using whereHas doesn't always work on children classes. Unless you do something like this:

// ugly
$passenger = Passenger::query()->where(function ($query) {
    $query->orWhereHas('car.parts', function ($query) {
        $query->where('type', 'tire');
    })->orWhereHas('vehicle.parts', function ($query) {
        $query->where('type', 'tire');
    });
})->first();

instead of just this:

// a little better
$passenger = Passenger::query()->whereHas('vehicle.parts', function ($query) {
    $query->where('type', 'tire');
})->first();

PS: could you tag a new release ?

Thanks

@calebporzio
Copy link
Contributor

Thanks for the good work on this!

@calebporzio calebporzio merged commit 06bdd84 into tighten:master Jan 31, 2019
@calebporzio
Copy link
Contributor

Also, I'm guessing you left this in there by accident, but I'm just curious. What do you use ".idea" files for? Just a scratch pad for ideas?

@adriandmitroca
Copy link
Contributor

.idea directory is used by all JetBrains' IDEs (PHPStorm, WebStorm) to store project configuration files.

@calebporzio
Copy link
Contributor

gotcha, will leave it in then

@calebporzio
Copy link
Contributor

Also will tag a release today

@calebporzio
Copy link
Contributor

I'm having a hard time wrapping my head around this. @wfeller, can you explain why the conditional is necessary here?:

    public function getMorphClass()
    {
        if ($this->parentHasHasChildrenTrait() && in_array(static::class, $this->getChildTypes())) {
            return (new $this->getParentClass())->getMorphClass();
        }

        return parent::getMorphClass();
    }

I think my brain is blocked on this for some reason. Why is "(new $this->getParentClass())->get.." any different than "parent::getMorphClass()"?

@wfeller
Copy link
Contributor Author

wfeller commented Jan 31, 2019

Hey @calebporzio,

Calling parent::getMorphClass will return the morph class using static::class being the child, while what we're trying to get here is the morph class of the parent (see code at the end).

You actually got me wondering about the in_array(static::class, $this->getChildTypes()) part... In my tests and use cases, I always define the child types on the parent, so I added this check without really thinking about why it should be there (and can't find a reason to leave it be). I think we should remove it, what do you think?

Using the examples in the tests, that means that (new Car)->getMorphClass() would currently return Vehicle::class, while (new Plane)->getMorphClass() would return Plane::class, while I think we'd want to return Vehicle::class in both cases.

public function getMorphClass()
 {
     if ($this->parentHasHasChildrenTrait() /*&& in_array(static::class, $this->getChildTypes())*/) {
         // This returns Vehicle::class
         return (new $this->getParentClass())->getMorphClass();
     }

     // And this returns Car::class
     return parent::getMorphClass();
 }

@calebporzio
Copy link
Contributor

Got it, thanks for the clarification. Also, great - I'm going to remove the "&&" part of the conditional. I'm just doing some more cleanup on your contribution and I'll have something up to be tagged shortly.

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

Successfully merging this pull request may close these issues.

3 participants