Opened 7 years ago

Closed 5 years ago

Last modified 5 years ago

#27462 closed Cleanup/optimization (fixed)

Clarify what's contained in m2m_changed's "pk_set" argument

Reported by: Seti Owned by: Carlton Gibson
Component: Documentation Version: dev
Severity: Normal Keywords:
Cc: Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

A code of signal;

@receiver(m2m_changed, sender=Group.user_set.through)
def changed_m2m(sender, instance, action, reverse, model, pk_set, **kwargs):

    print('Signal "{}" | Users in group: {} | pk_set: {}'.format(action, instance.user_set.all(), pk_set))

Run code:

        g1 = Group.objects.all()[0]

        g1.user_set.add(UserModel.objects.all()[0])
        g1.user_set.add(UserModel.objects.all()[0])
        g1.user_set.add(UserModel.objects.all()[0])

        g1.user_set.remove(UserModel.objects.all()[0])
        g1.user_set.remove(UserModel.objects.all()[0])
        g1.user_set.remove(UserModel.objects.all()[0])

Results:

Signal "pre_add" | Users in group: <QuerySet []> | pk_set: {UUID('d0d4a1eb-d2c0-4a27-b9a6-a057a4fa25bc')}
Signal "post_add" | Users in group: <QuerySet [<User: Martin Rivera>]> | pk_set: {UUID('d0d4a1eb-d2c0-4a27-b9a6-a057a4fa25bc')}
Signal "pre_add" | Users in group: <QuerySet [<User: Martin Rivera>]> | pk_set: set()
Signal "post_add" | Users in group: <QuerySet [<User: Martin Rivera>]> | pk_set: set()
Signal "pre_add" | Users in group: <QuerySet [<User: Martin Rivera>]> | pk_set: set()
Signal "post_add" | Users in group: <QuerySet [<User: Martin Rivera>]> | pk_set: set()
Signal "pre_remove" | Users in group: <QuerySet [<User: Martin Rivera>]> | pk_set: {UUID('d0d4a1eb-d2c0-4a27-b9a6-a057a4fa25bc')}
Signal "post_remove" | Users in group: <QuerySet []> | pk_set: {UUID('d0d4a1eb-d2c0-4a27-b9a6-a057a4fa25bc')}
Signal "pre_remove" | Users in group: <QuerySet []> | pk_set: {UUID('d0d4a1eb-d2c0-4a27-b9a6-a057a4fa25bc')}
Signal "post_remove" | Users in group: <QuerySet []> | pk_set: {UUID('d0d4a1eb-d2c0-4a27-b9a6-a057a4fa25bc')}
Signal "pre_remove" | Users in group: <QuerySet []> | pk_set: {UUID('d0d4a1eb-d2c0-4a27-b9a6-a057a4fa25bc')}
Signal "post_remove" | Users in group: <QuerySet []> | pk_set: {UUID('d0d4a1eb-d2c0-4a27-b9a6-a057a4fa25bc')}

If action == 'pre_remove', then pk_set has primary keys of objects which is not presets in a queryset of other model. It is an expected behaviour.

But if action == 'post_remove' then pk_set always contains primary keys of objects, even queryset is empty. Does is expected behaviour?

The Django`s docs says next:
https://docs.djangoproject.com/en/1.10/ref/signals/#m2m-changed

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.

So, why the pk_set for the action == 'post_remove' always not empty?

I tested it on the Django 1.10 and the 1.9, the result is same.

Change History (7)

comment:1 by Tim Graham, 7 years ago

Summary: Attribute "pk_set" is always is not empty, even m2m is not changed at all.Clarify what's contained in m2m_changed's "pk_set" argument
Triage Stage: UnreviewedAccepted
Type: UncategorizedCleanup/optimization

I think a documentation clarification indicating that pk_set may be the IDs that are passed in, not necessarily those that are removed, may have to do.

At least, it looks like it would take an extra query to fetch the IDs that are actually removed (and I think accuracy couldn't be guaranteed for pre_remove due to race conditions).

comment:2 by Carlton Gibson, 5 years ago

Has patch: set
Owner: changed from nobody to Carlton Gibson
Status: newassigned
Version: 1.10master

comment:3 by Mariusz Felisiak, 5 years ago

Triage Stage: AcceptedReady for checkin

comment:4 by Carlton Gibson <carlton@…>, 5 years ago

In bed4a152:

Refs #27462 -- Added tests of pk_set in m2m_changed signal receivers for repeated add/remove calls.

comment:5 by Carlton Gibson <carlton@…>, 5 years ago

Resolution: fixed
Status: assignedclosed

In bae05bc:

Fixed #27462 -- Clarifed pk_set difference in m2m_changed signal receivers for add() and remove().

Thank you to Mariusz Felisiak for review.

comment:6 by Carlton Gibson <carlton.gibson@…>, 5 years ago

In e4dc1b8c:

[3.0.x] Fixed #27462 -- Clarifed pk_set difference in m2m_changed signal receivers for add() and remove().

Thank you to Mariusz Felisiak for review.

Backport of bae05bcf68710cb6dafa51325c3ec83ddda83c39 from master

comment:7 by Carlton Gibson <carlton.gibson@…>, 5 years ago

In 268e7c77:

[2.2.x] Fixed #27462 -- Clarifed pk_set difference in m2m_changed signal receivers for add() and remove().

Thank you to Mariusz Felisiak for review.

Backport of bae05bcf68710cb6dafa51325c3ec83ddda83c39 from master

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