Skip to content
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

Is there an option to turn off the dynamic fields option (DynamicFieldsMixin) even when passing the query? #121

Closed
daidaitaotao opened this issue Dec 13, 2019 · 11 comments

Comments

@daidaitaotao
Copy link

We recently run into a situation that the children serializer break inside the to_representation method.
For example

class ParentSerializer(DynamicFieldsMixin, serializer.ModelSerializer):
    ...
    def to_representation(self, instance):
        representation = super().to_representation(instance)
        # Manually set the representation here. Imagine that we need to do some extra work that can't be put
        # into a serializer method field for some reason.
        # Because we pass `self.context`, any restql querying is using the parent query fields, so those fields
        # might not exist on the child, which raises a parser error.
        representation["children"] = ChildSerializer(data=instance.child, context=self.context)
class ChildSerializer(DynamicFieldsMixin, serializers.ModelSerializer)

As a result, ChildSerializer did not work because it did not pass correct data in the query params which were for the parent serializer. I wonder if we have an option to toggle dynamic field functionality when instantiating the serializer?

Thanks,
Tao

@yezyilomo
Copy link
Owner

yezyilomo commented Dec 13, 2019

I wonder if we have an option to toggle dynamic field functionality when instantiating the serializer?

You can remove DynamicFieldsMixin on your ChildSerializer if you don't want filtering to apply on a child serializer.

@ashleyredzko
Copy link
Collaborator

@yezyilomo What if there are other places where the ChildSerializer might be used with dynamic fields? Could we add an argument, like use_dynamic_fields that could be passed into the instance and checked?

@yezyilomo
Copy link
Owner

yezyilomo commented Dec 13, 2019

@yezyilomo What if there are other places where the ChildSerializer might be used with dynamic fields? Could we add an argument, like use_dynamic_fields that could be passed into the instance and checked?

I see your point, this could be very useful, though I wouldn't suggest the name of the argument to be use_dynamic_fields because that's implied when inheriting DynamicFieldsMixin, I think it should be something like disable_*, I can't find a good word to put on *.

@ashleyredzko
Copy link
Collaborator

@yezyilomo disable_dynamic_fields? 😉

@daidaitaotao
Copy link
Author

I like the idea, can we make it happen?

@yezyilomo
Copy link
Owner

@yezyilomo disable_dynamic_fields?

Kinda long but very descriptive, I think we can go with that. @daidaitaotao what's your opinion on this?

@yezyilomo
Copy link
Owner

I like the idea, can we make it happen?

Definitely, we are going to implement it.

@daidaitaotao
Copy link
Author

yeah disable_dynamic_fields sounds good, or we can have enable_dynamic_fields with the default value True and then only do enable_dynamic_fields = False when needed. But I am good with disable_dynamic_fields too. Thanks.

@yezyilomo
Copy link
Owner

But I am good with disable_dynamic_fields too.

Okay, disable_dynamic_fields it is.

@ashleyredzko
Copy link
Collaborator

This could also just be dynamic_fields and we could pass in a boolean (default to True) if we wanted to shorten it.

@yezyilomo
Copy link
Owner

yezyilomo commented Dec 13, 2019

This could also just be dynamic_fields and we could pass in a boolean (default to True) if we wanted to shorten it.

I think disable_dynamic_fields makes more sense and it's easy to predict that it's a boolean kwarg unlike dynamic_fields

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants