Active_model_serializers: JSONAPI - Wrong "type" for belongs_to polymorphic relationships

[furkanayhan]

Expected behavior vs actual behavior

For JSONAPI, after this pr is merged: #1857, type parameter returns the relation type, instead of resource type.

Steps to reproduce

serializer:

class CommentSerializer < ActiveModel::Serializer
  # ....
  belongs_to :commentable # post, image etc.
end

request:
/comments?include=commentable

expected:

//...
"relationships": {
  "commentable": {
    "data": {
      "id": "1",
      "type": "posts"
    }
  }
}
//...

result:

//...
"relationships": {
  "commentable": {
    "data": {
      "id": "1",
      "type": "commentables"
    }
  }
}
//...

Environment

ActiveModelSerializers Version, after this: #1857

[blmundie]

I have this same issue for >= 0.10.4

[blmundie]

For me removing 'oj' fixes it.

[NullVoxPopuli]

huh. I'm using OJ with 0.10.5 without this problem. I did experience this with the latest 0.10.6 though.

[andyhite]

I'm using 0.10.6 without oj and seeing this issue.

[beauby]

I cannot envision a single way OJ might change this behavior 🤔

[andyhite]

@beauby OJ has nothing to do with it. I've found the problem, I just can't decide how best to fix it - so I've hacked a fix in my private fork for now.

The problem is here: https://github.com/rails-api/active_model_serializers/pull/1857/files#diff-4ed94f8e25cfa5ca61c077e1dbd5daceR58

The default type for the resource identifier when using a belongs_to is the field's name (pluralized). That means if you have a relationship named organization that has a type of Company, the included resource will have the proper companies type, but the resource identifier for the relationship will have a type of organizations. The only way to fix this is to specify the type or class_name for the belongs_to, so that the default isn't used. However, if your resource uses STI then you're shit out of luck.

This is a pretty major issue and needs to be addressed ASAP.

[andyhite]

My temporary hack to fix this is the following:

# lib/active_model_serializers/adapter/json_api/relationship.rb:49
type = association.reflection.type ? association.reflection.type.to_s : association.lazy_association.serializer.json_key

# lib/active_model/serializer/reflection.rb:59-60
options[:class_name] ? options[:class_name].underscore.pluralize.to_sym : nil

I haven't tested anything other than my case though, so I'm not comfortable pull requesting it yet.

[blmundie]

Yes it wasn't oj. Had two issues at the same time. The newest version of oj with oj_mimick_json was causing my to_json to return an invalid string.

This issue I fixed by going to older version of ActiveModelSerializer.

Sorry for the confusion.

[j-arnaiz]

This do not solve the problem but as a workaround in your application you could change your belong_to :field, polymorphic: true to has_one :field, polymorphic: true and the type returned will be ok.

I tried to solve the problem... it seems like we should access attribute #{name}_type on polymorphic relationships on parent object, but on ::ActiveModel::Serializer::Reflection initialize method parent object is not initialized.

[burmashave]

Having this issue as well. Changing to has_one :field, polymorphic: true (per @j-arnaiz) does fix it for me, but would love to see belongs_to work as expected.

[ajahongir]

having same issue,
passing custom type: :versions but it still as model.table_name

[leadingproperties]

Having same issue.
Downgraded to 0.10.5

[Rio517]

I resolved this for myself with the attached patch.

It would be better if, rather than rely on the json_key as I have, I could expose the polymorphic_type variable as set on the following line, but I didn't have the time to sort out the control flow:

https://github.com/rails-api/active_model_serializers/blob/0-10-stable/lib/active_model/serializer/association.rb#L59

Diff:

--- a/lib/active_model_serializers/adapter/json_api/relationship.rb
+++ b/lib/active_model_serializers/adapter/json_api/relationship.rb
@@ -46,7 +46,11 @@ module ActiveModelSerializers
           if association.belongs_to? &&
               parent_serializer.object.respond_to?(association.reflection.foreign_key)
             id = parent_serializer.read_attribute_for_serialization(association.reflection.foreign_key)
-            type = association.reflection.type.to_s
+            if !association.polymorphic?
+              type = association.reflection.type.to_s
+            else
+              type = association.lazy_association.serializer.json_key
+            end
             ResourceIdentifier.for_type_with_id(type, id, serializable_resource_options)
           else
             # TODO(BF): Process relationship without evaluating lazy_association
-- 

[furkanayhan]

@Rio517 you may open a PR with this?

[turlockmike]

I downgraded and the issue was resolved. Is there not a workaround without a patch?

[andyhite]

I get the feeling the maintainers of active_model_serializers don't care that it's broken. It's been busted for at least 5 months, and no activity on the repo since. Seriously – no activity in this project at all since like May according to GitHub.

[NullVoxPopuli]

I highly recommend using http://jsonapi-rb.org/

[bf4]

@andyhite

I get the feeling the maintainers of active_model_serializers don't care that it's broken. It's been busted for at least 5 months, and no activity on the repo since.

More of just that maintainers tend to scratch their own itch, and I've been busy doing other things and no one has fixed it yet. Maybe part of the problem is ego because I worked really hard on the code in between 0.10.5 and 0.10.6 and don't really want to yank it. I'd rather fix it. But I need help.

[andyhite]

@bf4 sorry, I didn't mean to come across as rude as I did. It's just a pretty big issue and makes 0.10.6 unusable for a lot of people. I understand you worked hard on the code between 0.10.5 and 0.10.6, which should be good reason to fix it so we can all reap the benefits of 0.10.6, haha.

I spent about a day trying to figure it out, and came up with the band-aid I mentioned in a comment back in June (which I'm unfortunately running in production via my fork), but it unfortunately isn't a complete solution and breaks something that I'm not currently using (can't remember what, off the top of my head) - so it's fine for my case, but not something that can be merged upstream.

[andyhite]

If I have time to circle back to this, I'll see if I can come up with a complete solution...but I was hoping that someone with more knowledge of the AMS code would be able to take the cause of the problem (that I pointed out a few months ago), and my partial solution, to fill in the gaps because I took it about as far as I could given the time I could budget for it.

[bf4]

@andyhite

I think between https://github.com/rails-api/active_model_serializers/pull/1857/files#r124965507 and writing a failing test, we could do it together.

[bf4]

I think what it needs in the context of this problem is a polymorphic: true flag to tell AMS that we can only know the object type from an instance of it, and can't get it from the name of the association.

as @Rio517 wrote above:

assocation.reflection.type for association.polymorphic? belongs to should be something like type = association.lazy_association.serializer.json_key or maybe serializer._type || serializer.object.class.model_name.to_s.underscore (and unify the logic with the ResourceIdentifier which does serializer.object.class.name.underscore (should use model_name) )

[bf4]

Fixed by #2211