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] Option to use table names when morphing #38451

Merged
merged 4 commits into from Aug 19, 2021
Merged

Conversation

taylorotwell
Copy link
Member

This PR introduces a new method to instruct Laravel to use table names instead of class names when storing records in polymorphic tables. Typically this method can be called in the register method of your AppServiceProvider:

Relation::morphUsingTableNames();

Not much else to say about this one 😅

@taylorotwell taylorotwell changed the title Table name morph map Option to use table names when morphing Aug 19, 2021
@taylorotwell taylorotwell merged commit 5d5655b into 8.x Aug 19, 2021
@taylorotwell taylorotwell deleted the table-name-morphmap branch August 19, 2021 14:54
@GrahamCampbell GrahamCampbell changed the title Option to use table names when morphing [8.x] Option to use table names when morphing Aug 19, 2021
@@ -731,7 +731,9 @@ public function getMorphClass()
return array_search(static::class, $morphMap, true);
}

return static::class;
return Relation::$useTableNamesForMorphMap
Copy link
Contributor

Choose a reason for hiding this comment

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

While this code is being changed, maybe it would also be useful to add an optional property/method to models (eg. morphableName) that can be used - so each model can represent its own morphable name itself instead of being done elsewhere in a map?

@dmason30
Copy link
Contributor

This change was tried last year and it doesn't work with MorphTo relations and was decided it was impossible to work without a Morph map.

#35519

@rodrigopedra
Copy link
Contributor

rodrigopedra commented Aug 19, 2021

One doubt: how retrieving from the database would work? From the code changes it is not clear to me.

If we still need to use a morph map for retrieving related polymorphic models, wouldn't this be unneeded as the check occurs after checking if a morph map is being used?

Sorry if I am missing something obvious on this.

@taylorotwell
Copy link
Member Author

Well, reverting.

taylorotwell added a commit that referenced this pull request Aug 19, 2021
taylorotwell added a commit that referenced this pull request Aug 19, 2021
Krisell pushed a commit to Krisell/framework that referenced this pull request Aug 20, 2021
* support class basenames on morphs

* table name option for morphmap

* rename method

* fix test
Krisell pushed a commit to Krisell/framework that referenced this pull request Aug 20, 2021
Krisell pushed a commit to Krisell/framework that referenced this pull request Aug 20, 2021
* support class basenames on morphs

* table name option for morphmap

* rename method

* fix test
Krisell pushed a commit to Krisell/framework that referenced this pull request Aug 20, 2021
@sebastiaanluca
Copy link
Contributor

sebastiaanluca commented Aug 20, 2021

@taylorotwell To work around this issue, an alternative is to scan the models and auto-register the morph map when booting the framework. Probably too much of a change for a papercut issue.

See https://github.com/sebastiaanluca/laravel-auto-morph-map for a working example. Only drawback is the scanning of models part. Could probably be optimized.

What's supported:

  • Auto-registers morph map for all models
  • You can override existing ones with your own definitions using the regular ::morphMap() method
  • Caching for the model names
  • Ability to configure name (singular table, full table, or class name) and casing (snake, slug, camel, studly, none)

Interested in feedback, maybe worth a PR?

victorvilella pushed a commit to cdsistemas/framework that referenced this pull request Oct 12, 2021
* support class basenames on morphs

* table name option for morphmap

* rename method

* fix test
victorvilella pushed a commit to cdsistemas/framework that referenced this pull request Oct 12, 2021
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

6 participants