Code

Opened 2 years ago

Last modified 11 months ago

#17688 new Bug

No m2m_changed signal sent to when referenced object is deleted

Reported by: jblaine@… Owned by: nobody
Component: Database layer (models, ORM) Version: 1.3
Severity: Normal Keywords:
Cc: anssi.kaariainen@…, gamesbook Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

class Topping(models.Model):
    name = models.CharField()

class Pizza(models.Model):
    name = models.CharField()
    toppings = models.ManyToManyField(Topping, null=true, blank=true)

And this data established using those models:

    TOPPING
    id=1, name="Pepperoni"
    id=2, name="Onion"
    id=3, name="Mushroom"

    PIZZA
    id=1, name="foopizza"
    toppings=1,2,3
  1. Deleting any Topping object (for example, deleting id=1, name="Pepperoni") also removes it from foopizza.toppings. GOOD
  1. No m2m_changed signal is sent when 1 (above happens). BAD?

Attachments (1)

17688.diff (620 bytes) - added by akaariai 2 years ago.

Download all attachments as: .zip

Change History (6)

comment:1 Changed 2 years ago by akaariai

  • Cc anssi.kaariainen@… added
  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Triage Stage changed from Unreviewed to Accepted

I confirmed that no m2m_changed signal is sent. It seems it would be somewhat easy to send m2m_changed signals from models/deletion.py. I have no clue what the correct arguments for all the m2m_signal args should be.

The attached test shows that no signal is sent.

Changed 2 years ago by akaariai

comment:2 Changed 2 years ago by akaariai

jblaine: could you (or somebody else) provide the details what arguments do you expect the signal to send in this case? (I am talking about the pk_list, model, action, ...) arguments. I guess the right answer is the same as what you would send in tobbing.pizza_set.clear() case. However maybe it would be better to send all the deleted toppings in one batch (which seems to be possible from the delete code), so that you could get "all the pizzas for which toppings were removed" more effectively.

I am not 100% sure if the delete code really allows easily sending the m2m_changed signal. But it seems like it.

comment:3 Changed 2 years ago by anonymous

Being pretty new, I don't know that I am qualified to say. I just expected it to work the same as what the following scenario would cause to happen:

from django.db.models.signals import m2m_changed

def some_callback(sender, **kwargs):
    # custom code

m2m_changed.connect(some_callback, sender=Pizza.toppings.through)
foopizza.toppings.remove(ToppingObjectHere)

Because, in effect, that's what is happening for each Pizza with a relationship to the Topping that was deleted.

I think anything other than that will cause a new special case to document ("Note: when deleting an object referenced by a ManyToManyField, m2m_changed works as follows...").

No?

comment:4 Changed 2 years ago by gamesbook

  • Cc gamesbook added

comment:5 Changed 11 months ago by jblaine

As we only use Django for 1 internal site, are not Django-internals savvy, and I don't have the time to become Django-internals savvy, I have instead made a personal $25 donation to Django Project made in support of getting this straightened out properly in Django. That's all I can offer aside from the suggestion that this ticket among several others shows that, IMO, the overall signals stuff perhaps has not been looked at with a "Forget what's there for a second. What's the RIGHT thing to be doing in designing the signals stuff and addressing the various tickets that point out original design shortsightedness." mindset.

Thank you for your consideration.

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as new
The owner will be changed from nobody to anonymous. Next status will be 'assigned'
as The resolution will be set. Next status will be 'closed'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.