[7.x] HasManyField support for MorphMany #533
Conversation
duncanmcclean
changed the title
|
Can you please provide some more context around this PR? What was broken, what errors were you getting, how do the changes in this PR address that? |
|
Hi Duncan, The issue this PR addresses was that currently MorpMany is not supported by the module. This PR adds support by checking if the relation uses morph and sets the required "type" next to the currently stored "id". If you need further information please let me know! |
Oh, I see, thanks! 👍 |
|
Currently, Runway doesn't really support Morph relationships. When I last investigated adding support for them, I figured it would take quite a bit of refactoring across various parts of the code for them to be supported properly. For example: we'd need to allow users to select models from multiple models, instead of just the model defined in the field's config. That doesn't seem like such a big deal at first but we'd have to figure out which columns to display in the selector stack since not all models have the same sets of columns. We'd also need to make sure that anywhere we resolve relationships, we take the So, while I'm thankful for this pull request, I'd prefer if we added proper support for Morph relationships everywhere, rather than just "patching" it in the places people come accross. I hope my explanation makes sense! Feel free to upvote the feature request, #245. Until we add proper support for morphs, you could potentially look at applying this fix to your project using a composer patch. |
|
Was thinking about this, and wouldn't it be possible to handle the type with a select field. My PR only handles the case where a Model having a morphMany to a Model having a MorphTo. In this case the owner is know as it is the model where working from. The case your describing would come into play when creating an object that has a MorphTo relation. In my opinion thats a different functionality out of scope for this PR. I do agree that it would be the most optimal to support the whole spectrum of morph options all together. But as this is such a simple addition wouldn't it be nice if the morph many part is already covered? FYI i've updated the patch with the new Relationships class -> https://github.com/TheBnl/runway/tree/morphmany In the meantime i'll try to find a solution for the MorphTo case when I need id. |
|
@duncanmcclean We're having the same issues. It looks like the patch @TheBnl made is working for us. |
Add support for MorphMany.