[Django] #22296: Documentation clarification: m2m_changed pk_set inconsistent between post_add and post_remove

[Django]

#22296: Documentation clarification: m2m_changed pk_set inconsistent between
post_add and post_remove
-------------------------------+--------------------
Reporter: anentropic | Owner: nobody
Type: Uncategorized | Status: new
Component: Uncategorized | Version: 1.5
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------+--------------------
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
https://github.com/django/django/blob/master/django/db/models/fields/related.py#L1032

Whereas it looks like the add method has code to deliberately exclude
already added items
https://github.com/django/django/blob/master/django/db/models/fields/related.py#L1008

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

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

[Django]

#22296: m2m_changed pk_set value inconsistent between post_add and post_remove
-------------------------------+--------------------------------------

Reporter: anentropic | Owner: nobody
Type: Uncategorized | Status: new
Component: Uncategorized | Version: 1.5

Severity: Normal | Resolution:
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0

-------------------------------+--------------------------------------
Changes (by anentropic):

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

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

[Django]

#22296: m2m_changed pk_set value inconsistent between post_add and post_remove
-------------------------------------+-------------------------------------
Reporter: anentropic | Owner: nobody
Type: Bug | Status: new
Component: Database layer | Version: 1.5
(models, ORM) | Resolution:
Severity: Normal | Triage Stage: Accepted
Keywords: | Needs documentation: 0
Has patch: 0 | Patch needs improvement: 0
Needs tests: 0 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Changes (by aaugustin):

* component: Uncategorized => Database layer (models, ORM)
* type: Uncategorized => Bug
* stage: Unreviewed => Accepted

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

[Django]

#22296: m2m_changed pk_set value inconsistent between post_add and post_remove
-------------------------------------+-------------------------------------
Reporter: anentropic | Owner: nobody
Type: Bug | Status: closed

Component: Database layer | Version: 1.5
(models, ORM) |

Severity: Normal | Resolution: duplicate
Keywords: | Triage Stage: Accepted

Has patch: 0 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0

-------------------------------------+-------------------------------------
Changes (by Carlton Gibson):

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

Comment:

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 [https://code.djangoproject.com/ticket/29615#comment:2 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.

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