Opened 16 years ago

Closed 16 years ago

Last modified 16 years ago

#6565 closed (wontfix)

delete() method is not called on related objects

Reported by: Thomas Steinacher <tom@…> Owned by: nobody
Component: Database layer (models, ORM) Version: dev
Severity: Keywords:
Cc: Triage Stage: Design decision needed
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Consider the following models.py:

from django.db import models

class A(models.Model):
    pass

    def delete(self):
        print 'deleting a'
        super(A, self).delete()

class B(models.Model):
    a = models.ForeignKey(A)

    def delete(self):
        print 'deleting B'
        super(B, self).delete()

Test case:

In [1]: from test import models

In [2]: a = models.A.objects.create()

In [3]: b = models.B.objects.create(a=a)

In [4]: models.A.objects.all()
Out[4]: [<A: A object>]

In [5]: models.B.objects.all()
Out[5]: [<B: B object>]

In [6]: a.delete()
deleting a

In [7]: models.A.objects.all()
Out[7]: []

In [8]: models.B.objects.all()
Out[8]: []

When calling "a.delete()", the delete method of model B should be called, so the expected output is:

In [6]: a.delete()
deleting b
deleting a

Change History (4)

comment:1 by Malcolm Tredinnick, 16 years ago

Triage Stage: UnreviewedDesign decision needed

It's not clear that the delete() method on B *should* be called. The important thing is that the related objects are deleted, which works correctly. This isn't a subclassing relationship, so there's not a clear expectation that the corresponding method is called to actually do the deletion (it turns out to be less efficient than how we work now).

Needs some further thought, but I suspect this probably isn't a good idea. It introduces side-effects that could be difficult to control. If you want to do something when a related object is deleted, use the "delete" signal.

comment:2 by Thomas Steinacher <tom@…>, 16 years ago

I agree that not calling delete is more efficient. It looks like your suggestion to use signals is the best solution right now.

In my case, I need to call a function ALWAYS when an object is deleted. This requires me to write the following code for each affected model, which doesn't look that pretty:

class B(models.Model):
    a = models.ForeignKey(A)

    def pre_delete(self):
        print 'deleting b'

def delete_b(sender, instance, signal, *args, **kwargs):
    instance.pre_delete()

dispatcher.connect(delete_b, signal=signals.pre_delete, sender=B)

What about providing a simpler way to accomplish this?

comment:3 by Malcolm Tredinnick, 16 years ago

Resolution: wontfix
Status: newclosed

Beauty's in the eye of the beholder, but there's really no problem with the above code. Particularly as your delete_b method only has to be written once. You register the signal handler and write a handler function that does whatever you want. It can't get much simpler than that, unless Django automatically registered the signal handler for you (which isn't going to happen -- signal dispatching is not free; if you want it, you set it up and everybody else doesn't pay the penalty). You have to write pre_delete in any case (whether it's called pre_delete or delete doesn't really matter here) and delete_b will work for all instances, regardless of class type. So, basically, you need to write one signal registration call per model; one line of code. Not a huge extra burden that I can see.

What sort of simplification are you hoping for? Taking into account the above (no automatic signal registration), if you have a suggestion, please, let's hear it.

comment:4 by Thomas Steinacher <tom@…>, 16 years ago

I was hoping to do something like this:

class B(models.Model):
    a = models.ForeignKey(A)

    @signal(signals.pre_delete)
    def pre_delete(self):
        print 'deleting b'

Unfortunately this doesn't work as the decorator doesn't know to which class the function belongs.

Note: See TracTickets for help on using tickets.
Back to Top