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

Potentially misleading documentation for .update() in Nested Serializers #2191

Closed
disdanes opened this issue Dec 3, 2014 · 4 comments
Closed

Comments

@disdanes
Copy link

disdanes commented Dec 3, 2014

Problem:
It seems as though the code listed under the .update() method for nested serializers in /docs/api-guide/serializers.md might be potentially incorrect. The variable 'user' does not exist in the method's context, and looking at the internals for the .update() method on the ModelSerializer class - it seems as though 'user' might really want to be 'instance'. I think the flow is that instance is the model object that you are potentially updating, and the validated data is what your going to potentially update the model object 'instance' with in the scope of the function. The earlier documentation on that page also seems to suggest that you want to be using 'instance' inside the nested .update() function.

Current Documentation:
Here's an example for an update() method on our previous UserSerializer class.

def update(self, instance, validated_data):
    profile_data = validated_data.pop('profile')
    # Unless the application properly enforces that this field is
    # always set, the follow could raise a `DoesNotExist`, which
    # would need to be handled.
    profile = instance.profile

    user.username = validated_data.get('username', instance.username)
    user.email = validated_data.get('email', instance.email)
    user.save()

    profile.is_premium_member = profile_data.get(
        'is_premium_member',
        profile.is_premium_member
    )
    profile.has_support_contract = profile_data.get(
        'has_support_contract',
        profile.has_support_contract
     )
    profile.save()

    return user

Because the behavior of nested creates and updates can be ambiguous, and may require complex dependancies between related models, REST framework 3 requires you to always write these methods explicitly. The default ModelSerializer .create() and .update() methods do not include support for writable nested representations.

Potential Rework:

def update(self, instance, validated_data):
    profile_data = validated_data.pop('profile')
    # Unless the application properly enforces that this field is
    # always set, the follow could raise a `DoesNotExist`, which
    # would need to be handled.
    profile = instance.profile

    instance.username = validated_data.get('username', instance.username)
    instance.email = validated_data.get('email', instance.email)
    instance.save()

    profile.is_premium_member = profile_data.get(
        'is_premium_member',
        profile.is_premium_member
    )
    profile.has_support_contract = profile_data.get(
        'has_support_contract',
        profile.has_support_contract
     )
    profile.save()

    return instance
@tomchristie
Copy link
Member

user there should read instance, yes.

@houkk
Copy link

houkk commented Mar 20, 2015

one to many ,how to override the '.update()' method

@houkk
Copy link

houkk commented Mar 20, 2015

@tomchristie when many=True,how to override the '.update()' method

@xordoquy
Copy link
Collaborator

@houkk as you did when you hadn't many=True.
Please note that the issue tracker isn't a support forum. Please use the mailing list, IRC or stack overflow instead.

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

No branches or pull requests

4 participants