django-polymorphic / django-polymorphic-tree

Polymorphic MPTT tree support for models
Other
170 stars 45 forks source link

wrong pre_delete handler is called with multiple model types. #53

Open korkmaz opened 8 years ago

korkmaz commented 8 years ago

Hi I guess I found a problem on pre_delete. Let me clarify with an example. Let's say we have a tree like

root_node
--cat_1
----cat_1_1
------course_node_1
--cat_2
----course_node_2
----cat_2_2
------course_node_3

and we have two signal handlers

@receiver(pre_delete, sender=CourseNode)
def course_node_deleted(sender, instance, **kwargs):
    ...

@receiver(pre_delete, sender=CategoryNode)
def category_node_deleted(sender, instance, **kwargs):
    ...

when we want to delete root_node, in other words, when we call root_node.delete() it calls its subtypes' delete as well. But django expects its all children will be homogenous, so it assumes that all of the objects' types are the same with the first one and so it takes the type of the first one. see https://github.com/django/django/blob/master/django/db/models/deletion.py#L168

So in the example above course_node_2 will be handled by category_node_deleted, because the first item in the same level of tree is a category_node.

Because django-polymorphic-tree promises that You can write Django models that form a tree structure where each node can be a different model type., it should be handled by django-polymorphic-tree.

what do you think?

vdboor commented 7 years ago

You're completely right that the signals shouldn't fire this way. They are likely firing with the base or derived model type, depending on how it's called. This is an issue within django-polymorphic in fact and it's hard to fix as the pre_delete is invoked deep within Django's Collector.delete() function.