Opened 10 years ago

Closed 6 years ago

#22296 closed Bug (duplicate)

m2m_changed pk_set value inconsistent between post_add and post_remove

Reported by: Paul Garner Owned by: nobody
Component: Database layer (models, ORM) Version: 1.5
Severity: Normal Keywords:
Cc: Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no


the docs say:

"pk_set: For the pre_add, post_add, pre_remove and post_remove actions, this is a set of primary key values that have been added to or removed from the relation."

What I found is that for action=='post_add':

  • first time add(x) -> set([x])
  • next time (same x) add(x) -> set([])

but for action=='post_remove':

  • first time remove(x) -> set([x])
  • next time (same x) remove(x) -> set([x])

I feel like they should be consistent. Either way is fine but the behaviour of post_add seems more useful to me (can avoid unnecessary action in your signal receiver)... but it should be documented that you get empty set if nothing needed adding.

I've only tried on 1.5 so far but looking at source I don't see any new code to change this behaviour

Whereas it looks like the add method has code to deliberately exclude already added items

I'm happy to make a patch if it's felt this is useful?

Change History (3)

comment:1 by Paul Garner, 10 years ago

Summary: Documentation clarification: m2m_changed pk_set inconsistent between post_add and post_removem2m_changed pk_set value inconsistent between post_add and post_remove

comment:2 by Aymeric Augustin, 10 years ago

Component: UncategorizedDatabase layer (models, ORM)
Triage Stage: UnreviewedAccepted
Type: UncategorizedBug

comment:3 by Carlton Gibson, 6 years ago

Resolution: duplicate
Status: newclosed

As per #29615, the behaviour here is now different.

  • For add(), multiple calls with the same ids now only result in a single signal being dispatched.
  • For delete(), the signal is dispatched each time.

See comment on #29615 for more detail but, the underlying difference is that add() calls filter duplicates to avoid IntegrityError when inserting the records. There's no need for this with delete.

Happy for others to comment differently but, I thought on #29615 that the extra DB query would not be worth it to eliminate the extra signal, but that the anomaly could be documented.

Given all that I'll close this as a duplicate (despite it being the older ticket, and my having previous thought them not duplicates... sigh...). Further discussion on #29615 please.

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