Opened 7 years ago

Closed 4 years ago

Last modified 4 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 Changed 7 years ago by Tim Graham

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 Changed 4 years ago by Carlton Gibson

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

comment:3 Changed 4 years ago by Mariusz Felisiak

Triage Stage: AcceptedReady for checkin

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

In bed4a152:

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

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

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 Changed 4 years ago by Carlton Gibson <carlton.gibson@…>

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 Changed 4 years ago by Carlton Gibson <carlton.gibson@…>

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