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

Fix Owner Morph Class Resolution and Add Check for 0 in Morph Map Keys #55

Merged
merged 3 commits into from
Nov 21, 2024

Conversation

rbondoc96
Copy link
Member

@rbondoc96 rbondoc96 commented Nov 20, 2024

Because of the changes made in this commit, the output of getMorphClass on the polymorphic owner is incompatible with the audit_changes.owner_type and audit_models.owner_type columns.

This PR also adds error checking against morph maps with a 0-key, as they ultimately get resolved to false deep in Laravel's relationship logic, which prevents models with that key from being morphed properly.

@rbondoc96 rbondoc96 marked this pull request as ready for review November 20, 2024 18:19
@rbondoc96 rbondoc96 changed the title fix: logger command fails when resolving owner morph class Fix Owner Morph Class Resolution and Add Check for 0 in Morph Map Keys Nov 20, 2024
Comment on lines +68 to +69
> [!CAUTION]
> `0` is not a valid key for the morph map, as it will get resolved to `false` in Laravel's morphing logic.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just to clarify on this for anyone interested: It's specifically the buildDictionary logic that is used when eager loading.

    protected function buildDictionary(Collection $models)
    {
        foreach ($models as $model) {
            if ($model->{$this->morphType}) {
                $morphTypeKey = $this->getDictionaryKey($model->{$this->morphType});
                $foreignKeyKey = $this->getDictionaryKey($model->{$this->foreignKey});

                $this->dictionary[$morphTypeKey][$foreignKeyKey][] = $model;
            }
        }
    }

The if ($model->{$this->morphType}) { check is what get resolved to false when the morph map value is a 0. This prevents the model from being added to dictionary and ultimately results in attempts at eager loading relationships such as entity always returning null when the relationship is to the model with a morph map value of 0.

@rbondoc96 rbondoc96 merged commit 2ad9e10 into master Nov 21, 2024
6 checks passed
@rbondoc96 rbondoc96 deleted the fix-owner-morphs branch November 21, 2024 16:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants