Skip to content

Allow specifying omit/fields params via the Serializer context #17

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

cb109
Copy link

@cb109 cb109 commented Sep 26, 2017

Why this

I really like the flexibility that drf-dynamic-fields provides on a request level, but I found it lacking an internal way to specify this directly on a Serializer object. I can use a crude hack to extend the wrapped _request.GET dict, but I would prefer this PR instead.

What it does

This allows passing omit/fields values to drf-dynamic-fields from a lower level API, useful when creating serializers manually, e.g. inside other serializers.

If both query_params and serializer.context specify the same parameters the context takes precedence, since it is the more internal interface.

How to use

context = {"request": request, "omit": "field1,field2"}
serializer = MyModelSerializer(data, context=context)

This allows passing omit/fields values to drf-dynamic-fields from a lower level API, useful when creating serializers manually, e.g. inside other serializers.
@cb109 cb109 force-pushed the feature/allow-specifying-fields-via-serializer-context branch from 4b4a3c1 to 817e49c Compare September 26, 2017 11:43
@jtrain
Copy link
Collaborator

jtrain commented Sep 26, 2017

@cb109 i mentioned in the other thread about my worries with a nested serializer. Especially for model serializers whose get_fields function is very slow.

Try for yourself, and see if a plain serializer without the dynamic fields is more performant.

Having said that, performance isn't everything, and you've done a good job re-using the existing code.

I worry that the interface (via context) isn't very declarative. Let's say you had a nested Serializer like so:

class Teacher(DynamicFieldMixin, ModelSerializer):
    students = StudentSerializer(many=True)

If that StudentSerializer were to only require name and id fields, how is it obvious from this? Are you overriding where the context is created in the Teacher to add your fields there?

or are you doing something like this:

class Teacher(DynamicFieldsMixin, ModelSerializer):
    students = serializers.SerializerMethodField()

    def get_students(self, teacher):
        context = self.context.copy()
        context['fields'] = 'id,name'
        return [StudentSerializer(student, context=context) for student in teacher.students.all()]

@cb109
Copy link
Author

cb109 commented Sep 27, 2017

@jtrain Good to know! I am always looking at serializer performance, since changing things there can easily break usability for me.

You are right, using the context is not very declarative, it was just an existing interface I could use with minimal changes. My use case is less nested serializers but more specific view functions where I create serializers manually and know which fields are not important. I am doing that similar to your last example. I don't want to have the client pass query_params, instead I am passing these myself inside the view currently by adding them to request._request.GET, which drf-dynamic-fields will pick up. I don't like that hack and find using the context and improvement to that.

@dbrgn
Copy link
Owner

dbrgn commented Nov 4, 2017

Currently blocked by #18.

@dbrgn dbrgn mentioned this pull request Nov 4, 2017
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

Successfully merging this pull request may close these issues.

3 participants