3

When creating an object initially I use the currently logged-in user to assign the model field 'owner'.

The model:

class Account(models.Model):

    id = models.AutoField(primary_key=True)
    owner = models.ForeignKey(User)
    name = models.CharField(max_length=32, unique=True)
    description = models.CharField(max_length=250, blank=True)

Serializer to set owner:

class AccountSerializer(serializers.ModelSerializer):
    class Meta:
        model = models.Account
        fields = ('name', 'description')

    def restore_object(self, attrs, instance=None):
        instance = super().restore_object(attrs, instance)

        request = self.context.get('request', None)
        setattr(instance, 'owner', request.user)

        return instance

It is possible for a different user in my system to update another's Account object, but the ownership should remain with the original user. Obviously the above breaks this as the ownership would get overwritten upon update with the currently logged in user.

So I've updated it like this:

class AccountSerializer(serializers.ModelSerializer):
    class Meta:
        model = models.Account
        fields = ('name', 'description')

    def restore_object(self, attrs, instance=None):
        new_instance = False
        if not instance:
            new_instance = True

        instance = super().restore_object(attrs, instance)

        # Only set the owner if this is a new instance
        if new_instance:
            request = self.context.get('request', None)
            setattr(instance, 'owner', request.user)

        return instance

Is this the recommended way to do something like this? I can't see any other way, but I have very limited experience so far.

Thanks

From reviewing @zaphod100.10's answer. Alternatively, in the view code (with custom restore_object method in above serializer removed):

def post(self, request, *args, **kwargs):

    serializer = self.get_serializer(data=request.DATA, files=request.FILES)

    if serializer.is_valid():
        serializer.object.owner = request.user
        self.pre_save(serializer.object)
        self.object = serializer.save(force_insert=True)
        self.post_save(self.object, created=True)
        headers = self.get_success_headers(serializer.data)

        return Response(serializer.data, status=status.HTTP_201_CREATED,
                        headers=headers)
    return Response(serializer.errors, status=status.HTTP_400_BAD_REQUEST)
dpwr
  • 2,732
  • 1
  • 23
  • 38

3 Answers3

2

Basically you want the owner to be set on creation and not on subsequent updates. For this I think you should set the owner in the POST view. I think it is more logical and robust that way. Update is done via PUT view so your data should always be correct since no way on updation the owner can be changed if the owner is not editable on PUT.

For making the views you can use DRF's generic class based views. Use the RetrieveUpdateDeleteView as it is. For ListCreateView override the post method. Use a django model form for validating the data and creating an account instance.

You will have to copy the request.DATA dict and insert 'owner' as the current user.

The code for the POST method can be:

def post(self, request, *args, **kwargs):
    data = deepcopy(request.DATA)
    data['owner'] = request.user
    form = AccountForm(data=data)
    if form.is_valid():
        instance = form.save(commit=false)
        instance.save()
        return Response(dict(id=instance.pk), status=status.HTTP_201_CREATED)
    return Response(form.errors, status=status.HTTP_400_BAD_REQUEST)
zaphod100.10
  • 3,331
  • 24
  • 38
  • I agree that it feels correct to have this in the post logic rather than in the serializer. The reason you use a form to validate this is just to validate the owner as well though? As that data doesn't actually come from the user that doesn't feel quite right to me. Have a look at my alternative in my updated question. Does that seem better/worse/the same as the form validation solution? – dpwr Apr 22 '14 at 13:33
  • Yes, you are correct I am using model form for validation. I think it is a matter of preference and situation. I think it is better to validate everything just to be sure. For example for some reason or bug the current user might be an anonymous user which does not have a foreign key. So in such a case the code will throw an error 'no foreign key' which needs to be handled. Otherwise your solution is same as mine. – zaphod100.10 Apr 22 '14 at 14:54
  • Ok, great. Thanks. I find very often with django that I'm inheriting a lot of behaviour (especially with Mixins it becomes a bit/lot confusing) so completely overriding anything is scary. – dpwr Apr 22 '14 at 15:12
1

Potential other option using pre_save which I think seems to be intended for just this kind of thing.

class AccountList(generics.ListCreateAPIView):
    serializer_class = serializers.AccountSerializer
    permission_classes = (permissions.IsAuthenticated)

    def get_queryset(self):
        """
        This view should return a list of all the accounts
        for the currently authenticated user.
        """
        user = self.request.user
        return models.Account.objects.filter(owner=user)

    def pre_save(self, obj):
        """
        Set the owner of the object to the currently logged in user as this
        field is not populated by the serializer as the user can not set it
        """

        # Throw a 404 error if there is no authenticated user to use although
        # in my case this is assured more properly by the permission_class
        # specified above, but this could be any criteria.
        if not self.request.user.is_authenticated():
            raise Http404()

        # In the case of ListCreateAPIView this is not necessary, but
        # if doing this on RetrieveUpdateDestroyAPIView then this may
        # be an update, but if it doesn't exist will be a create. In the
        # case of the update, we don't wish to overwrite the owner.
        # obj.owner will not exist so the way to test if the owner is
        # already assigned for a ForeignKey relation is to check for
        # the owner_id attribute
        if not obj.owner_id:
            setattr(obj, 'owner', self.request.user)

I think this is the purpose of pre_save and it is quite concise.

dpwr
  • 2,732
  • 1
  • 23
  • 38
  • 1
    As of [version 3](http://www.django-rest-framework.org/api-guide/generic-views/#genericapiview) of the Django Rest framework, the `pre_save` method and the other save/deletion hooks have been replaced by `perform_create`, `perform_update` and `perform_destroy`. – Emile Bergeron May 22 '15 at 02:44
0

Responsibilities should be split here, as the serializer/view only receives/clean the data and make sure all the needed data is provided, then it should be the model responsibility to set the owner field accordingly. It's important to separate these two goals as the model might be updated from elsewhere (like from an admin form).

views.py

class AccountCreateView(generics.CreateAPIView):
    serializer_class = serializers.AccountSerializer
    permission_classes = (permissions.IsAuthenticated,)

    def post(self, request, *args, **kwargs):
        # only need this
        request.data['owner'] = request.user.id
        return super(AccountCreateView, self).post(request, *args, **kwargs)

models.py

class Account(models.Model):
    # The id field is provided by django models.
    # id = models.AutoField(primary_key=True)

    # you may want to name the reverse relation with 'related_name' param.
    owner = models.ForeignKey(User, related_name='accounts') 
    name = models.CharField(max_length=32, unique=True)
    description = models.CharField(max_length=250, blank=True)

    def save(self, *args, **kwargs):
        if not self.id:
            # only triggers on creation
            super(Account, self).save(*args, **kwargs)

        # when updating, remove the "owner" field from the list
        super(Account, self).save(update_fields=['name', 'description'], *args, **kwargs)
Emile Bergeron
  • 17,074
  • 5
  • 83
  • 129