Skip to content
This repository has been archived by the owner on Jul 16, 2021. It is now read-only.

[Proposal] Custom morphTo ownerKey #587

Closed
fntneves opened this issue May 12, 2017 · 4 comments
Closed

[Proposal] Custom morphTo ownerKey #587

fntneves opened this issue May 12, 2017 · 4 comments

Comments

@fntneves
Copy link

fntneves commented May 12, 2017

Hello everyone, this is a proposal to make morphTo relationship method customizable, as well as its motivation.

Motivation:

Most of the relationships provided by Laravel are customizable in a sense that keys and relations can be changed to match out-of-convention situations.

The belongsTo is the inverse relationship of both hasOne and hasMany relationships.
In the same category, we have morphTo.

Now, compare both method signatures:

  • belongsTo($related, $foreignKey, $ownerKey, string $relation)
  • morphTo($name, $type, $id)

In fact, both methods represent inverse relationships. However, morphTo does not allow to pass a custom "ownerKey". The ownerKey under the scope of morphTo, to the best of my knowledge, is the name of the column that matches the primary key of the target model.

Let me show an example. Assume the following and that it follows Laravel conventions:

  • Models: Share, Post and Comment.
  • Relationships:
    • Share 1:N Comment
    • Post 1:N Comment

This is a typical use case for polymorphic relationships. Then, the Comment model would have a morphTo association with commentables (Share and Post).
The morphTo method would find either the Share or Post instance that matches the commentable_id and commentable_type (not relevant here) from the Comment's table. The primary key, in both Share and Post models, is id.

However, the Polymorphic Relation is considered an Anti-Pattern in literature. Moreover, the alternative solutions to achieve the same behavior but with some constraints, even if it requires additional tables, are not achievable without the customization of the morphTo relationship. And it is relevant to people who want to take full advantage of Eloquent ORM.

Use Case

In a project of ours, we adopted the following alternative:

commentables

In summary, we added an intermediate table, as well as its model, that represents the entities that are able to receive comments. Thus, we ended with something like this:

  • Models: Share, Post, Comment and Commentable.
  • Relationships:
    • Commentable 1:N Comment
    • Commentable 1:N Comment
    • Post 1:1 Commentable
    • Share 1:1 Commentable

And the corresponding schema is:

screen shot 2017-05-12 at 15 34 47

Roughly, each concrete commentable entity, such as Post and Share, when is inserted, receives a Commentable identifier. From this identifier, then we achieve its comments. It ensures a constraint that if a Commentable entity is removed, then its likes will be also removed, with ON CASCADE. At least, database will raise an error. Note: The type column will be explained later.

It seems good for the following direction:

  • Post --- belongsTo ---> Commentable --- hasMany ---> Comments
  • Share --- belongsTo ---> Commentable --- hasMany ---> Comments

Problem

What if we want to achieve the concrete commentable entity from its comment?

  • Comment --- belongsTo ---> Commentable --- ??? ---> Share/Post

Actually, we could achieve that with pure SQL. However, it would disable us to use some of the Eloquent ORM features, like eager loading. Instead, we still used morphTo to achieve the inverse direction, namely, to access the concrete entity of commentables.

Here is the solution:

  • Comment --- belongsTo ---> Commentable --- morphTo ---> Share/Post

But... No, it is not the solution. Actually, it is part of the solution.

Why ? Let's analyze how we would get the concrete entity:

First we have a Comment. From the Comment, through a belongsTo relationship, we get its Commentable. Then, from the Commentable, we could use morphTo, and that's why we added a type column, to allow us to reach the concrete entity.

However, the default behavior of morphTo is to match the *_id with the primary key column (id, in this case) of the concrete commentable model. In other words, it would use the idvalue of a given Commentable to match the foreign id column in the table that stores the models corresponding to the type stored in type.

The commentable_id is, in fact, what should be used to find an id of a concrete commentable entity.

However, we only have this:

  • morphTo('commentable', 'type', 'id')

Proposal

The proposal is quite simple to achieve. However, we decided to give a motivation and a use case to support it.

In concrete, we would like to propose a signature of morphTo to match the remaining parameters of the MorphTo relationship.

Recall the signature of the morphTo relationship:

  • morphTo($name, $type, $id)

And the MorphTo constructor:

  • MorphTo(Builder $query, Model $parent, string $foreignKey, string $ownerKey, string $type, string $relation)

The proposal is to add, at least, the $ownerKey parameter to the morphTo relationship:

  • morphTo($name, $type, $id, $ownerKey)

screen shot 2017-05-12 at 15 34 47

So the mentioned problem would be solved by calling:

  • morphTo('commentable', 'type', 'id', 'commentable_id')

Let us know what is your opinion.

Thanks in advance! 🍻

References:
Karwin, Bill. SQL antipatterns: avoiding the pitfalls of database programming. Pragmatic Bookshelf, 2010.

@fntneves fntneves changed the title [Propostal] Custom morphTo ownerKey [Proposal] Custom morphTo ownerKey May 12, 2017
@v0ctor
Copy link

v0ctor commented Sep 14, 2017

Any news on this? 🙏

@v0ctor
Copy link

v0ctor commented Sep 21, 2017

This feature has been merged to the master branch, so it will be available in the next major version of Laravel. The proposal can be closed. Thanks, @fntneves!

@fntneves
Copy link
Author

Thank you for implementing this proposal!
I was waiting for a discussion to begin the development process. Still, it is awesome that you've implemented it! Congratulations @victordzmr, I'm glad to know this is useful! 😄

@antonkomarev
Copy link

antonkomarev commented Sep 21, 2017

Thank you @victordzmr! The only missing feature in 5.5 for me =)
Special thanks to @fntneves for the proposal!

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

No branches or pull requests

3 participants