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

Is morph_to feature equivalent to Orator ones ? #224

Closed
girardinsamuel opened this issue Oct 25, 2020 · 9 comments
Closed

Is morph_to feature equivalent to Orator ones ? #224

girardinsamuel opened this issue Oct 25, 2020 · 9 comments
Labels
question Open question that needs to be answered by contributors or maintainers

Comments

@girardinsamuel
Copy link
Contributor

girardinsamuel commented Oct 25, 2020

Opened this issue to clarify the use of morph_to.
I was using this with Orator:

# migration for creating table notifications
# ..
table.increments("id")
table.morphs("notifiable") # == any model than can be notified such as a User

# and on Notification Model
class Notification(Model):

    @morph_to
    def notifiable(self):
        """Get the notifiable entity that the notification belongs to."""
        return

# on a Notifiable model such as User

class User(Model)

    @morph_many('notifiable')
    def notifications(self):
         return

Then I can call:

user.notifications() # should return all notifications a user is linked to

n = Notification.create(notifiable_id=user.id, notifiable_type="users")

n.notifiable # should return the User instance linked to the notifications

So now with ORM, I figured I could do what is below but I can't figure out:

  • what column using in the Notification migration for notifiable. Is there such a column ? Or is this simply ORM level stuff ?
  • how should I revert the relation to get user.notifications() ?
  • should I define morph_map or can it default table names ?
class Notification(Model)

    @morph_to("notifiable_type", "notifiable_id")
    def record(self):
        return self

morph_to.set_morph_map({"user": User, "other_notifiable": OtherNotifiable})

(I mainly looked here #168 for info)
@josephmancuso do you have some tips :) ?

@josephmancuso
Copy link
Member

morph_to is going from the polymorphic table to the actual record table.

To go back the other way I think you would just need a simple belongs_to.

Something like this:

class Like:

    # morphs to articles and comments. An article and a comment can both have likes
    @morph_to
    def record(self):
        pass

then on the article or comment model:

class Comment:

    @has_many('comment_id', 'record_id')
    def likes(self):
         return Likes.where('record_type', 'Comment')

Is there a better way to do this? Maybe I'm missing something

@girardinsamuel
Copy link
Contributor Author

girardinsamuel commented Oct 26, 2020

@josephmancuso Well seems okay to me. Other questions to make this work:

  • I guess we should create "record_type" and "record_id" on the Like table manually ? (maybe we could add a morphs method to do that as in Orator) => opened Add morphs helper to blueprint #228
  • Is calling morph_map mandatory ? Or if not, types will be set to table name by default ?

@josephmancuso
Copy link
Member

josephmancuso commented Oct 27, 2020

Is calling morph_map mandatory ? Or if not, types will be set to table name by default ?

@girardinsamuel currently its mandatory yeah. Do you know a way we can automatically guess the model?

@girardinsamuel
Copy link
Contributor Author

girardinsamuel commented Oct 27, 2020

@josephmancuso In Django there is a helper which can return the model class from a string : "users.User" or "blog.Article".
It follows the pattern : "app_name.model_name". here we don't have the app part but, I guess we could use same idea for the model part + how autoload module is able to get and load classes in a specific folder.

The process would be:

  • get all classes in app/ and check if they inherit Model => build a list of all Model's app
  • fetch their __table__ argument : so we have ("articles", Article), ("users", User)
  • from that we could get the related model from the record_type field

@josephmancuso
Copy link
Member

That's typicaly how its done yeah. We usually store something like {namespace}.{model_name} so if you store you User model in app/models it would be app.models.User. Then we just fetch it. We can default to that. I know in Laravel it does that by default but if I want the ability to move my models around I usually just store it as User and then set something in the morph map:

<?php

use App\Models\User;
..->morphMap([
    'User': User::class
])

@girardinsamuel
Copy link
Contributor Author

girardinsamuel commented Oct 27, 2020

Or maybe we should just add a __morph_key__ attribute on the Model (default to __table__ value) and then, record_type would be set to this attribute.
this way the setting is close to the model and there is not need to call morph_map somewhere in different places for different models.

@girardinsamuel
Copy link
Contributor Author

@josephmancuso what do you think ?

@josephmancuso
Copy link
Member

umm if we added a morph key, how would we really know? we would have to register the morph map somehow. I think it would be easier to save the namespace and model name and then use pydoc to locate the model if there's no record in the morph map

@josephmancuso josephmancuso added the question Open question that needs to be answered by contributors or maintainers label Dec 11, 2020
@josephmancuso
Copy link
Member

I think this question has been answered. If you (or anybody) wants to come up with a proposed solution on enhancing the morph map here I'm all for it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Open question that needs to be answered by contributors or maintainers
Projects
None yet
Development

No branches or pull requests

2 participants