Skip to content

get_related_resource_type failing for polymorphic serializers #621

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

Closed
favll opened this issue May 8, 2019 · 4 comments · Fixed by #767
Closed

get_related_resource_type failing for polymorphic serializers #621

favll opened this issue May 8, 2019 · 4 comments · Fixed by #767

Comments

@favll
Copy link

favll commented May 8, 2019

We have run into a problem where DRF JSON API resolves the wrong related resource type when trying to include a resource using a polymorphic serializer. Specifically, DRF JSON API will resolve the base model instead of the inherited model.

Below is an abbreviated example for the issue we are running into. In the following example utils.get_related_resource_type will resolve the parent_model to models.Product instead of models.ArtProduct (line here). Subsequently, the method will try to access art_details on the parent_model which will fail since it only exists on models.ArtProduct and not models.Product (line here).

A proposed fix would be to resolve the parent_model by checking if the parent_serializer is a polymorphic serializer and resolving the parent_model with something along the lines of:
parent_serializer.get_polymorphic_serializer_for_instance(parent_serializer.instance).Meta.model

If desired, I can provide further clarification and prepare a PR.

class ArtProductSerializer(serializers.ModelSerializer):
    price = MoneyField(max_digits=19, decimal_places=4)

    included_serializers = {
        "art_details": ArtDetailSerializer,
    }

    class JSONAPIMeta:
        included_resources = ("art_details",)
        resource_name = "ArtProduct"

    class Meta:
        fields = (
            "id",
            "price",
            "art_details",
        )
        model = models.ArtProduct


class PolymorphicProductSerializer(serializers.PolymorphicModelSerializer):
    polymorphic_serializers = [ArtProductSerializer]

    class Meta:
        model = models.Product


class CartItemSerializer(serializers.ModelSerializer):

    included_serializers = {
        "product": PolymorphicProductSerializer,
    }

    class Meta:
        model = models.CartItem
        fields = ("id", "product")


class CartSerializer(serializers.ModelSerializer):
    included_serializers = {
        "items": CartItemSerializer,
    }

    class Meta:
        model = models.Cart
        fields = ("id", "items")
        read_only_fields = ("id", "items")

    class JSONAPIMeta:
        included_resources = ("items", "items.product")
@sliverc
Copy link
Member

sliverc commented May 14, 2019

It would be great if you could open a PR which reproduces this error and a potential fix.

Once a reproducing test is ready it might also be clearer whether your suggested fix works or whether there are better solutions.

@ro70
Copy link
Contributor

ro70 commented May 30, 2019

@favll Probably I ran into the same problem. Any news on this?

@David-Guillot
Copy link
Contributor

David-Guillot commented Jun 25, 2019

@favll @ro70 did you make sure you're properly using the PolymorphicResourceRelatedField as indicated in the docs?

Defining a polymorphic serializer is enough when using it directly for its model endpoint, but for related models, as in your example @favll , a PolymorphicResourceRelatedField referring to your polymorphic serializer should do the trick, i'm seeing it working right now on my own project and you can see it implemented in the example app.

@sliverc
Copy link
Member

sliverc commented Sep 18, 2019

As there hasn't been any feedback and as of comment of @David-Guillot I assume this issue has been solved so closing. Please don't hesitate to comment if this issue still exists though.

@sliverc sliverc closed this as completed Sep 18, 2019
sliverc pushed a commit that referenced this issue Feb 7, 2020
Fixes #621 

When adding a ResourceRelatedField to a serializer that is included in the polymorphic_serializers
list of a polymorphic serializer, the parent_model is not correctly resolved in get_related_resource_type: instead of resolving to the inherited type, the base type is resolved. This can cause an AttributeError if the field in question is not present on the base model.

We ran into this, and then found the closed and seemingly abandoned issue referenced above. This PR implements the fix suggested by the original author, who deserves all the credit. The example code has been expanded to trigger the error when running the test-suite.

Thanks in advance for your time and please let us know if you have any suggestions for improvement.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants