Skip to content

Pass on instance when using polymorphic serializers #759

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
josebama opened this issue Jan 14, 2020 · 1 comment · Fixed by #764
Closed

Pass on instance when using polymorphic serializers #759

josebama opened this issue Jan 14, 2020 · 1 comment · Fixed by #764

Comments

@josebama
Copy link
Contributor

I'm trying to understand the code in PolymorphicModelSerializer and I wonder if this is a bug or me just not understanding something, but in PolymorphicModelSerializer.to_internal_value, in the end, when in inits serializer_class like so: return serializer_class(data, context=self.context, partial=self.partial).to_internal_value(data), why is it passing data as the first argument?

For what I see, the init in BaseSerializer is expecting the first argument to be instance and the second one to be data: def __init__(self, instance=None, data=empty, **kwargs):. Wouldn't it make more sense for PolymorphicModelSerializer.to_internal_value to init serializer_class this other way: return serializer_class(self.instance, data, context=self.context, partial=self.partial).to_internal_value(data)?

That way the child serializer will keep the instance if there is any and will set the initial_data too.

@sliverc
Copy link
Member

sliverc commented Jan 18, 2020

I have looked at the code as well and this seems invalid indeed. Usually this is not noticed as it only affects serializers which internally access self.instance in case of patch call. However this is a bug and wouldn't need to be changed as you have suggested.

PR is welcome.

@sliverc sliverc changed the title Why does PolymorphicModelSerializer init serializer_class with data as the first argument? Pass on instance when using polymorphic serializers Jan 18, 2020
josebama added a commit to novastone-media/django-rest-framework-json-api that referenced this issue Jan 22, 2020
sliverc pushed a commit to sliverc/django-rest-framework-json-api that referenced this issue Jan 24, 2020
josebama added a commit to novastone-media/django-rest-framework-json-api that referenced this issue Jan 27, 2020
sliverc pushed a commit that referenced this issue Jan 30, 2020
Fixes #759 

Pass self.instance as the first parameter when initializing the child serializer from a polymorphic serialiser. This does not affect basic functionality, but makes the child serializer instance consistent with its usage of instance and data, which can fix potential issues when extending serilaizers that are part of a polymorphic serializer.
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.

2 participants