django-json-api / django-rest-framework-json-api

JSON:API support for Django REST framework
https://django-rest-framework-json-api.readthedocs.org/
BSD 2-Clause "Simplified" License
1.18k stars 294 forks source link

Metadata on PolymorphicModelSerializer #706

Open Ardillen66 opened 4 years ago

Ardillen66 commented 4 years ago

PolymorphicModelSerializer does not implement get_root_meta() to populate the meta key of the response JSON. This might be by design, but it took me some time to figure this out and implement this method to delegate to the appropriate serializer. I implemented get_root_meta() on the serializer that is extending PolymorphicModelSerializer in a similar way to get_fields(). The code:

class MySerializer(serializers.PolymorphicModelSerializer):

    def get_root_meta(self, resource, many):
        if self.instance not in (None, []):
            if not isinstance(self.instance, QuerySet):
                serializer_class = self.get_polymorphic_serializer_for_instance(self.instance)
                return serializer_class(self.instance, context=self.context).get_root_meta(resource, many)
            else:
                raise Exception("Cannot get metadata from a polymorphic serializer given a queryset")
        return {}

Not sure if this would be the way to imlpement it on PolymorphicModelSerializer, but it works for me. I think it would be nice if this is provided by default, or the documentation mentions that you need to implement it yourself when using PolymorphicModelSerializer.

p.s. First time posting an issue, so let me know if something is not clear or I missed something.

sliverc commented 4 years ago

Thanks for reporting. This seems to have been missed when implementing polymorphic serailizers.

At a quick glance your code snippet looks good. It would be great if you could open a PR with a corresponding test.

4nickel commented 4 years ago

Hi, I'm interested in helping with this issue.

The code above sadly doesn't seem to work. The reason is that (in my case, at least) self.instance is a list and self.get_polymorphic_serializer_for_instance expects an object with a _meta attribute.

I'm happy to work on a PR, but if anybody more familiar with the code-base could provide some guidance as to how this is supposed to work, it would be much appreciated.

sliverc commented 4 years ago

@4nickel I do not quite see how self.instance can become a list. Are you able to reproduce this in the context of a DJA unit test?

sliverc commented 4 months ago

We want to think about how to continue supporting Django Polymorphic in the future. As this issue is about polymorphic support, I assume you are or have been using Django Polymorphic and DJA together. It would be very valuable if you could add your use case and feedback to our discussion at #1194 so we can make an informed decision. Thanks.