Delete method related object behavior is wrong

[jeffbowen]

If you have two models that inherit from the logicaldelete Model one with a GenericForeignKey pointing to the other and the other with a GenericRelation for the type of the other with a fields argument, when you delete the one with the GenericForeignKey, the other one will be deleted as well. If you delete the one with the GenericRelation, the other one will not be deleted. This is backwards from the expected behavior. If you do the same with two Django Models (not logicaldelete Models), the inverse behavior will occur. This is hard to talk about without an example so here one is. First here's a setup without django-logicaldelete.

from django.contrib.contenttypes.fields import GenericRelation
from django.contrib.contenttypes import generic
from django.contrib.contenttypes.models import ContentType
from django.db import models

class Comment(models.Model):
    content_type = models.ForeignKey(ContentType)
    object_id = models.PositiveIntegerField()
    content_object = generic.GenericForeignKey()

class Like(models.Model):
    content_type = models.ForeignKey(ContentType)
    object_id = models.PositiveIntegerField()
    object = generic.GenericForeignKey()

class Post(models.Model):
    comments = GenericRelation(Comment, related_query_name="posts")
    likes = GenericRelation(Like, related_query_name="posts")

With this setup, if you delete a post, all comments and likes pointing to it will be deleted. If you delete a like or a comment, the corresponding post will not be deleted. This is correct and expected behavior. Here's the same example but with the Comment and Post models (but not the Like model) inheriting from the django-logicaldelete model.

from django.contrib.contenttypes.fields import GenericRelation
from django.contrib.contenttypes import generic
from django.contrib.contenttypes.models import ContentType
from django.db import models
from logicaldelete.models import Model

class Comment(Model):
    content_type = models.ForeignKey(ContentType)
    object_id = models.PositiveIntegerField()
    content_object = generic.GenericForeignKey()

class Like(models.Model):
    content_type = models.ForeignKey(ContentType)
    object_id = models.PositiveIntegerField()
    object = generic.GenericForeignKey()

class Post(Model):
    comments = GenericRelation(Comment, related_query_name="posts")
    likes = GenericRelation(Like, related_query_name="posts")

With this setup, if you delete a post, neither its likes nor comments will be deleted (opposite of expected behavior). If you delete a like, its post will not be deleted (expected behavior). If you delete a comment, the corresponding post will be deleted (opposite of expected behavior).

You can just imagine the code for this next example but if you make the Post model inherit from Django's Model, not logicaldelete's Model, but leave Comment inheriting from logicaldelete's Model, when you delete a comment, the corresponding post is not deleted (expected behavior). When you delete a post, any corresponding comments are actually, not logically, deleted. That's a separate issue from this one but something I came across in the process.

Back to the issue at hand. I discovered this because when I added functionality to delete comments to my app, posts started getting deleted. The silver lining was that since my posts model was using logicaldelete I was able to restore them after I quickly discovered the problem.

Hope that's all clear. This appears to blame to @acangiani's 11/28/13 commit (https://github.com/paltman/django-logicaldelete/commit/31f16844dd98516b1f57e3913d0fdba3e5715aa8).

[paltman]

@jeffbowen thanks for this report. this seems like a perfect set of scenarios to create some tests for. I'll see if I can get to that soon.

[psychok7]

I believe this https://github.com/pinax/pinax-models/pull/11 would fix this behaviour since it uses django admin internal way of getting related models to delete (or not)

[kodeine]

any updates on this?

[psychok7]

@kodeine you can test and see if https://github.com/pinax/pinax-models/pull/11 solves this issue since its already merged