Code

Opened 6 years ago

Closed 5 years ago

#6778 closed (duplicate)

dispatch signal needed on many to many relationship alterations (add, delete)

Reported by: xore.ander@… Owned by: mysliceof314
Component: Core (Other) Version: master
Severity: Keywords: dispatcher relation m2m
Cc: gsong, christian, rvdrijst Triage Stage: Accepted
Has patch: yes Needs documentation: yes
Needs tests: no Patch needs improvement: yes
Easy pickings: UI/UX:

Description

When a model has a relation added, no signal is sent.

This breaks functionality where the model is internally caching processed relational data, and the model becomes associated with new (or disassociated with) items. There is no method to inform the original model that it's cache is stale.

it would seem appropriate that all functions with the property 'alters_data' (originally intended for template use) would have pre/post signals.
(django/db/models/fields/related.py, django/contrib/contenttypes/generic.py: add, create, remove, clear )

to give a trivial example:

class Attendee(models.Model):
	name = models.CharField(maxlength=255)

class Conference(models.Model):
	name = models.CharField(maxlength=255)
	attendees = models.ManyToManyField(Attendee, related_name='conferences')
	def num_attendees(self):
		if not hasattr(self,"attendee_count"):
			setattr(self,"attendee_count",len(self.attendees.all()))
		return self.attendee_count
	def stale_cache(self):
		if hasattr(self,"attendee_count"):
			delattr(self,"attendee_count")

(generally, i'm looking to do more complex processes than take the len() of a queryset...)

a,b,c = Attendee(name="Alice"), Attendee(name="Bob"), Conference(name="PyCon")
for i in (a,b,c):
	i.save()

c.attendees.add(a)
c.num_attendees()
c.attendees.add(b)
c.num_attendees()

obviously calls to stale_cache() would fix the problem, but the application-level programmer shouldn't necessarily have to deal with cache maintenance. (DRY... someone's bound to forget to update cache somewhere), it would be better of there is a signal to do this on relational updates.

Enabling this should be fairly straightforward:

  • add appropriate signals to django.db.models.signals
  • call the dispatcher from appropriate functions in the relational model managers, etc

Attachments (2)

models.py (1.8 KB) - added by mysliceof314 6 years ago.
model tests for the m2m signals
patch.diff (7.4 KB) - added by mysliceof314 6 years ago.

Download all attachments as: .zip

Change History (11)

comment:1 Changed 6 years ago by telenieko

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Triage Stage changed from Unreviewed to Accepted

comment:2 Changed 6 years ago by remo

This is a big issue. Right now, it is impossible to perform tasks when relations are added or removed. Consider this use-case:

class Survey(models.Model):
    participants = models.ManyToManyField(User)

How would I send a message to an User when he is added to the Survey? Hooking into post_save of Survey does not work, as its unrelated to the 'participants' manager. #6095 would offer a way to achieve this (by hooking management code into an intermediary model). But using intermediary models will break the admin interface (eg. 'filter_horizontal'), and that's a huge consequence to adding some management code.

So signals seem to be the best solution to this problem. I suggest we name those signals 'pre/post_add_relation' and 'pre/post_remove_relation'. As I see it, there are two issues.

  1. When recieving such a signal, we'll need to know
    • the model instance with the m2m field (Survey)
    • the m2m field itself (Survey.participants, as there might be several m2m fields to User)
    • the related object (User instance)

We just have the sender and instance arguments to pass information.. maybe we can use a tuple to pass both related objects as instance and use the m2m field as sender?

  1. The ManyRelatedManager does alot of low-level stuff (ie. it uses custom queries to add objects, to remove objects and to clear itself). Sending appropriate signals would void many of these optimizations.

Thinking of it, we could refractor the m2m manager to use a generic intermediary model. This model would then send the stock pre/post_save signals. This would allow a much cleaner implementation of #6095, too.

comment:3 Changed 6 years ago by gsong

  • Cc gsong added

comment:4 Changed 6 years ago by mysliceof314

  • Owner changed from nobody to mysliceof314
  • Status changed from new to assigned

Changed 6 years ago by mysliceof314

model tests for the m2m signals

comment:5 Changed 6 years ago by mysliceof314

  • Has patch set

comment:6 follow-up: Changed 6 years ago by SmileyChris

  • Needs documentation set
  • Patch needs improvement set

Docs would be good (/docs/ref/signals.txt)

Also, you can't use the new decorator format - Django has to be Python 2.3 compatible

Changed 6 years ago by mysliceof314

comment:7 in reply to: ↑ 6 Changed 6 years ago by mysliceof314

Replying to SmileyChris:

Docs would be good (/docs/ref/signals.txt)

Also, you can't use the new decorator format - Django has to be Python 2.3 compatible

removed new decorator format, added docs and put the whole patch into one file

comment:8 Changed 5 years ago by christian

  • Cc christian added

comment:9 Changed 5 years ago by rvdrijst

  • Cc rvdrijst added
  • Resolution set to duplicate
  • Status changed from assigned to closed

Looks like a duplicate of #5390

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.