Code

Opened 4 years ago

Closed 4 years ago

Last modified 3 years ago

#13087 closed (fixed)

m2m_changed fires before a clear but after add/remove

Reported by: rcatherman Owned by: nobody
Component: Database layer (models, ORM) Version: 1.2-beta
Severity: Keywords:
Cc: Triage Stage: Design decision needed
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description

In db/models/fields/related.py I notice that _add_items and _remove_items perform the db actions and then fire the signal to the m2m_changed listeners. _clear_items fires the signal prior to the db interaction. The result is inconsistent. For my use case, I'd like to have my listener assert a relationship has at least one particular item no matter if it is accidentily cleared. It works on removal, but not on clear because I notice the DELETE happens after my re-INSERT.

I've made the change here and it works as expected now.

Attachments (0)

Change History (7)

comment:1 Changed 4 years ago by russellm

  • milestone set to 1.2
  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Triage Stage changed from Unreviewed to Design decision needed

This certainly needs to be decided before 1.2. The current behaviour is deliberate, and documented as implemented; I need to think about this (preferably with a clear head) to determine if the current behavior is the most desirable.

comment:2 Changed 4 years ago by russellm

  • Component changed from Uncategorized to Database layer (models, ORM)

comment:3 Changed 4 years ago by malcalypse

  • Owner changed from nobody to malcalypse
  • Status changed from new to assigned

comment:4 Changed 4 years ago by malcalypse

  • Owner changed from malcalypse to nobody
  • Status changed from assigned to new

comment:5 Changed 4 years ago by gav

Why not do both? We have pre/post-save, and pre/post-delete. So, pre/post-m2m_changed would be very much in line with what Django already has.

comment:6 Changed 4 years ago by russellm

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

(In [12888]) Fixed #13087 -- Modified m2m signals to provide greater flexibility over exactly when notifications are delivered.

This is a BACKWARDS INCOMPATIBLE CHANGE for anyone using the signal names introduced in r12223.

  • If you were listening to "add", you should now listen to "post_add".
  • If you were listening to "remove", you should now listen to "post_remove".
  • If you were listening to "clear", you should now listen to "pre_clear".

You may also want to examine your code to see whether the "pre_add", "pre_remove" or "post_clear" would be better suited to your application.

comment:7 Changed 3 years ago by jacob

  • milestone 1.2 deleted

Milestone 1.2 deleted

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


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

 
Note: See TracTickets for help on using tickets.