#30022: Doc how to combine post_save signal with on_commit to alter a m2m
relation
when saving a model instance
-------------------------------------+-------------------------------------
Reporter: George Tantiras | Owner: nobody
Type: | Status: closed
Cleanup/optimization |
Component: Documentation | Version: 2.1
Severity: Normal | Resolution: wontfix
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Simon Charette):
Hello George,
This is getting in into django-user territory but the `test_user` test
happens to fail if you remove the `on_transaction_commit` decorator on the
`post_save` receiver because of
[https://docs.djangoproject.com/en/2.1/topics/forms/modelforms/#the-save-
method how model forms m2m saving works].
`ModelForm.save()` start by saving their attached instance, which triggers
`post_save` signals, and then save their m2m values. In your case since
`superuser_data` doesn't include any value for `groups` then the following
chain of actions happen on `change_form.save()` if you remove the
`on_transaction_commit` decorator.
1. `user` get assigned `superuser_data` and its `save()` method is
invoked.
2. The `post_save` signal is fired and the group gets added.
3. The form saves the `groups` m2m to an empty set because `groups` is
missing from `superuser_data`.
You can confirm this is the case by changing your `test_user` test to the
following.
{{{#!python
def test_user(group, user_data, superuser_data):
'''
This test will fail if on_commit is not used in the signal.
'''
creation_form = UserCreationForm(data=user_data)
user = creation_form.save()
change_form = UserChangeForm(instance=user, data=superuser_data)
assert change_form.is_valid()
with transaction.atomic():
superuser = change_form.save(commit=False)
superuser.save()
assert group in superuser.groups.all()
superuser.save_m2m()
assert group not in superuser.groups.all()
}}}
When your `post_save` receiver is decorated with `on_transaction_commit`
the group addition happens to be performed ''after'' `save_m2m` is called
which happens to make the test pass.
What you should do to assert `user.is_superuser ==
user.groups.filter(name='superusers').exists()` consistency is hook into
the `m2m_changed` signal as well instead of relying on unrelated
`on_commit` behaviour. For example,
{{{#!python
@receiver(
m2m_changed,
sender=User.groups.through
)
def ensure_groups_consistency(signal, sender, instance, action, pk_set,
**kwargs):
if instance.is_superuser and action in ('pre_remove', 'post_clear'):
group = Group.objects.get(name='Superusers')
if action == 'pre_remove' and group.pk in pk_set:
pk_set.remove(group.pk)
elif action == 'post_clear':
instance.groups.add(group)
}}}
As I said earlier this is probably better discussed on support channels
that here as this is expected behaviour.
--
Ticket URL: <https://code.djangoproject.com/ticket/30022#comment:4>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
--
You received this message because you are subscribed to the Google Groups
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
To post to this group, send email to [email protected].
To view this discussion on the web visit
https://groups.google.com/d/msgid/django-updates/066.ee19edd35212e10c5491c2c33fe4d5b9%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.
