@leo-naeka your code looks nice, I don't currently have a use for this in my own apps so I'd welcome usability feedback from the community.

Would it be better to set the polymorphic model in SETTINGS rather than detecting it in utils.POLYMORPHIC_ANCESTORS? Autodetecting it feels a little like magic that might trip someone up. "Why isn't polymorphism working?" followed by "Oh, I forgot to install that package". It also may be possible for someone to be using both packages.

Well, this is a pertinent question.

The arguments you raised seems not legitimate since utils.POLYMORPHIC_ANCESTORS checks for modules present in the environment, so if you forgot to install one of the modules, your models are not polymorphic in that sense. This is also a dynamically constructed tuple, so if both modules are available, anybody can use both seamlessly.

The main advantage of the auto-detection approach is to support these polymorphic modules without requiring extra configuration.
But this is definitely less flexible (other polymorphic modules like @adaptivdesign's one are simply ignored) and the whole thing would require extra maintenance and in some way, pollute the code.
So I'm totally open to declare these in a JSON_API_POLYMORPHIC_ANCESTORS setting and use this instead. That would require a little paragraph in the documentation.

If I see this correctly the whole 'POLYMORPHIC_ANCESTORS' is for autodetection right? If one would explictly define a ResourceRelatedField relation, with is_polymorphic=True this would work too right?

I believe this is a classic case of implicit vs explicit behaviour. Since we explicitly have to declare serializers for each model, I think it's natural practice to explicitly declare a relation if it has different behaviour.

This also enables different behaviour when dealing with list vs detail api calls. I personally can imagine situations where you do not want to access entities in a polymorphic manner when performing list calls, only when retrieving the details for an entity.

The is_polymorphic flag can even be renamed to something like resource_type_from_instance.

Ok, ofcourse what i mentioned only applies to polymorphic relations, I see that 'POLYMORPHIC_ANCESTORS' is ofcourse also used by the renderer. I believe this can be made explicit as well. As the method build_json_resource_obj is called at line:

294, 312, 443, 458 and all those lines have access to a serializer instance. Thus it should be possible to access a 'resource_type_from_instance' attribute defined in the serializer.

I'm willing to make a pull request if you guys want.

Any progress on this?

We need to use use relations with polymorphism our selves, but after a bit of testing this branch we noticed that it does not handle incoming data? It seems that the method ResourceRelatedField.to_internal_value still determines the expected resource name from the queryset. Which is the base model in many polymorphism cases. This needs to be sorted out based on the instances in some way. I think you should add some tests for receiving data with relations and make sure that it works.

I'm willing to put some effort into getting this landed.

👍 on tests for the create(POST) cases. Definitely want those to get those in.

In our app we use django-polymorphic in python land but not the api/frontend level. We just expose each sybtype as a separate resource. We may rework our client to use polymorphic behavior if this makes it in.

I do think the autodetection would break our current API, because some relations would be serialized to their real type and not their base type. That may be our issue, not an issue with this feature. It just really depends how people are building their API.

We recently made a simple field for our code to make it work with django-polymorphic. I'm a big fan of a more explicit approach with this field than doing lots of magic in ResourceRelatedField.

The fundamental reason for this to work is that we need to get the model name based on the object ID with the queryset. That will make the type verification work.

from rest_framework_json_api.exceptions import Conflict
from rest_framework_json_api.relations import ResourceRelatedField
from rest_framework_json_api.utils import get_resource_type_from_instance
class PolymorphicRelatedField(ResourceRelatedField):
 """Polymorphic Related Field.

 A field to ignore failed type verification in
 `ResourceRelatedField.to_internal_value`. Rest Framework JSON API does not
 support Django-polymorphic yet. Its type verification is based on
 retrieving resource name from `queryset.model` on the field. In our case
 that will always return the base model name, which will fail for sub models.
 """
 def to_internal_value(self, data):
 """Override to ignore `Conflict` exception.

 Since the resource name is retrieved from `queryset.model` on the
 field, we make sure it's ignored.
 """
 try:
 return super().to_internal_value(data)
 except Conflict:
 obj = self.queryset.filter(id=data['id']).first()
 if data['type'] != get_resource_type_from_instance(obj):
 raise
 # Luckily the Conflict exception is raised at the end so we can
 # explicitly call super on `ResourceRelatedField`, which is the
 # `PrimaryKeyRelatedField`.
 return super(ResourceRelatedField, self).to_internal_value(
 data['id'])

to_representation() will just work out of the box because django-polymorphic returns the correct sub type of the model. What do you think?

@stianpr I'm back on this PR.
I also prefer handling polymorphism with an opt-in approach, that's why there's both PolymorphicResourceRelatedField and PolymorphicModelSerializer and not so much magic put in ResourceRelatedField (the strict minimum to be inherited).
Would be much appreciated if anyone involved with polymorphic models plays a bit with this implementation and gives feedback (doc has been updated).

I use polymorphic and as stated in my previous comment I like opt-in.

Hmm, I'm still not convinced of why we need the JSON_API_POLYMORPHIC_ANCESTORS setting? Is it absolutely necessary to have the code in ResourceRelatedField.__init__? Can't we have all the logic in the PolymorphicResourceRelatedField?

I tried rebasing this on develop to see if it is usable in a project I'm working on currently. There are some issues with fields selection for both list and item views.

Any update on where this is at?

I was able to rebase this against develop without too much hassle. All drf-json-api tests are passing but I am seeing the same issue as ntadej in my tests.

File "/home/skidder/.venvs/django_bm_engines/local/lib/python2.7/site-packages/django/core/handlers/base.py", line 217, in _get_response response = self.process_exception_by_middleware(e, request) File "/home/skidder/.venvs/django_bm_engines/local/lib/python2.7/site-packages/django/core/handlers/base.py", line 215, in _get_response response = response.render() File "/home/skidder/.venvs/django_bm_engines/local/lib/python2.7/site-packages/django/template/response.py", line 109, in render self.content = self.rendered_content File "/home/skidder/.venvs/django_bm_engines/local/lib/python2.7/site-packages/rest_framework/response.py", line 72, in rendered_content ret = renderer.render(self.data, accepted_media_type, context) File "/home/skidder/.venvs/django_bm_engines/src/djangorestframework-jsonapi/rest_framework_json_api/renderers.py", line 459, in render fields = utils.get_serializer_fields(serializer) File "/home/skidder/.venvs/django_bm_engines/src/djangorestframework-jsonapi/rest_framework_json_api/utils.py", line 96, in get_serializer_fields fields = getattr(serializer.child, 'fields') File "/home/skidder/.venvs/django_bm_engines/local/lib/python2.7/site-packages/rest_framework/serializers.py", line 363, in fields for key, value in self.get_fields().items(): File "/home/skidder/.venvs/django_bm_engines/src/djangorestframework-jsonapi/rest_framework_json_api/serializers.py", line 211, in get_fields serializer_class = self.get_polymorphic_serializer_for_instance(self.instance) File "/home/skidder/.venvs/django_bm_engines/src/djangorestframework-jsonapi/rest_framework_json_api/serializers.py", line 225, in get_polymorphic_serializer_for_instance return cls._poly_model_serializer_map[instance._meta.model] AttributeError: 'list' object has no attribute '_meta'

The bulk of this conversation seems to have died off back in September of last year. Given that was 8 months ago and there are conflicts on this branch, I'd like to consider closing this PR unless it finds a champion who can bring it to closure.

Is that ok? What needs to be done to bring this to a conclusion?

The code looks pretty good to me. My primary concern is that addition of polymorphic support does not disrupt package users who have no need for polymorphic models. As far as I can tell, you've achieved that.

For the actually functionality, there are tests (hurray!), and I presume that you're scratching your own itch and have made this work for polymorphic models.

I think if we can do a bit of cleanup and extend some documentation at spots, this will be ready to merge. Thanks @leo-naeka and @astronouth7303!

Thank you for the review @mblayman, I'll work on it today.

@leo-naeka Is there another way you'd like your name spelled in AUTHORS?

@mblayman I think that's all your code comments. However, the documentation still needs improvement.

Thanks for the feedback!

Is there anything else that needs to happen?

I thought the generic relation support oddness was still in here and I was waiting on that, but I see that was cleaned up in 05cdb51.

Thanks so much to the both of you! I really like how this all came together. I'll see if I can get the publish permissions so I can cut a new release to get this feature out there.

Is there a reason that polymorphic "list" endpoints aren't supported (at least with the capability to display all of the fields of the parent model)?

There is an exception raised by the get_fields method of PolymorphicModelSerializer that just says "("Cannot get fields from a polymorphic serializer given a queryset")"

Also in the tests there is a "test_polymorphism_on_polymorphic_model_list_post" where you can POST to a polymorphic endpoint, but there is no "test_polymorphism_on_polymorphic_model_list" (ie. no GET project-list)

I've patched this by removing the line that throws the exception in get_fields and then using the following (I changed the models to fit the Project example)

class PolyListSerializer(serializers.ListSerializer):
 _poly_force_type_resolution = True
class PolymorphicProjectSerializer(serializers.PolymorphicModelSerializer):
 polymorphic_serializers = [ArtProjectSerializer, ResearchProjectSerializer]
 class Meta:
 model = ProjectSerializer
 list_serializer_class = PolyListSerializer

This returns a list where the individual items are listed with the proper "type", unlike before where they were all listed with the parent type.

I haven't tested this very thoroughly but thought I'd submit here for some feedback before digging any deeper.

For me this merge is breaking some of my existing code.

The problem is that at the end of the parser you add the type to the parsed data:

...
# Construct the return data
parsed_data = {'id': data.get('id')} if 'id' in data else {}
parsed_data['type'] = data.get('type')
parsed_data.update(self.parse_attributes(data))
...

In case you have a model that has a type attribute and that you send a request without this field it will assign the JSON API type to the field.

@leo-naeka As you were the initial initiator of the polymorphic support within DJA I wonder whether you are still using it nowadays? Would you mind joining our discussion on #1194 so we can get a better picture on how the polymorphic support is used nowadays? Thanks.

Anyone getting notified here and is still using polymorphic would also be great if you could join our discussion at #1194.