Code

Ticket #5390: 5390-against-12033.diff

File 5390-against-12033.diff, 18.0 KB (added by frans, 4 years ago)

updated diff with correct clear() management and more recent version of trunk

Line 
1Index: django/db/models/fields/related.py
2===================================================================
3--- django/db/models/fields/related.py  (.../original)  (revision 784)
4+++ django/db/models/fields/related.py  (.../branches/only-5390)        (revision 784)
5@@ -424,7 +424,8 @@
6     through = rel.through
7     class ManyRelatedManager(superclass):
8         def __init__(self, model=None, core_filters=None, instance=None, symmetrical=None,
9-                join_table=None, source_field_name=None, target_field_name=None):
10+                join_table=None, source_field_name=None, target_field_name=None,
11+                field_name=None, reverse=False):
12             super(ManyRelatedManager, self).__init__()
13             self.core_filters = core_filters
14             self.model = model
15@@ -434,6 +435,8 @@
16             self.target_field_name = target_field_name
17             self.through = through
18             self._pk_val = self.instance.pk
19+            self.field_name = field_name
20+            self.reverse = reverse
21             if self._pk_val is None:
22                 raise ValueError("%r instance needs to have a primary key value before a many-to-many relationship can be used." % instance.__class__.__name__)
23 
24@@ -513,14 +516,24 @@
25                     source_field_name: self._pk_val,
26                     '%s__in' % target_field_name: new_ids,
27                 })
28-                vals = set(vals)
29-
30+               new_ids = new_ids - set(vals)
31                 # Add the ones that aren't there already
32-                for obj_id in (new_ids - vals):
33+                for obj_id in new_ids :
34                     self.through._default_manager.using(self.instance._state.db).create(**{
35                         '%s_id' % source_field_name: self._pk_val,
36                         '%s_id' % target_field_name: obj_id,
37                     })
38+                added_objs = [obj for obj in objs if \
39+                      (isinstance(obj, self.model) and obj._get_pk_val() in new_ids) \
40+                      or obj in new_ids]
41+                if self.reverse:
42+                    sender = self.model
43+                else:
44+                    sender = self.instance.__class__
45+                    signals.m2m_changed.send(sender=sender, action='add',
46+                        instance=self.instance, model=self.model,
47+                        reverse=self.reverse, field_name=self.field_name,
48+                        objects=added_objs)
49 
50         def _remove_items(self, source_field_name, target_field_name, *objs):
51             # source_col_name: the PK colname in join_table for the source object
52@@ -541,9 +554,25 @@
53                     source_field_name: self._pk_val,
54                     '%s__in' % target_field_name: old_ids
55                 }).delete()
56+                if self.reverse:
57+                    sender = self.model
58+                else:
59+                    sender = self.instance.__class__
60+                signals.m2m_changed.send(sender=sender, action="remove",
61+                    instance=self.instance, model=self.model,
62+                    reverse=self.reverse, field_name=self.field_name, 
63+                    objects=list(objs))
64 
65+
66         def _clear_items(self, source_field_name):
67             # source_col_name: the PK colname in join_table for the source object
68+            if self.reverse:
69+                sender = self.model
70+            else:
71+                sender = self.instance.__class__
72+            signals.m2m_changed.send(sender=sender, action="clear",
73+                instance=self.instance, model=self.model, reverse=self.reverse,
74+                field_name=self.field_name, objects=None)
75             self.through._default_manager.using(self.instance._state.db).filter(**{
76                 source_field_name: self._pk_val
77             }).delete()
78@@ -576,7 +605,9 @@
79             instance=instance,
80             symmetrical=False,
81             source_field_name=self.related.field.m2m_reverse_field_name(),
82-            target_field_name=self.related.field.m2m_field_name()
83+            target_field_name=self.related.field.m2m_field_name(),
84+            field_name=self.related.field.name,
85+            reverse=True
86         )
87 
88         return manager
89@@ -590,9 +621,19 @@
90             raise AttributeError, "Cannot set values on a ManyToManyField which specifies an intermediary model. Use %s.%s's Manager instead." % (opts.app_label, opts.object_name)
91 
92         manager = self.__get__(instance)
93-        manager.clear()
94-        manager.add(*value)
95+        previous=set(manager.all())
96+        new=set(value)
97+        if not new:
98+            manager.clear()
99+        else:
100+            added=new-previous
101+            removed=previous-new
102+            if added :
103+                manager.add(*added)
104+            if removed :
105+                manager.remove(*removed)
106 
107+
108 class ReverseManyRelatedObjectsDescriptor(object):
109     # This class provides the functionality that makes the related-object
110     # managers available as attributes on a model class, for fields that have
111@@ -626,7 +667,9 @@
112             instance=instance,
113             symmetrical=(self.field.rel.symmetrical and isinstance(instance, rel_model)),
114             source_field_name=self.field.m2m_field_name(),
115-            target_field_name=self.field.m2m_reverse_field_name()
116+            target_field_name=self.field.m2m_reverse_field_name(),
117+            field_name=self.field.name,
118+            reverse=False
119         )
120 
121         return manager
122@@ -640,8 +683,17 @@
123             raise AttributeError, "Cannot set values on a ManyToManyField which specifies an intermediary model.  Use %s.%s's Manager instead." % (opts.app_label, opts.object_name)
124 
125         manager = self.__get__(instance)
126-        manager.clear()
127-        manager.add(*value)
128+        previous=set(manager.all())
129+        new=set(value)
130+        if not new:
131+            manager.clear()
132+        else:
133+            added=new-previous
134+            removed=previous-new
135+            if added :
136+                manager.add(*added)
137+            if removed :
138+                manager.remove(*removed)
139 
140 class ManyToOneRel(object):
141     def __init__(self, to, field_name, related_name=None,
142Index: django/db/models/signals.py
143===================================================================
144--- django/db/models/signals.py (.../original)  (revision 784)
145+++ django/db/models/signals.py (.../branches/only-5390)        (revision 784)
146@@ -12,3 +12,5 @@
147 post_delete = Signal(providing_args=["instance"])
148 
149 post_syncdb = Signal(providing_args=["class", "app", "created_models", "verbosity", "interactive"])
150+
151+m2m_changed = Signal(providing_args=["instance", "action", "model", "field_name", "reverse", "objects"])
152Index: tests/modeltests/m2m_signals/__init__.py
153===================================================================
154--- tests/modeltests/m2m_signals/__init__.py    (.../original)  (revision 0)
155+++ tests/modeltests/m2m_signals/__init__.py    (.../branches/only-5390)        (revision 784)
156@@ -0,0 +1 @@
157+
158Index: tests/modeltests/m2m_signals/models.py
159===================================================================
160--- tests/modeltests/m2m_signals/models.py      (.../original)  (revision 0)
161+++ tests/modeltests/m2m_signals/models.py      (.../branches/only-5390)        (revision 784)
162@@ -0,0 +1,166 @@
163+"""
164+Testing signals emitted on changing m2m relations.
165+"""
166+
167+from django.db import models
168+
169+class Part(models.Model):
170+    name = models.CharField(max_length=20)
171+       
172+    def __unicode__(self):
173+        return self.name
174+
175+class Car(models.Model):
176+    name = models.CharField(max_length=20)
177+    default_parts = models.ManyToManyField(Part)
178+    optional_parts = models.ManyToManyField(Part, related_name='cars_optional')
179+
180+    def __unicode__(self):
181+        return self.name
182+
183+def m2m_changed_test(signal, sender, **kwargs):
184+    print 'm2m_changed signal'
185+    print 'instance:', kwargs['instance']
186+    print 'action:', kwargs['action']
187+    print 'reverse:', kwargs['reverse']
188+    print 'field_name:', kwargs['field_name']
189+    print 'model:', kwargs['model']
190+    print 'objects:',kwargs['objects']
191+   
192+
193+__test__ = {'API_TESTS':"""
194+>>> models.signals.m2m_changed.connect(m2m_changed_test, Car)
195+
196+# Test the add, remove and clear methods on both sides of the
197+# many-to-many relation
198+
199+>>> c1 = Car.objects.create(name='VW')
200+>>> c2 = Car.objects.create(name='BMW')
201+>>> c3 = Car.objects.create(name='Toyota')
202+>>> p1 = Part.objects.create(name='Wheelset')
203+>>> p2 = Part.objects.create(name='Doors')
204+>>> p3 = Part.objects.create(name='Engine')
205+>>> p4 = Part.objects.create(name='Airbag')
206+>>> p5 = Part.objects.create(name='Sunroof')
207+
208+# adding some default parts to our car
209+>>> c1.default_parts.add(p1, p2, p3)
210+m2m_changed signal
211+instance: VW
212+action: add
213+reverse: False
214+field_name: default_parts
215+model: <class 'modeltests.m2m_signals.models.Part'>
216+objects: [<Part: Wheelset>, <Part: Doors>, <Part: Engine>]
217+
218+# give the BMW and Toyata some doors as well
219+>>> p2.car_set.add(c2, c3)
220+m2m_changed signal
221+instance: Doors
222+action: add
223+reverse: True
224+field_name: default_parts
225+model: <class 'modeltests.m2m_signals.models.Car'>
226+objects: [<Car: BMW>, <Car: Toyota>]
227+
228+# remove the engine from the VW and the airbag (which is not set but is returned)
229+>>> c1.default_parts.remove(p3, p4)
230+m2m_changed signal
231+instance: VW
232+action: remove
233+reverse: False
234+field_name: default_parts
235+model: <class 'modeltests.m2m_signals.models.Part'>
236+objects: [<Part: Engine>, <Part: Airbag>]
237+
238+# give the VW some optional parts (second relation to same model)
239+>>> c1.optional_parts.add(p4,p5)
240+m2m_changed signal
241+instance: VW
242+action: add
243+reverse: False
244+field_name: optional_parts
245+model: <class 'modeltests.m2m_signals.models.Part'>
246+objects: [<Part: Airbag>, <Part: Sunroof>]
247+
248+# add airbag to all the cars (even though the VW already has one)
249+>>> p4.cars_optional.add(c1, c2, c3)
250+m2m_changed signal
251+instance: Airbag
252+action: add
253+reverse: True
254+field_name: optional_parts
255+model: <class 'modeltests.m2m_signals.models.Car'>
256+objects: [<Car: BMW>, <Car: Toyota>]
257+
258+# remove airbag from the VW (reverse relation with custom related_name)
259+>>> p4.cars_optional.remove(c1)
260+m2m_changed signal
261+instance: Airbag
262+action: remove
263+reverse: True
264+field_name: optional_parts
265+model: <class 'modeltests.m2m_signals.models.Car'>
266+objects: [<Car: VW>]
267+
268+# clear all parts of the VW
269+>>> c1.default_parts.clear()
270+m2m_changed signal
271+instance: VW
272+action: clear
273+reverse: False
274+field_name: default_parts
275+model: <class 'modeltests.m2m_signals.models.Part'>
276+objects: None
277+
278+# take all the doors off of cars
279+>>> p2.car_set.clear()
280+m2m_changed signal
281+instance: Doors
282+action: clear
283+reverse: True
284+field_name: default_parts
285+model: <class 'modeltests.m2m_signals.models.Car'>
286+objects: None
287+
288+# take all the airbags off of cars (clear reverse relation with custom related_name)
289+>>> p4.cars_optional.clear()
290+m2m_changed signal
291+instance: Airbag
292+action: clear
293+reverse: True
294+field_name: optional_parts
295+model: <class 'modeltests.m2m_signals.models.Car'>
296+objects: None
297+
298+# alternative ways of setting relation:
299+
300+>>> c1.default_parts.create(name='Windows')
301+m2m_changed signal
302+instance: VW
303+action: add
304+reverse: False
305+field_name: default_parts
306+model: <class 'modeltests.m2m_signals.models.Part'>
307+objects: [<Part: Windows>]
308+<Part: Windows>
309+
310+# direct assignment clears the set first, then adds
311+>>> c1.default_parts = [p1,p2,p3]
312+m2m_changed signal
313+instance: VW
314+action: clear
315+reverse: False
316+field_name: default_parts
317+model: <class 'modeltests.m2m_signals.models.Part'>
318+objects: None
319+m2m_changed signal
320+instance: VW
321+action: add
322+reverse: False
323+field_name: default_parts
324+model: <class 'modeltests.m2m_signals.models.Part'>
325+objects: [<Part: Wheelset>, <Part: Doors>, <Part: Engine>]
326+
327+>>> models.signals.m2m_changed.disconnect(m2m_changed_test)
328+"""}
329Index: docs/topics/signals.txt
330===================================================================
331--- docs/topics/signals.txt     (.../original)  (revision 784)
332+++ docs/topics/signals.txt     (.../branches/only-5390)        (revision 784)
333@@ -28,7 +28,10 @@
334 
335       Sent before or after a model's :meth:`~django.db.models.Model.delete`
336       method is called.
337+     
338+    * :data:`django.db.models.signals.m2m_changed`
339 
340+      Sent when a :class:`ManyToManyField` on a model is changed.
341 
342     * :data:`django.core.signals.request_started` &
343       :data:`django.core.signals.request_finished`
344Index: docs/ref/signals.txt
345===================================================================
346--- docs/ref/signals.txt        (.../original)  (revision 784)
347+++ docs/ref/signals.txt        (.../branches/only-5390)        (revision 784)
348@@ -170,6 +170,139 @@
349         Note that the object will no longer be in the database, so be very
350         careful what you do with this instance.
351 
352+m2m_changed
353+-----------
354+
355+.. data:: django.db.models.signals.m2m_changed
356+   :module:
357+
358+Sent when a :class:`ManyToManyField` is changed on a model instance.
359+Strictly speaking, this is not a model signal since it is sent by the
360+:class:`ManyToManyField`, but since it complements the
361+:data:`pre_save`/:data:`post_save` and :data:`pre_delete`/:data:`post_delete`
362+when it comes to tracking changes to models, it is included here.
363+
364+Arguments sent with this signal:
365+
366+    ``sender``
367+        The model class containing the :class:`ManyToManyField`.
368+
369+    ``instance``
370+        The instance whose many-to-many relation is updated. This can be an
371+        instance of the ``sender``, or of the class the :class:`ManyToManyField`
372+        is related to.
373+
374+    ``action``
375+        A string indicating the type of update that is done on the relation.
376+        This can be one of the following:
377+       
378+        ``"add"``
379+            Sent *after* one or more objects are added to the relation,
380+        ``"remove"``       
381+            Sent *after* one or more objects are removed from the relation,
382+        ``"clear"``
383+            Sent *before* the relation is cleared.
384+       
385+    ``model``
386+        The class of the objects that are added to, removed from or cleared
387+        from the relation.
388+       
389+    ``field_name``
390+        The name of the :class:`ManyToManyField` in the ``sender`` class.
391+        This can be used to identify which relation has changed
392+        when multiple many-to-many relations to the same model
393+        exist in ``sender``.
394+       
395+    ``reverse``
396+       Indicates which side of the relation is updated.
397+       It is ``False`` for updates on an instance of the ``sender`` and
398+        ``True`` for updates on an instance of the related class.
399+       
400+    ``objects``
401+        With the ``"add"`` and ``"remove"`` action, this is a list of
402+        objects that have been added to or removed from the relation.
403+        The class of these objects is given in the ``model`` argument.
404+       
405+        For the ``"clear"`` action, this is ``None`` .
406+       
407+        Note that if you pass primary keys to the ``add`` or ``remove`` method
408+        of a relation, ``objects`` will contain primary keys, not instances.
409+        Also note that if you pass objects to the ``add`` method that are
410+        already in the relation, they will not be in the ``objects`` list.
411+        (This doesn't apply to ``remove``.)
412+       
413+For example, if a ``Pizza`` can have multiple ``Topping`` objects, modeled
414+like this:
415+
416+.. code-block:: python
417+
418+    class Topping(models.Model):
419+        # ...
420+
421+    class Pizza(models.Model):
422+        # ...
423+        toppings = models.ManyToManyField(Topping)
424+
425+If we would do something like this:
426+
427+.. code-block:: python
428+
429+    >>> p = Pizza.object.create(...)
430+    >>> t = Topping.objects.create(...)
431+    >>> p.toppings.add(t)
432+       
433+the arguments sent to a :data:`m2m_changed` handler would be:
434+
435+    ==============  ============================================================
436+    Argument        Value
437+    ==============  ============================================================
438+    ``sender``      ``Pizza`` (the class containing the field)
439+
440+    ``instance``    ``p`` (the ``Pizza`` instance being modified)
441+
442+    ``action``      ``"add"`` since the ``add`` method was called
443+   
444+    ``model``       ``Topping`` (the class of the objects added to the
445+                    ``Pizza``)
446+   
447+    ``reverse``     ``False`` (since ``Pizza`` contains the
448+                    :class:`ManyToManyField`)
449+   
450+    ``field_name``  ``"topping"`` (the name of the :class:`ManyToManyField`)
451+   
452+    ``objects``     ``[t]`` (since only ``Topping t`` was added to the relation)
453+    ==============  ============================================================
454+
455+And if we would then do something like this:
456+
457+.. code-block:: python
458+
459+    >>> t.pizza_set.remove(p)
460+       
461+the arguments sent to a :data:`m2m_changed` handler would be:
462+
463+    ==============  ============================================================
464+    Argument        Value
465+    ==============  ============================================================
466+    ``sender``      ``Pizza`` (the class containing the field)
467+
468+    ``instance``    ``t`` (the ``Topping`` instance being modified)
469+
470+    ``action``      ``"remove"`` since the ``remove`` method was called
471+   
472+    ``model``       ``Pizza`` (the class of the objects removed from the
473+                    ``Topping``)
474+   
475+    ``reverse``     ``True`` (since ``Pizza`` contains the
476+                    :class:`ManyToManyField` but the relation is modified
477+                    through ``Topping``)
478+   
479+    ``field_name``  ``"topping"`` (the name of the :class:`ManyToManyField`)
480+   
481+    ``objects``     ``[p]`` (since only ``Pizza p`` was removed from the
482+                    relation)
483+    ==============  ============================================================
484+
485 class_prepared
486 --------------
487