Serializer update/create commit

331 views
Skip to first unread message

André Ericson

unread,
Feb 25, 2015, 12:18:07 AM2/25/15
to django-res...@googlegroups.com
Hello,

Would it make sense to add a commit argument to the update/create methods (similar to Form's save)?

Use case that I can think of would be for something in the lines of this:

class AccountSignUpSerializer(serializers.ModelSerializer):

    class Meta:
        model = Account

    def create(self, validated_data, commit=True):
        instance = super(AccountSignUpSerializer, self).create(validated_data, commit=False)
        instance.set_password(validated_data['password'])
        if commit:
            instance.save()
        return instance


this would avoid repetitive .save() from each class that implements create. I'm sorry if this was discussed before, a quick search in the mailing list didn't return anything.

Thanks,

André Ericson

Tom Christie

unread,
Feb 25, 2015, 6:43:59 AM2/25/15
to django-res...@googlegroups.com
There's nothing (majorly) wrong with doing that in your own codebase (although mutating but not saving model instances goes against this design principal) but it's not something we have any reason to need in the core codebase.

André Ericson

unread,
Feb 25, 2015, 11:27:10 AM2/25/15
to django-res...@googlegroups.com
So, correct implementation of the class used for the example (without the commit) in the current version of DRF would require to either call save() on the instance twice or duplicate create() code from ModelSerializer  and not make a call to super, correct?

Tom Christie

unread,
Feb 25, 2015, 3:18:58 PM2/25/15
to django-res...@googlegroups.com
> or duplicate create() code from ModelSerializer  and not make a call to super, correct?

Actually it's slightly different - you wouldn't be duplicating *any* code from model serializer, because that does two things:

1. Calls Model.objects.create(...)
2. Sets any many to many relations (which can only ever be populated once you have a saved instance)

In your case you don't want to do *either* of those, so you'd instead have...

    def create(self, validated_data, commit=True):
        return MyModel(**validated_data)

Note that as mentioned any many to many fields cannot ever be set until *after* you have a saved instance to associate them with, so if you have any of those you'd need to remove them, and handle them explicitly at the point you have saved the instance...

    def create(self, validated_data, commit=True):
        validated_data = validated_data.pop('some_m2m_relation')
        return MyModel(**validated_data) 

The above also gives you a bit of a clue as to why we *don't* expose `commit=False` as an option.

André Ericson

unread,
Feb 25, 2015, 11:42:21 PM2/25/15
to django-res...@googlegroups.com
I see your point. Thanks for taking the time explaining it.

--
You received this message because you are subscribed to a topic in the Google Groups "Django REST framework" group.
To unsubscribe from this topic, visit https://groups.google.com/d/topic/django-rest-framework/QcCcM2pDblQ/unsubscribe.
To unsubscribe from this group and all its topics, send an email to django-rest-fram...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
Reply all
Reply to author
Forward
0 new messages