#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 django-updates+unsubscr...@googlegroups.com.
To post to this group, send email to django-updates@googlegroups.com.
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.
