Opened 9 months ago

Closed 9 months ago

Last modified 9 months ago

#22998 closed Bug (fixed)

GenericRelation cascade deletion doesn't fire pre_delete signals

Reported by: gwahl@… Owned by: nobody
Component: Database layer (models, ORM) Version: 1.6
Severity: Release blocker Keywords:
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

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.

Change History (8)

comment:1 Changed 9 months ago by gwahl@…

  • Has patch set
  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

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

comment:2 Changed 9 months ago by timo

  • Component changed from Uncategorized to Database layer (models, ORM)
  • Severity changed from Normal to Release blocker
  • Triage Stage changed from Unreviewed to Accepted

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

comment:3 Changed 9 months ago 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.

comment:5 Changed 9 months ago by Anssi Kääriäinen <akaariai@…>

  • Resolution set to fixed
  • Status changed from new to closed

In 6e2b82fdf65c63c4a32620b7577f492579ecd91e:

Fixed #22998 -- Updated the fast_delete logic for GFKs

comment:6 Changed 9 months ago by Anssi Kääriäinen <akaariai@…>

In 72419ca8da8c6f472896754e9c01932900fd099d:

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

Backport of 6e2b82fdf6 from master

comment:7 Changed 9 months ago by gwahl@…

Is this going to get backported to 1.6 too?

comment:8 Changed 9 months ago 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

Note: See TracTickets for help on using tickets.
Back to Top