Django model: delete() not triggered

Question

I have a model:

class MyModel(models.Model):
 ...
    def save(self):
        print "saving"
        ...
    def delete(self):
        print "deleting"
        ...

The save()-Method is triggered, but the delete() is not. I use the latest svn-Version (Django version 1.2 pre-alpha SVN-11593), and concerning the documentation at http://www.djangoproject.com/documentation/models/save_delete_hooks/ this should work. Any ideas?

Neman

Perhaps you're not deleting anything? Can you show us what you're invoking delete() on? – Generatrix

I tried it by just deleting an item in the admin area, and did not call it manually. – Neman

Answer 1

I think you're probably using the admin's bulk delete feature, and are running into the fact that the admin's bulk delete method doesn't call delete() (see the related ticket).

I've got round this in the past by writing a custom admin action for deleting models.

If you're not using the admin's bulk delete method (e.g. you're clicking the delete button on the object's edit page) then something else is going on.

See the warning here:

The “delete selected objects” action uses QuerySet.delete() for efficiency reasons, which has an important caveat: your model’s delete() method will not be called.

If you wish to override this behavior, simply write a custom action which accomplishes deletion in your preferred manner – for example, by calling Model.delete() for each of the selected items.

For more background on bulk deletion, see the documentation on object deletion.

My custom admin model looks like this:

from photoblog.models import PhotoBlogEntry
from django.contrib import admin    

class PhotoBlogEntryAdmin(admin.ModelAdmin):
    actions=['really_delete_selected']

    def get_actions(self, request):
        actions = super(PhotoBlogEntryAdmin, self).get_actions(request)
        del actions['delete_selected']
        return actions

    def really_delete_selected(self, request, queryset):
        for obj in queryset:
            obj.delete()

        if queryset.count() == 1:
            message_bit = "1 photoblog entry was"
        else:
            message_bit = "%s photoblog entries were" % queryset.count()
        self.message_user(request, "%s successfully deleted." % message_bit)
    really_delete_selected.short_description = "Delete selected entries"

admin.site.register(PhotoBlogEntry, PhotoBlogEntryAdmin)

Virgilio

yup, that's it, thanks a lot. could you shortly explain how your custom admin method looks like? – Neman

btw - there may well be more elegant ways of accomplishing it, but it works! – Virgilio

This works great. One thing I notice is that this doesn't display the "are you sure?" page that was there previously. Any thoughts? – Counterreply

@Counterreply - go take a look at code.djangoproject.com/browser/django/trunk/django/contrib/… - you can probably just override the bit you need (currently line 47 -queryset.delete()) - using something similar to the advice on extending generic views (b-list.org/weblog/2006/nov/16/…) - i.e. write a method to do the initial setup and handle the post, and otherwise, just delegate to django.contrib.admin.actions.delete_selected. If that doesn't make sense, ask a question for more help, and ping me in this comment thread. – Virgilio

I'm having trouble making sense of the ticket you linked to. It looks like it was fixed 3 years ago? – Psychotechnics

@Psychotechnics - the ticket I mentioned was to fix the documentation, which is now done (take a look at the warning on docs.djangoproject.com/en/dev/ref/contrib/admin/actions). – Virgilio

The main issue with this approach is that it ignores the checking "do you really want to delete this" interstitial that the original admin functionality provides. – Humanly

Answer 2

I know this question is ancient, but I just ran into this again and wanted to add that you can always move your code to a pre_delete or post_delete signal like so:

from django.db.models.signals import pre_delete
from django.dispatch.dispatcher import receiver

@receiver(pre_delete, sender=MyModel)
def _mymodel_delete(sender, instance, **kwargs):
    print("deleting")

It works with the admin's bulk delete action (at least as of 1.3.1).

Dried

very slick. Just wondering is there any improvement with this in django 1.4 ? – Flaunch

thnx a ton! I used post_delete to avoid recursive deletes. – Drida

Answer 3

The bulk action of the admin calls queryset.delete().

