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

[8.x] Add support for disallowing class morphs #38656

Merged
merged 3 commits into from
Sep 3, 2021

Conversation

olivernybroe
Copy link
Contributor

This PR adds a new prevention flag to models. This flag is for disallowing morphs without a morph map on it.

For enabling it simply do a

Model::preventClassMorphs();

When a model does not have a morh map registered a ClassMorphViolationException will be thrown.

For preventing the exception, simply register the model in the morph map

Relation::morphMap([
    'user' => User::class,
]);

@driesvints
Copy link
Member

Were the concerns from #38451 taken into account on this PR?

@olivernybroe
Copy link
Contributor Author

@driesvints Did not know about that.
But as far as I understand from the PR, the issue is when you are getting the data from the database, it needs a map to lookup what model the string in the database corresponds to.

The feature I am proposing here should not be impacted for that, as this flag simply prevents you from saving a morph relation without a morphmap, it does not prevent you from loading a model without a morphmap.

So you can still load a model from the database where the column value is a FQCN, it is only the saving part that is prevented.

@aarondfrancis
Copy link
Contributor

@olivernybroe this is wonderful! I've been enforcing this myself via a trait aaronfrancis.com/2020/laravel-consistent-morphs but obviously would much rather this be first party.

Here is a thread where a couple of folks talk about their need for this feature: https://twitter.com/aarondfrancis/status/1427399215479672837.

The issue this would solve is that while it is simple to understand that if you change a class name you need to update everything in the DB, it's also extremely easy to forget. And when you have a big team with a huge application, that problem becomes much worse.

This makes the implicit explicit, which is super helpful for teams!

Thanks Oliver!

@taylorotwell
Copy link
Member

Moved the method to Relation like morphMap.

Added new method enforceMorphMap that can be used to define a morph map and turn off class mapping in one method call instead of two:

Relation::enforceMorphMap([
    'user' => User::class,
]);

@taylorotwell taylorotwell merged commit ab9d3c7 into laravel:8.x Sep 3, 2021
victorvilella pushed a commit to cdsistemas/framework that referenced this pull request Oct 12, 2021
* Add support for disallowing class morphs and thus requiring morph maps

* move method

* formatting - add method

Co-authored-by: Taylor Otwell <taylorotwell@gmail.com>
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.

None yet

4 participants