jsonapi-suite / jsonapi_compliable

MIT License
20 stars 35 forks source link

Add polymorphic has many #68

Open happycollision opened 6 years ago

happycollision commented 6 years ago

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

richmolj commented 6 years ago

@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:

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 commented 6 years ago

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 commented 6 years ago

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.

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 commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

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.