Skip to content

Add polymorphic has many #68

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

happycollision
Copy link

I tried to use the polymorphic_has_many in one of my resources files and discovered that the method for it doesn't actually exist. I only have a vague idea of how this works, but I am starting a PR regardless, because maybe others can help.

I am trying to implement a tagging system in a personal project, and I think this is the way I need to do it.

My first commit is a copy/paste job between has_many and polymorphic_belongs_to

  • confirm that the changes actually do anything (I have not run this code yet)
  • add tests

@richmolj
Copy link
Contributor

richmolj commented Feb 22, 2018

@happycollision this is fantastic to see, thanks so much! ❤️ 🙌

I've avoided this so far because the use case is a bit difficult for me to track down...I actually was struggling to think of the right test, which is why it's not there currently. Some scenarios I can think of:

  • If, say, a User belongs_to CreditCard which can be of type visa, mastercard, etc - a polymorphic belongs_to makes sense, but going the other way we can simply say CreditCard has_many :users, and there's no need for a polymorphic has_many.

  • If we're trying to say "Find all Products tagged with X", I typically do this with a filter, ie /products?filter[tag]=X.

  • A valid use case would be something like, "load this tag, and sideload any entities associated with this tag, whether they be Foos or Bars". I tend to think a separate model wrapping Foo and Bar might make more sense here. In polymorphic_belongs_to, we key off of a a group_by, typically a type column in Rails. In other words, group our objects by type, and query associations differently for each type. I'm not sure what we would key off of in the polymorphic_has_many case. Maybe Tag has type and we're loading multiple types of tags while sideloading their Products? I typically do this with an STI superclass, but I could see how that's not always possible.

I think there's definitely a valid use case here, and polymorphic_has_many is definitely an expectation of users. Maybe we could walk through exactly what we need to do in your case and then figure out the right pattern?

@happycollision
Copy link
Author

For me, it is your last option. I am creating a database with lots of taggable entities. I want to be able to get every tagged thing from the tag itself.

# Tag model
class Tag < ApplicationRecord
  has_many :taggings
  
  def taggables # Gives us all the objects in an array
    taggings.map { |x| x.taggable }
  end
end

# Tagging model (to facilitate many to many)
class Tagging < ApplicationRecord
  belongs_to :tag
  belongs_to :taggable, polymorphic: true
end

# Everything that needs to be tagged will have the following look
class Thing1 < ApplicationRecord
  has_many :taggings, as: :taggable
  has_many :tags, through: :taggings, as: :taggable
end

I spent tons of time trying to find a way to make this happen in JsonApi as easily as it happens in those model definitions. Can't figure it out. That's when I saw the polymorphic_has_many and I assumed that would be the way to do it. Ultimately, I want this:

{
  "data": {
    "type": "tags",
    "attributes": {
      "name": "Red"
    },
    "relationships": {
      "taggables": {
        "data": [
          { "type": "people", "id": "9" },
          { "type": "places", "id": "4" },
          { "type": "things", "id": "14" },
          { "type": "ideas", "id": "7" }
        ]
      }
    }
  }
}

I am not sure if that is a valid JsonApi document or not. If not, I don't actually want to do it.

@richmolj
Copy link
Contributor

OK, so I think this is maybe less polymorphic_has_many and more polymorphic_many_to_many. For instance, if you were OK exposing the taggings relationship you could do this:

# app/resources/tag_resource.rb
has_many :taggings,
  resource: TaggingResource,
  foreign_key: :tag_id,
  scope: -> { Tagging.all }
# app/resources/tagging_resource.rb
polymorphic_belongs_to :taggable,
  group_by: :taggable_type,
  groups: {
    'Person' => {
      scope: -> { Person.all },
      resource: PersonResource,
      foreign_key: :taggable_id
    },
    'Place' => {
      scope: -> { Place.all },
      resource: PlaceResource,
      foreign_key: :taggable_id
    }
  }

You might want to expose Taggable for independent reasons. But I think we should support not exposing it as well.

Following the explanation of allow_sideload I am thinking it could look something like this:

def polymorphic_many_to_many(association_name, group_by:, groups:, &blk)
  allow_sideload association_name, polymorphic: true, type: :polymorphic_many_to_many do
    scope do |tags|
      # ... code ...
      groups.each_pair do |type, config|
        allow_sideload type, parent: self, type: :belongs_to, <OTHER_OPTIONS> do
          scope do |tags|
              taggables = Taggable.where(tag_id: tags.map(&:id), taggable_type: type).pluck(:taggable_id)
              type.classify.constantize.where(id: taggable_ids)
          end
          # etc
        end
      end
    end
  end
end

This is rough pseudo-code but enough to explain what we're going for.

  • The top-level allow_sideload passes polymorphic: true. This lets the code know we're going to nest further allow_sideloads within it. Not that when we do nest another allow_sideload, we pass parent: self for the opposite purpose.
  • The groups are a hash, with a separate config for each type. Nest another allow_sideload for each type.
  • Figure out how we want to query this group. Here I hardcoded Taggable just to show an example - grabbing all taggable_ids for the given taggable_type and using the corresponding model (Person, Place, Thing, etc.) to actually query based on those taggable_ids.

I know this is getting complicated, such is the nature of these polymorphic sideloads that hit different tables. But hopefully this points you in the right direction of what we'd need to do.

@happycollision
Copy link
Author

I tried that polymorphic_belongs_to business before. But how do you define the taggings_controller then? I can't find a way to make strong_resources accept a polymorphic relationship.

Neither of the following works:

# tagging_controller.rb

  strong_resource :tagging do
    belongs_to :tag
    belongs_to :taggable # Resource with name taggable not registered
  end

  strong_resource :tagging do
    belongs_to :tag
    polymorphic_belongs_to :taggable # no such method
  end

@richmolj
Copy link
Contributor

You can specify the resource directly:

belongs_to :taggable, resource: :person

In practice, you probably do want a common :taggable SR template defined, which whitelists the allowed fields that Person/Place/Thing share (or at least, contains all possible attributes). Something that would make this easier is "inheritable" strong resources - like factory_bot has - but it's not on my near-term roadmap and might be better to be explicit anyway.

Remember, though, SR is only used for writes. I would personally go through the people endpoint and sidepost the tag. You could also go through the /tags endpoint but sidepost the relationships explicitly instead of polymorphism.

Let me know if any of that makes sense / works for you.

@happycollision
Copy link
Author

I think it's getting a little too "into the weeds" for me. I'll probably just avoid dealing with taggables from the tag side of things. The whole idea of the tagging system I am creating for my app is that very disparate things can share the same tag. So there won't really be any common attributes among them. When I revisit my code for a refactor after everything is working, I'll have to give this another go.

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.

2 participants