You could override the .delete() method of the queryset, so it always does a 1-by-1 deletion of objects. For example:

in managers.py:

from django.db import models
from django.db.models.query import QuerySet

class PhotoQuerySet(QuerySet):
    """ Methods that appear both in the manager and queryset. """
    def delete(self):
        # Use individual queries to the attachment is removed.
        for photo in self.all():
            photo.delete()

In models.py:

from django.db import models

class Photo(models.Model):
    image = models.ImageField(upload_to='images')

    objects = PhotoQuerySet.as_manager()

    def delete(self, *args, **kwargs):
        # Note this is a simple example. it only handles delete(),
        # and not replacing images in .save()
        super(Photo, self).delete(*args, **kwargs)
        self.image.delete()

Disintegration

Out of all the alternatives I've tried to capture and modify the behaviour of the Bulk Delete action in Django's admin, this is the only one that actually worked! – Mediation

One thing though, this can be simplified (at least on Django 2.1) by removing the Mixin and implementing the delete method only in the QuerySet class and it'll work just fine. – Mediation

@SebastiánVansteenkiste thanks for mentioning this. I've updated the code example to modern Django styles with QuerySet.as_manager(). No need for mixins indeed. – Disintegration

Nice! I didn't know that syntax existed! I checked the docs and it's already present in 2.1 even! – Mediation

Answer 4

Using django v2.2.2, I solved this problem with the following code

models.py

class MyModel(models.Model):
    file = models.FileField(upload_to=<path>)

    def save(self, *args, **kwargs):
        if self.pk is not None:
            old_file = MyModel.objects.get(pk=self.pk).file
            if old_file.path != self.file.path:
                self.file.storage.delete(old_file.path)

        return super(MyModel, self).save(*args, **kwargs)

    def delete(self, *args, **kwargs):
        ret = super(MyModel, self).delete(*args, **kwargs)
        self.file.storage.delete(self.file.path)
        return ret

admin.py

class MyModelAdmin(admin.ModelAdmin):

    def delete_queryset(self, request, queryset):
        for obj in queryset:
            obj.delete()

For the DefaultAdminSite the delete_queryset is called if the user has the correct permissions, the only difference is that the original function calls queryset.delete() which doesn't trigger the model delete method. This is less efficient since is not a bulk operation anymore, but it keeps the filesystem clean =)

Kaylee

This is the correct answer IMHO for the recent versions of Django. Thanks! – Ankara

This answer is worked for me. I am using Django 2.1 – Uund

Answer 5

The main issue is that the Django admin's bulk delete uses SQL, not instance.delete(), as noted elsewhere. For an admin-only solution, the following solution preserves the Django admin's "do you really want to delete these" interstitial. vdboor's solution is the most general, however.

from django.contrib.admin.actions import delete_selected

class BulkDeleteMixin(object):
    class SafeDeleteQuerysetWrapper(object):
        def __init__(self, wrapped_queryset):
            self.wrapped_queryset = wrapped_queryset

        def _safe_delete(self):
            for obj in self.wrapped_queryset:
                obj.delete()

        def __getattr__(self, attr):
            if attr == 'delete':
                return self._safe_delete
            else:
                return getattr(self.wrapped_queryset, attr)

        def __iter__(self):
            for obj in self.wrapped_queryset:
                yield obj

        def __getitem__(self, index):
            return self.wrapped_queryset[index]

        def __len__(self):
            return len(self.wrapped_queryset)

    def get_actions(self, request):
        actions = super(BulkDeleteMixin, self).get_actions(request)
        actions['delete_selected'] = (BulkDeleteMixin.action_safe_bulk_delete, 'delete_selected', ugettext_lazy("Delete selected %(verbose_name_plural)s"))
        return actions

    def action_safe_bulk_delete(self, request, queryset):
        wrapped_queryset = BulkDeleteMixin.SafeDeleteQuerysetWrapper(queryset)
        return delete_selected(self, request, wrapped_queryset)


class SomeAdmin(BulkDeleteMixin, ModelAdmin):
    ...

Humanly