[Django] #33369: Add through_defaults as argument for m2m_changed signal
Django
-------------------------------------+-------------------------------------
Reporter: | Owner: nobody
ShmuelTreiger |
Type: New | Status: new
feature |
Component: Database | Version: 4.0
layer (models, ORM) |
Severity: Normal | Keywords: m2m_changed
Triage Stage: | Has patch: 0
Unreviewed |
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-------------------------------------+-------------------------------------
Use case:
In English:
I have many policies, each of which has a status from a table of statuses
and is not unique to that policy. I associate these policies to my many
domains through the many_to_many table. Domains can have many policies,
but only one of each type of policy. Likewise, the same policy can apply
to multiple domains. (In practice, there are a few thousand domains and a
dozen or so policies.) In the following code, I have successfully
implemented this relationship at the database level.
In pseudocode:
{{{
Domain(models.Model)
<fields>
PolicyStatus(models.Model):
status = models.CharField(<>)
Policy(models.Model)
status = models.ForeignKey(PolicyStatus, <>)
domains = models.ManyToManyField(Domain, through="Domain_Policy_m2m",
<>)
<fields>
Domain_Policy_m2m(models.Model):
class Meta:
constraints = [
models.UniqueConstraint(
fields=["domain", "status", ],
name="unique_constraint"
)
]
domain = models.ForeignKey(Domain, <>)
policy = models.ForeignKey(Policy, <>)
status = models.ForeignKey(PolicyStatus, <>)
}}}
Scenario:
I go to associate a new policy with a domain:
` domain.policy_set.add(policy, through_defaults={"status":
policy.status})`
However, the domain already has a policy with this status and raises
IntegrityError. Instead, I would like to replace the existing policy with
the new one. I can do this in my code:
{{{
new_policy = Policy(<>)
existing_policy =
domain.policy_set.filter(status=new_policy.status).first()
if existing_policy:
domain.policy_set.remove(existing_policy)
domain.policy_set.add(policy, through_defaults={"status":
policy.status})
}}}
Instead, I wanted to take care of this in a more permanent way, so I
created a m2m_change signal receive:
{{{
from <> import Policy
@receiver(m2m_changed, sender=Domain_Policy_m2m)
def delete_old_m2m(action, instance, pk_set, **kwargs):
if action == "pre_add":
pk = min(pk_set)
policy = Policy.objects.get(pk=pk)
status = policy.status
existing_policy = Domain_Policy_m2m.objects.filter(status=status,
domain=instance)
if existing_policy.exists():
existing_policy.delete()
}}}
This works, but it's not pretty. It would be excellent if I could have the
through_defaults as an argument, so I could do something like:
{{{
@receiver(m2m_changed, sender=Domain_Policy_m2m)
def delete_old_m2m(action, instance, through_defaults, **kwargs):
if action == "pre_add":
status = through_defaults["status"]
existing_policy = Domain_Policy_m2m.objects.filter(status=status,
domain=instance)
if existing_policy.exists():
existing_policy.delete()
}}}
Even the object(s) being added would be so much more convenient than the
pk:
{{{
@receiver(m2m_changed, sender=Domain_Policy_m2m)
def delete_old_m2m(action, instance, model_set, **kwargs):
if action == "pre_add":
model = min(model_set)
existing_policy =
Domain_Policy_m2m.objects.filter(status=model.status, domain=instance)
if existing_policy.exists():
existing_policy.delete()
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/33369>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
-------------------------------------+-------------------------------------
Type: New feature | Status: new
Component: Database layer | Version: dev
(models, ORM) |
Severity: Normal | Resolution:
Keywords: m2m_changed | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* version: 4.0 => dev
--
Ticket URL: <https://code.djangoproject.com/ticket/33369#comment:1>
Django
-------------------------------------+-------------------------------------
Type: New feature | Status: closed
Component: Database layer | Version: dev
(models, ORM) |
Keywords: m2m_changed | Triage Stage:
| Unreviewed
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* status: new => closed
* resolution: => invalid
Comment:
Thanks for the report , however the `m2m_changed` signal was created for
automatically created intermediate models. If you have an explicit
`through` model, you can attached `Model` signals to it, i.e. `pre_save`,
`post_save`, `pre_delete`, and `post_delete`. Personally, I would use add
this logic to the
[https://docs.djangoproject.com/en/4.0/ref/models/instances/#django.db.models.Model.save
Domain_Policy_m2m.save()] and don't use signals at all, this is not a
situation where signals are needed. If have further questions, see
TicketClosingReasons/UseSupportChannels for ways to get help.
--
Ticket URL: <https://code.djangoproject.com/ticket/33369#comment:2>
Django
-------------------------------------+-------------------------------------
Type: New feature | Status: new
Component: Database layer | Version: dev
(models, ORM) |
Severity: Normal | Resolution:
| Unreviewed
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* status: closed => new
* resolution: invalid =>
Comment:
`domain.policy_set.add(<>)` does not call save() on the m2m model. From
the documentation here
https://docs.djangoproject.com/en/4.0/ref/models/relations/#django.db.models.fields.related.RelatedManager.add
>Using add() with a many-to-many relationship, however, will not call any
save() methods (the bulk argument doesn’t exist), but rather create the
relationships using QuerySet.bulk_create(). If you need to execute some
custom logic when a relationship is created, listen to the m2m_changed
signal, which will trigger pre_add and post_add actions.
I tried `save`, it didn't work, so I found the documentation which told me
to use `m2m_changed`.
I suppose instead of `add`, I could do
`Domain_Policy_m2m.objects.create(<>)` but that's just seems less than
ideal.
--
Ticket URL: <https://code.djangoproject.com/ticket/33369#comment:3>
Django
-------------------------------------+-------------------------------------
Type: New feature | Status: closed
Component: Database layer | Version: dev
(models, ORM) |
Keywords: m2m_changed | Triage Stage:
| Unreviewed
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* status: new => closed
* resolution: => wontfix
Comment:
Please don't reopen closed ticket. You have a custom intermediate models,
so in your case `.create()` or `.save()` seem ideal to handle this logic.
As you already described there are at least two ways for your use case. We
don't want to add a third way. Moreover, signals are required in really
rare cases and it's definitely not one of them.
--
Ticket URL: <https://code.djangoproject.com/ticket/33369#comment:4>
Django
-------------------------------------+-------------------------------------
Type: New feature | Status: closed
Component: Database layer | Version: dev
(models, ORM) |
Severity: Normal | Resolution: wontfix
Keywords: m2m_changed | Triage Stage:
| Unreviewed
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by ShmuelTreiger):
I'm literally doing what your documentation tells me to do, and it's
shitty, hacky, and horrible. Your response is "Won't fix". Excellent.
--
Ticket URL: <https://code.djangoproject.com/ticket/33369#comment:5>
Django
-------------------------------------+-------------------------------------
Type: New feature | Status: closed
Component: Database layer | Version: dev
(models, ORM) |
Severity: Normal | Resolution: wontfix
Keywords: m2m_changed | Triage Stage:
| Unreviewed
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Mariusz Felisiak):
You can start a discussion on DevelopersMailingList if you don't agree,
where you'll reach a wider audience and see what other think, and
[https://docs.djangoproject.com/en/stable/internals/contributing/bugs-and-
features/#requesting-features follow the guidelines with regards to
requesting features].
Also, please don't use offensive language, and be aware that
[https://www.djangoproject.com/conduct/ "Code of Conduct"] applies to the
Trac.
--
Ticket URL: <https://code.djangoproject.com/ticket/33369#comment:6>