Opened 14 months ago

Closed 14 months ago

Last modified 14 months ago

#34733 closed New feature (wontfix)

m2m_changed signal is unaware if .set() method is being called

Reported by: Leif Kjos Owned by: Leif Kjos
Component: Database layer (models, ORM) Version: dev
Severity: Normal Keywords:
Cc: Triage Stage: Unreviewed
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description (last modified by Leif Kjos)

When a related_m2m_field.set() method is called, the m2m_changed signal is fired four times for action: pre_remove, post_remove, pre_add, and post_add (or pre_clear / post_clear).

This poses a problem if the signal is supposed to run validation on the final state of the model. For example, let's say I have a model called Customer with a many-to-many relation to SubscriptionPlan and I have a m2m_changed signal that is supposed to validate if their SubscriptionPlan is valid.

If I call Customer.subscription_plans.set([some_plans]), the many-to-many manager will first call subscription_plans.remove() then .add() inside an atomic transaction. If my signal validates on the first .remove(), it could be in an invalid state, even though it won't be by the time the .add() completes. However, I still need to be able to validate on the signal if the standard .remove() method is called.

To get around this, I had to create a custom ManyToManyField with a custom RelatedManager that set an instance variable on the model when the .set() method was called. I'd like to propose adding a feature to the RelatedManager or to the signals to make the m2m_changed signal aware of if the .set() method was called when running. This could be a private attribute on the instance or extra information passed to the signal receiver.

If this feature already exists or there's a decent workaround, let me know and i'll close the ticket! Otherwise, I have a patch in mind that I can raise a PR for and attach.

Change History (3)

comment:1 by Leif Kjos, 14 months ago

Description: modified (diff)
Owner: changed from nobody to Leif Kjos
Status: newassigned

in reply to:  description ; comment:2 by Mariusz Felisiak, 14 months ago

Resolution: wontfix
Status: assignedclosed

Thanks for this ticket, however, signals are the last resort, required in really rare cases and could probably be avoided in yours. We don't want to add new features to them. Personally, I'd use a custom intermediate models, and add this logic to the .create() or .save() methods.

Please see TicketClosingReasons/UseSupportChannels for ways to get help with Django usage, where folks can help you in re-designing this validation.

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 follow triaging guidelines with regards to wontfix tickets.

in reply to:  2 comment:3 by Leif Kjos, 14 months ago

Replying to Mariusz Felisiak:

Thanks for this ticket, however, signals are the last resort, required in really rare cases and could probably be avoided in yours. We don't want to add new features to them. Personally, I'd use a custom intermediate models, and add this logic to the .create() or .save() methods.

Please see TicketClosingReasons/UseSupportChannels for ways to get help with Django usage, where folks can help you in re-designing this validation.

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 follow triaging guidelines with regards to wontfix tickets.

Thanks for the response! Will try that.

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