[Django] #22998: GenericRelation cascade deletion doesn't fire pre_delete signals

60 views
Skip to first unread message

Django

unread,
Jul 10, 2014, 3:52:02 PM7/10/14
to django-...@googlegroups.com
#22998: GenericRelation cascade deletion doesn't fire pre_delete signals
-------------------------------+--------------------
Reporter: gwahl@… | Owner: nobody
Type: Bug | Status: new
Component: Uncategorized | Version: 1.6
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------+--------------------
I have three models:

Node (gfk to)-> Content (fk to)-> Related

Deleting a Related cascades to delete the Content which cascades to delete
the Node (because Content has a GenericRelation). This is the expected
behavior. However, in Django 1.5 pre_delete would be trigger on Node
before deleting it. In 1.6, this is no longer the case.

I've written a test case here:
https://github.com/fusionbox/django/tree/generic_relation_cascade_signal.
The test passes on 1.5 and fails on 1.6, 1.7, and master.

This is a regression introduced in
97774429aeb54df4c09895c07cd1b09e70201f7d for #19385.

This may be related to #22594 -- both involve fast_delete incorrectly
returning True.

--
Ticket URL: <https://code.djangoproject.com/ticket/22998>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

Django

unread,
Jul 10, 2014, 4:20:50 PM7/10/14
to django-...@googlegroups.com
#22998: GenericRelation cascade deletion doesn't fire pre_delete signals
-------------------------------+--------------------------------------

Reporter: gwahl@… | Owner: nobody
Type: Bug | Status: new
Component: Uncategorized | Version: 1.6
Severity: Normal | Resolution:
Keywords: | Triage Stage: Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0
-------------------------------+--------------------------------------
Changes (by gwahl@…):

* needs_better_patch: => 0
* has_patch: 0 => 1
* needs_tests: => 0
* needs_docs: => 0


Comment:

Uploaded a patch to that branch. I'm not really sure if the logic is
correct, but all the tests pass.

--
Ticket URL: <https://code.djangoproject.com/ticket/22998#comment:1>

Django

unread,
Jul 10, 2014, 6:45:28 PM7/10/14
to django-...@googlegroups.com
#22998: GenericRelation cascade deletion doesn't fire pre_delete signals
-------------------------------------+-------------------------------------

Reporter: gwahl@… | Owner: nobody
Type: Bug | Status: new
Component: Database layer | Version: 1.6
(models, ORM) | Resolution:
Severity: Release blocker | Triage Stage: Accepted
Keywords: | Needs documentation: 0
Has patch: 1 | Patch needs improvement: 0
Needs tests: 0 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Changes (by timo):

* component: Uncategorized => Database layer (models, ORM)
* severity: Normal => Release blocker
* stage: Unreviewed => Accepted


Comment:

Qualifies for a backport to 1.6 since it's a regression from 1.5.

--
Ticket URL: <https://code.djangoproject.com/ticket/22998#comment:2>

Django

unread,
Jul 14, 2014, 6:28:38 AM7/14/14
to django-...@googlegroups.com
#22998: GenericRelation cascade deletion doesn't fire pre_delete signals
-------------------------------------+-------------------------------------

Reporter: gwahl@… | Owner: nobody
Type: Bug | Status: new
Component: Database layer | Version: 1.6
(models, ORM) | Resolution:
Severity: Release blocker | Triage Stage: Accepted
Keywords: | Needs documentation: 0
Has patch: 1 | Patch needs improvement: 0
Needs tests: 0 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------

Comment (by akaariai):

The code changes look good to me.

If you want to work on this: the tests should be included in
generic_relations_regress test module. Also, rename test_it() to
test_ticket_22998 for example. After this create a pull request.

I think we want to get this fixed for next RC of 1.7 which will hopefully
be released later this week. If you don't have time to work on this in the
next couple of days, please inform us as soon as possible.

--
Ticket URL: <https://code.djangoproject.com/ticket/22998#comment:3>

Django

unread,
Jul 14, 2014, 12:45:14 PM7/14/14
to django-...@googlegroups.com
#22998: GenericRelation cascade deletion doesn't fire pre_delete signals
-------------------------------------+-------------------------------------

