From 69759a98ae0ffbd5815f19bc93f69cb5a75deeda Mon Sep 17 00:00:00 2001 From: Roman Gorbil Date: Tue, 27 Dec 2016 15:46:15 +0700 Subject: [PATCH 1/2] Fix deletion of inherted models As referenced in #13, not all related objects should be deleted. But I think that ``ForeignKey`` should resolve it by itself, using ``on_delete`` param. The same applies to ``OneToOneField``. But there is another case with inherited models. Consider following case: ``` class BaseModel(UndeleteBase): name = models.CharField(default='Empty name', max_length=256) class InhertedModel(BaseModel): description = models.TextField(default='Some description') ``` In this case, when the instance of ``InhertedModel`` would be deleted, related objects will contain linked the instance of ``BaseModel`` and it's ``delete`` method will be called. Which collect instance of ``InhertedModel`` in related objects and call it's ``delete`` method. So we will have endless recursion. This commit fixes it --- pinax/models/models.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/pinax/models/models.py b/pinax/models/models.py index f219649..8e5f36a 100644 --- a/pinax/models/models.py +++ b/pinax/models/models.py @@ -25,7 +25,9 @@ def delete(self): to_delete = get_related_objects(self) for obj in to_delete: - obj.delete() + # check that model is not inherited to avoid circular deletion + if not issubclass(obj.__class__, self.__class__): + obj.delete() # Soft delete the object self.date_removed = timezone.now() From 9cc99f359f730fa0a27f3b1ccecff1ef3b201a9e Mon Sep 17 00:00:00 2001 From: Roman Gorbil Date: Thu, 26 Jan 2017 17:27:07 +0700 Subject: [PATCH 2/2] Fix deletion of related objects Previous fix worked just for inherited models, but fails with models that have ``ForeignKey('self')``. Now it fixed --- pinax/models/models.py | 22 ++++++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-) diff --git a/pinax/models/models.py b/pinax/models/models.py index 8e5f36a..92c73aa 100644 --- a/pinax/models/models.py +++ b/pinax/models/models.py @@ -20,13 +20,27 @@ def active(self): return self.date_removed is None active.boolean = True - def delete(self): + def delete(self, _collect_related=True): + """Soft-delete the object. + + Args: + _collect_related(bool): is deletion of this object requires to + collect and delete some related objects. + + `_collect_related` used with cascade deletion. Just root object + should collect related objects. If related objects will also start to + collect realted objects we may fail into endless recursion + """ # Fetch related models - to_delete = get_related_objects(self) + if _collect_related: + to_delete = get_related_objects(self) + else: + to_delete = [] for obj in to_delete: - # check that model is not inherited to avoid circular deletion - if not issubclass(obj.__class__, self.__class__): + if isinstance(obj, LogicalDeleteModel): + obj.delete(_collect_related=False) + else: obj.delete() # Soft delete the object