music_and_cities
music_and_cities

Reputation: 147

Can I disable delete() in django-rest-famework by not calling object.delete()

I have a pattern that I use:

class View(generics.RetrieveUpdateDestroyAPIView):

...

def delete(self, request, pk):
    if cantDelete():
       return JsonResponse({ 'success' : False, 'message': "Can't delete this"})
    self.get_object().delete()
    return JsonResponse({ 'success' : True, 'message': "Deleted"})

I have a reason to believe that the objects are sometimes being deleted if cantDelete() is true. Is this a bad pattern? Can't I disable delete() in this way?

I will do some experiments, but it's a production back end and a bit of an emergency, so if anyone can chime in with an answer before I get a chance to post an answer myself, it would be much appreciated.

Upvotes: 0

Views: 1596

Answers (3)

jackotonye
jackotonye

Reputation: 3853

You could call .perform_destroy if the can_delete is True

class ConditionalDestroyMixin(object):

    def destroy(self, request, *args, **kwargs):
        instance = self.get_object()
        can_delete = instance.can_delete()
        if can_delete:
            self.perform_destroy(instance)
            return Response(status=status.HTTP_204_NO_CONTENT)

        msg = _(f'Unable to delete {instance}.')
        data = {'detail': six.text_type(msg)}
        return Response(data, status=status.HTTP_403_FORBIDDEN)

Upvotes: 0

ostergaard
ostergaard

Reputation: 3507

I would do something like this:

def perform_destroy(self, conversation):
    if conversation.messages.exists():
        return Response({'status': 'conversation has messages'}, status=status.HTTP_400_BAD_REQUEST)
    else:
        conversation.delete()

But to your question, I would say that if objects are being deleted when cantDelete() is True then they are not being deleted by that delete function.

Upvotes: 0

music_and_cities
music_and_cities

Reputation: 147

According to this post https://stackoverflow.com/a/37308092/2073793 a better pattern would be:

class View(generics.RetrieveUpdateDestroyAPIView):

   ...

   def destroy(self, request, *args, **kwargs):
       if cantDelete():
          return JsonResponse({ 'success' : False, 'message': "Can't delete this"}, status = 403)
       self.perform_destroy(self.get_object())
       return JsonResponse({ 'success' : True, 'message': "Deleted"})

That is, override the destroy method (not the delete method), and then call perform_destroy() rather than the object's delete() method to actually delete the object if it's allowed.

But I still don't know whether the original code is problematic or not.

Upvotes: 1

Related Questions