Reporter: gwahl@… | Owner: nobody
Type: Bug | Status: new
Component: Database layer | Version: 1.6
(models, ORM) | Resolution:
Severity: Release blocker | Triage Stage: Accepted
Keywords: | Needs documentation: 0
Has patch: 1 | Patch needs improvement: 0
Needs tests: 0 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------

Comment (by gwahl@…):

https://github.com/django/django/pull/2916

--
Ticket URL: <https://code.djangoproject.com/ticket/22998#comment:4>

Django

unread,
Jul 16, 2014, 5:42:27 AM7/16/14
to django-...@googlegroups.com
#22998: GenericRelation cascade deletion doesn't fire pre_delete signals
-------------------------------------+-------------------------------------
Reporter: gwahl@… | Owner: nobody
Type: Bug | Status: closed

Component: Database layer | Version: 1.6
(models, ORM) | Resolution: fixed

Severity: Release blocker | Triage Stage: Accepted
Keywords: | Needs documentation: 0
Has patch: 1 | Patch needs improvement: 0
Needs tests: 0 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Changes (by Anssi Kääriäinen <akaariai@…>):

* status: new => closed
* resolution: => fixed


Comment:

In [changeset:"6e2b82fdf65c63c4a32620b7577f492579ecd91e"]:
{{{
#!CommitTicketReference repository=""
revision="6e2b82fdf65c63c4a32620b7577f492579ecd91e"
Fixed #22998 -- Updated the fast_delete logic for GFKs
}}}

--
Ticket URL: <https://code.djangoproject.com/ticket/22998#comment:5>

Django

unread,
Jul 16, 2014, 5:51:21 AM7/16/14
to django-...@googlegroups.com
#22998: GenericRelation cascade deletion doesn't fire pre_delete signals
-------------------------------------+-------------------------------------
Reporter: gwahl@… | Owner: nobody

Type: Bug | Status: closed
Component: Database layer | Version: 1.6
(models, ORM) | Resolution: fixed
Severity: Release blocker | Triage Stage: Accepted
Keywords: | Needs documentation: 0
Has patch: 1 | Patch needs improvement: 0
Needs tests: 0 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------

Comment (by Anssi Kääriäinen <akaariai@…>):

In [changeset:"72419ca8da8c6f472896754e9c01932900fd099d"]:
{{{
#!CommitTicketReference repository=""
revision="72419ca8da8c6f472896754e9c01932900fd099d"
[1.7.x] Fixed #22998 -- Updated the fast_delete logic for GFKs

Backport of 6e2b82fdf6 from master
}}}

--
Ticket URL: <https://code.djangoproject.com/ticket/22998#comment:6>

Django

unread,
Jul 16, 2014, 12:49:09 PM7/16/14
to django-...@googlegroups.com
#22998: GenericRelation cascade deletion doesn't fire pre_delete signals
-------------------------------------+-------------------------------------
Reporter: gwahl@… | Owner: nobody

Type: Bug | Status: closed
Component: Database layer | Version: 1.6
(models, ORM) | Resolution: fixed
Severity: Release blocker | Triage Stage: Accepted
Keywords: | Needs documentation: 0
Has patch: 1 | Patch needs improvement: 0
Needs tests: 0 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------

Comment (by gwahl@…):

Is this going to get backported to 1.6 too?

--
Ticket URL: <https://code.djangoproject.com/ticket/22998#comment:7>

Django

unread,
Jul 16, 2014, 1:26:49 PM7/16/14
to django-...@googlegroups.com
#22998: GenericRelation cascade deletion doesn't fire pre_delete signals
-------------------------------------+-------------------------------------
Reporter: gwahl@… | Owner: nobody

Type: Bug | Status: closed
Component: Database layer | Version: 1.6
(models, ORM) | Resolution: fixed
Severity: Release blocker | Triage Stage: Accepted
Keywords: | Needs documentation: 0
Has patch: 1 | Patch needs improvement: 0
Needs tests: 0 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------

Comment (by timo):

It was, but Trac didn't pick up the commit. I'll add to the release notes
now.

In 227a0f27a6927febc054cd90d17200203402c50d:

[1.6.x] Fixed #22998 -- Updated the fast_delete logic for GFKs

Backport of 6e2b82fdf6 from master

--
Ticket URL: <https://code.djangoproject.com/ticket/22998#comment:8>

Reply all
Reply to author
Forward
0 new messages