New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Delete method related object behavior is wrong #9

Open
jeffbowen opened this Issue Feb 26, 2015 · 4 comments

Comments

Projects
None yet
5 participants
@jeffbowen
Contributor

jeffbowen commented Feb 26, 2015

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 (31f1684).

@paltman

This comment has been minimized.

Show comment
Hide comment
@paltman

paltman May 25, 2015

Member

@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.

Member

paltman commented May 25, 2015

@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

This comment has been minimized.

Show comment
Hide comment
@psychok7

psychok7 Aug 5, 2015

Contributor

I believe this #11 would fix this behaviour since it uses django admin internal way of getting related models to delete (or not)

Contributor

psychok7 commented Aug 5, 2015

I believe this #11 would fix this behaviour since it uses django admin internal way of getting related models to delete (or not)

@kodeine

This comment has been minimized.

Show comment
Hide comment
@kodeine

kodeine Jun 9, 2016

any updates on this?

kodeine commented Jun 9, 2016

any updates on this?

@psychok7

This comment has been minimized.

Show comment
Hide comment
@psychok7

psychok7 Jun 9, 2016

Contributor

@kodeine you can test and see if #11 solves this issue since its already merged

Contributor

psychok7 commented Jun 9, 2016

@kodeine you can test and see if #11 solves this issue since its already merged

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment