Fixed #9475 -- Allowed RelatedManager.add(), create(), etc. for m2m with a through model.

django/db/models/fields/related_descriptors.py

@@ -956,32 +956,23 @@ def create_forward_many_to_many_manager(superclass, rel, reverse):
             constrained_target = self.constrained_target
             return constrained_target.count() if constrained_target else super().count()
 
-        def add(self, *objs):
-            if not rel.through._meta.auto_created:
-                opts = self.through._meta
-                raise AttributeError(
-                    "Cannot use add() on a ManyToManyField which specifies an "
-                    "intermediary model. Use %s.%s's Manager instead." %
-                    (opts.app_label, opts.object_name)
-                )
+        def add(self, *objs, through_defaults=None):
             self._remove_prefetched_objects()
             db = router.db_for_write(self.through, instance=self.instance)
             with transaction.atomic(using=db, savepoint=False):
-                self._add_items(self.source_field_name, self.target_field_name, *objs)
-
-                # If this is a symmetrical m2m relation to self, add the mirror entry in the m2m table
+                self._add_items(
+                    self.source_field_name, self.target_field_name, *objs,
+                    through_defaults=through_defaults,
+                )
+                # If this is a symmetrical m2m relation to self, add the mirror
+                # entry in the m2m table. `through_defaults` aren't used here
+                # because of the system check error fields.E332: Many-to-many
+                # fields with intermediate tables mus not be symmetrical.
                 if self.symmetrical:
                     self._add_items(self.target_field_name, self.source_field_name, *objs)
         add.alters_data = True
 
         def remove(self, *objs):
-            if not rel.through._meta.auto_created:
-                opts = self.through._meta
-                raise AttributeError(
-                    "Cannot use remove() on a ManyToManyField which specifies "
-                    "an intermediary model. Use %s.%s's Manager instead." %
-                    (opts.app_label, opts.object_name)
-                )
             self._remove_prefetched_objects()
             self._remove_items(self.source_field_name, self.target_field_name, *objs)
         remove.alters_data = True
@@ -1005,15 +996,7 @@ def create_forward_many_to_many_manager(superclass, rel, reverse):
                 )
         clear.alters_data = True
 
-        def set(self, objs, *, clear=False):
-            if not rel.through._meta.auto_created:
-                opts = self.through._meta
-                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)
-                )
-
+        def set(self, objs, *, clear=False, through_defaults=None):
             # Force evaluation of `objs` in case it's a queryset whose value
             # could be affected by `manager.clear()`. Refs #19816.
             objs = tuple(objs)
@@ -1022,7 +1005,7 @@ def create_forward_many_to_many_manager(superclass, rel, reverse):
             with transaction.atomic(using=db, savepoint=False):
                 if clear:
                     self.clear()
-                    self.add(*objs)
+                    self.add(*objs, through_defaults=through_defaults)
                 else:
                     old_ids = set(self.using(db).values_list(self.target_field.target_field.attname, flat=True))
 
@@ -1038,49 +1021,41 @@ def create_forward_many_to_many_manager(superclass, rel, reverse):
                             new_objs.append(obj)
 
                     self.remove(*old_ids)
-                    self.add(*new_objs)
+                    self.add(*new_objs, through_defaults=through_defaults)
         set.alters_data = True
 
-        def create(self, **kwargs):
-            # This check needs to be done here, since we can't later remove this
-            # from the method lookup table, as we do with add and remove.
-            if not self.through._meta.auto_created:
-                opts = self.through._meta
-                raise AttributeError(
-                    "Cannot use create() on a ManyToManyField which specifies "
-                    "an intermediary model. Use %s.%s's Manager instead." %
-                    (opts.app_label, opts.object_name)
-                )
+        def create(self, through_defaults=None, **kwargs):
             db = router.db_for_write(self.instance.__class__, instance=self.instance)
             new_obj = super(ManyRelatedManager, self.db_manager(db)).create(**kwargs)
-            self.add(new_obj)
+            self.add(new_obj, through_defaults=through_defaults)
             return new_obj
         create.alters_data = True
 
-        def get_or_create(self, **kwargs):
+        def get_or_create(self, through_defaults=None, **kwargs):
             db = router.db_for_write(self.instance.__class__, instance=self.instance)
             obj, created = super(ManyRelatedManager, self.db_manager(db)).get_or_create(**kwargs)
             # We only need to add() if created because if we got an object back
             # from get() then the relationship already exists.
             if created:
-                self.add(obj)
+                self.add(obj, through_defaults=through_defaults)
             return obj, created
         get_or_create.alters_data = True
 
-        def update_or_create(self, **kwargs):
+        def update_or_create(self, through_defaults=None, **kwargs):
             db = router.db_for_write(self.instance.__class__, instance=self.instance)
             obj, created = super(ManyRelatedManager, self.db_manager(db)).update_or_create(**kwargs)
             # We only need to add() if created because if we got an object back
             # from get() then the relationship already exists.
             if created:
-                self.add(obj)
+                self.add(obj, through_defaults=through_defaults)
             return obj, created
         update_or_create.alters_data = True
 
-        def _add_items(self, source_field_name, target_field_name, *objs):
+        def _add_items(self, source_field_name, target_field_name, *objs, through_defaults=None):
             # source_field_name: the PK fieldname in join table for the source object
             # target_field_name: the PK fieldname in join table for the target object
             # *objs - objects to add. Either object instances, or primary keys of object instances.
+            through_defaults = through_defaults or {}
 
             # If there aren't any objects, there is nothing to do.
             from django.db.models import Model
@@ -1130,10 +1105,10 @@ def create_forward_many_to_many_manager(superclass, rel, reverse):
 
                     # Add the ones that aren't there already
                     self.through._default_manager.using(db).bulk_create([
-                        self.through(**{
+                        self.through(**dict(through_defaults, **{
                             '%s_id' % source_field_name: self.related_val[0],
                             '%s_id' % target_field_name: obj_id,
-                        })
+                        }))
                         for obj_id in new_ids
                     ])
 

docs/ref/models/relations.txt

@@ -36,7 +36,7 @@ Related objects reference
       In this example, the methods below will be available both on
       ``topping.pizza_set`` and on ``pizza.toppings``.
 
-    .. method:: add(*objs, bulk=True)
+    .. method:: add(*objs, bulk=True, through_defaults=None)
 
         Adds the specified model objects to the related object set.
 
@@ -66,7 +66,15 @@ Related objects reference
         Using ``add()`` on a relation that already exists won't duplicate the
         relation, but it will still trigger signals.
 
-    .. method:: create(**kwargs)
+        Use the ``through_defaults`` argument to specify values for the new
+        :ref:`intermediate model <intermediary-manytomany>` instance(s), if
+        needed.
+
+        .. versionchanged:: 2.2
+
+            The ``through_defaults`` argument was added.
+
+    .. method:: create(through_defaults=None, **kwargs)
 
         Creates a new object, saves it and puts it in the related object set.
         Returns the newly created object::
@@ -96,6 +104,14 @@ Related objects reference
         parameter ``blog`` to ``create()``. Django figures out that the new
         ``Entry`` object's ``blog`` field should be set to ``b``.
 
+        Use the ``through_defaults`` argument to specify values for the new
+        :ref:`intermediate model <intermediary-manytomany>` instance, if
+        needed.
+
+        .. versionchanged:: 2.2
+
+            The ``through_defaults`` argument was added.
+
     .. method:: remove(*objs, bulk=True)
 
         Removes the specified model objects from the related object set::
@@ -149,7 +165,7 @@ Related objects reference
         For many-to-many relationships, the ``bulk`` keyword argument doesn't
         exist.
 
-    .. method:: set(objs, bulk=True, clear=False)
+    .. method:: set(objs, bulk=True, clear=False, through_defaults=None)
 
         Replace the set of related objects::
 
@@ -172,6 +188,14 @@ Related objects reference
         race conditions. For instance, new objects may be added to the database
         in between the call to ``clear()`` and the call to ``add()``.
 
+        Use the ``through_defaults`` argument to specify values for the new
+        :ref:`intermediate model <intermediary-manytomany>` instance(s), if
+        needed.
+
+        .. versionchanged:: 2.2
+
+            The ``through_defaults`` argument was added.
+
     .. note::
 
        Note that ``add()``, ``create()``, ``remove()``, ``clear()``, and
@@ -179,11 +203,6 @@ Related objects reference
        related fields. In other words, there is no need to call ``save()``
        on either end of the relationship.
 
-       Also, if you are using :ref:`an intermediate model
-       <intermediary-manytomany>` for a many-to-many relationship, then the
-       ``add()``, ``create()``, ``remove()``, and ``set()`` methods are
-       disabled.
-
        If you use :meth:`~django.db.models.query.QuerySet.prefetch_related`,
        the ``add()``, ``remove()``, ``clear()``, and ``set()`` methods clear
        the prefetched cache.

docs/releases/2.2.txt

@@ -256,6 +256,13 @@ Models
   specified on initialization to ensure that the aggregate function is only
   called for each distinct value of ``expressions``.
 
+* The :meth:`.RelatedManager.add`, :meth:`~.RelatedManager.create`,
+  :meth:`~.RelatedManager.remove`,  :meth:`~.RelatedManager.set`,
+  ``get_or_create()``, and ``update_or_create()`` methods are now allowed on
+  many-to-many relationships with intermediate models. The new
+  ``through_defaults`` argument is used to specify values for new intermediate
+  model instance(s).
+
 Requests and Responses
 ~~~~~~~~~~~~~~~~~~~~~~
 

docs/topics/db/examples/many_to_many.txt

@@ -34,10 +34,7 @@ objects, and a ``Publication`` has multiple ``Article`` objects:
             return self.headline
 
 What follows are examples of operations that can be performed using the Python
-API facilities. Note that if you are using :ref:`an intermediate model
-<intermediary-manytomany>` for a many-to-many relationship, some of the related
-manager's methods are disabled, so some of these examples won't work with such
-models.
+API facilities.
 
 Create a few ``Publications``::
 

docs/topics/db/models.txt

@@ -511,37 +511,31 @@ the intermediate model::
     >>> beatles.members.all()
     <QuerySet [<Person: Ringo Starr>, <Person: Paul McCartney>]>
 
-Unlike normal many-to-many fields, you *can't* use ``add()``, ``create()``,
-or ``set()`` to create relationships::
-
-    >>> # The following statements will not work
-    >>> beatles.members.add(john)
-    >>> beatles.members.create(name="George Harrison")
-    >>> beatles.members.set([john, paul, ringo, george])
-
-Why? You can't just create a relationship between a ``Person`` and a ``Group``
-- you need to specify all the detail for the relationship required by the
-``Membership`` model. The simple ``add``, ``create`` and assignment calls
-don't provide a way to specify this extra detail. As a result, they are
-disabled for many-to-many relationships that use an intermediate model.
-The only way to create this type of relationship is to create instances of the
-intermediate model.
-
-The :meth:`~django.db.models.fields.related.RelatedManager.remove` method is
-disabled for similar reasons. For example, if the custom through table defined
-by the intermediate model does not enforce uniqueness on the
-``(model1, model2)`` pair, a ``remove()`` call would not provide enough
-information as to which intermediate model instance should be deleted::
+You can also use ``add()``, ``create()``, or ``set()`` to create relationships,
+as long as your specify ``through_defaults`` for any required fields::
+
+    >>> beatles.members.add(john, through_defaults={'date_joined': date(1960, 8, 1)})
+    >>> beatles.members.create(name="George Harrison", through_defaults={'date_joined': date(1960, 8, 1)})
+    >>> beatles.members.set([john, paul, ringo, george], through_defaults={'date_joined': date(1960, 8, 1)})
+
+You may prefer to create instances of the intermediate model directly.
+
+If the custom through table defined by the intermediate model does not enforce
+uniqueness on the ``(model1, model2)`` pair, allowing multiple values, the
+:meth:`~django.db.models.fields.related.RelatedManager.remove` call will
+remove all intermediate model instances::
 
     >>> Membership.objects.create(person=ringo, group=beatles,
     ...     date_joined=date(1968, 9, 4),
     ...     invite_reason="You've been gone for a month and we miss you.")
     >>> beatles.members.all()
     <QuerySet [<Person: Ringo Starr>, <Person: Paul McCartney>, <Person: Ringo Starr>]>
-    >>> # This will not work because it cannot tell which membership to remove
+    >>> # This deletes both of the intermediate model instances for Ringo Starr
     >>> beatles.members.remove(ringo)
+    >>> beatles.members.all()
+    <QuerySet [<Person: Paul McCartney>]>
 
-However, the :meth:`~django.db.models.fields.related.RelatedManager.clear`
+The :meth:`~django.db.models.fields.related.RelatedManager.clear`
 method can be used to remove all many-to-many relationships for an instance::
 
     >>> # Beatles have broken up
@@ -550,10 +544,9 @@ method can be used to remove all many-to-many relationships for an instance::
     >>> Membership.objects.all()
     <QuerySet []>
 
-Once you have established the many-to-many relationships by creating instances
-of your intermediate model, you can issue queries. Just as with normal
-many-to-many relationships, you can query using the attributes of the
-many-to-many-related model::
+Once you have established the many-to-many relationships, you can issue
+queries. Just as with normal many-to-many relationships, you can query using
+the attributes of the many-to-many-related model::
 
     # Find all the groups with a member whose name starts with 'Paul'
     >>> Group.objects.filter(members__name__startswith='Paul')

tests/m2m_through/models.py

@@ -66,7 +66,7 @@ class CustomMembership(models.Model):
 class TestNoDefaultsOrNulls(models.Model):
     person = models.ForeignKey(Person, models.CASCADE)
     group = models.ForeignKey(Group, models.CASCADE)
-    nodefaultnonull = models.CharField(max_length=5)
+    nodefaultnonull = models.IntegerField()
 
 
 class PersonSelfRefM2M(models.Model):

tests/m2m_through/tests.py

 from datetime import datetime
 from operator import attrgetter
 
+from django.db import IntegrityError
 from django.test import TestCase
 
 from .models import (
@@ -56,52 +57,76 @@ class M2mThroughTests(TestCase):
             expected
         )
 
-    def test_cannot_use_add_on_m2m_with_intermediary_model(self):
-        msg = 'Cannot use add() on a ManyToManyField which specifies an intermediary model'
+    def test_add_on_m2m_with_intermediate_model(self):
+        self.rock.members.add(self.bob, through_defaults={'invite_reason': 'He is good.'})
+        self.assertSequenceEqual(self.rock.members.all(), [self.bob])
+        self.assertEqual(self.rock.membership_set.get().invite_reason, 'He is good.')
 
-        with self.assertRaisesMessage(AttributeError, msg):
-            self.rock.members.add(self.bob)
+    def test_add_on_m2m_with_intermediate_model_value_required(self):
+        self.rock.nodefaultsnonulls.add(self.jim, through_defaults={'nodefaultnonull': 1})
+        self.assertEqual(self.rock.testnodefaultsornulls_set.get().nodefaultnonull, 1)
 
-        self.assertQuerysetEqual(
-            self.rock.members.all(),
-            []
-        )
+    def test_add_on_m2m_with_intermediate_model_value_required_fails(self):
+        with self.assertRaises(IntegrityError):
+            self.rock.nodefaultsnonulls.add(self.jim)
 
-    def test_cannot_use_create_on_m2m_with_intermediary_model(self):
-        msg = 'Cannot use create() on a ManyToManyField which specifies an intermediary model'
+    def test_create_on_m2m_with_intermediate_model(self):
+        annie = self.rock.members.create(name='Annie', through_defaults={'invite_reason': 'She was just awesome.'})
+        self.assertSequenceEqual(self.rock.members.all(), [annie])
+        self.assertEqual(self.rock.membership_set.get().invite_reason, 'She was just awesome.')
 
-        with self.assertRaisesMessage(AttributeError, msg):
-            self.rock.members.create(name='Annie')
+    def test_create_on_m2m_with_intermediate_model_value_required(self):
+        self.rock.nodefaultsnonulls.create(name='Test', through_defaults={'nodefaultnonull': 1})
+        self.assertEqual(self.rock.testnodefaultsornulls_set.get().nodefaultnonull, 1)
 
-        self.assertQuerysetEqual(
-            self.rock.members.all(),
-            []
-        )
+    def test_create_on_m2m_with_intermediate_model_value_required_fails(self):
+        with self.assertRaises(IntegrityError):
+            self.rock.nodefaultsnonulls.create(name='Test')
 
-    def test_cannot_use_remove_on_m2m_with_intermediary_model(self):
-        Membership.objects.create(person=self.jim, group=self.rock)
-        msg = 'Cannot use remove() on a ManyToManyField which specifies an intermediary model'
+    def test_get_or_create_on_m2m_with_intermediate_model_value_required(self):
+        self.rock.nodefaultsnonulls.get_or_create(name='Test', through_defaults={'nodefaultnonull': 1})
+        self.assertEqual(self.rock.testnodefaultsornulls_set.get().nodefaultnonull, 1)
 
-        with self.assertRaisesMessage(AttributeError, msg):
-            self.rock.members.remove(self.jim)
+    def test_get_or_create_on_m2m_with_intermediate_model_value_required_fails(self):
+        with self.assertRaises(IntegrityError):
+            self.rock.nodefaultsnonulls.get_or_create(name='Test')
 
-        self.assertQuerysetEqual(
-            self.rock.members.all(),
-            ['Jim'],
-            attrgetter("name")
-        )
+    def test_update_or_create_on_m2m_with_intermediate_model_value_required(self):
+        self.rock.nodefaultsnonulls.update_or_create(name='Test', through_defaults={'nodefaultnonull': 1})
+        self.assertEqual(self.rock.testnodefaultsornulls_set.get().nodefaultnonull, 1)
+
+    def test_update_or_create_on_m2m_with_intermediate_model_value_required_fails(self):
+        with self.assertRaises(IntegrityError):
+            self.rock.nodefaultsnonulls.update_or_create(name='Test')
+
+    def test_remove_on_m2m_with_intermediate_model(self):
+        Membership.objects.create(person=self.jim, group=self.rock)
+        self.rock.members.remove(self.jim)
+        self.assertSequenceEqual(self.rock.members.all(), [])
 
-    def test_cannot_use_setattr_on_m2m_with_intermediary_model(self):
-        msg = 'Cannot set values on a ManyToManyField which specifies an intermediary model'
+    def test_remove_on_m2m_with_intermediate_model_multiple(self):
+        Membership.objects.create(person=self.jim, group=self.rock, invite_reason='1')
+        Membership.objects.create(person=self.jim, group=self.rock, invite_reason='2')
+        self.assertSequenceEqual(self.rock.members.all(), [self.jim, self.jim])
+        self.rock.members.remove(self.jim)
+        self.assertSequenceEqual(self.rock.members.all(), [])
+
+    def test_set_on_m2m_with_intermediate_model(self):
         members = list(Person.objects.filter(name__in=['Bob', 'Jim']))
+        self.rock.members.set(members)
+        self.assertSequenceEqual(self.rock.members.all(), [self.bob, self.jim])
 
-        with self.assertRaisesMessage(AttributeError, msg):
-            self.rock.members.set(members)
+    def test_set_on_m2m_with_intermediate_model_value_required(self):
+        self.rock.nodefaultsnonulls.set([self.jim], through_defaults={'nodefaultnonull': 1})
+        self.assertEqual(self.rock.testnodefaultsornulls_set.get().nodefaultnonull, 1)
+        self.rock.nodefaultsnonulls.set([self.jim], through_defaults={'nodefaultnonull': 2})
+        self.assertEqual(self.rock.testnodefaultsornulls_set.get().nodefaultnonull, 1)
+        self.rock.nodefaultsnonulls.set([self.jim], through_defaults={'nodefaultnonull': 2}, clear=True)
+        self.assertEqual(self.rock.testnodefaultsornulls_set.get().nodefaultnonull, 2)
 
-        self.assertQuerysetEqual(
-            self.rock.members.all(),
-            []
-        )
+    def test_set_on_m2m_with_intermediate_model_value_required_fails(self):
+        with self.assertRaises(IntegrityError):
+            self.rock.nodefaultsnonulls.set([self.jim])
 
     def test_clear_removes_all_the_m2m_relationships(self):
         Membership.objects.create(person=self.jim, group=self.rock)
@@ -125,52 +150,23 @@ class M2mThroughTests(TestCase):
             attrgetter("name")
         )
 
-    def test_cannot_use_add_on_reverse_m2m_with_intermediary_model(self):
-        msg = 'Cannot use add() on a ManyToManyField which specifies an intermediary model'
+    def test_add_on_reverse_m2m_with_intermediate_model(self):
+        self.bob.group_set.add(self.rock)
+        self.assertSequenceEqual(self.bob.group_set.all(), [self.rock])
 
-        with self.assertRaisesMessage(AttributeError, msg):
-            self.bob.group_set.add(self.bob)
+    def test_create_on_reverse_m2m_with_intermediate_model(self):
+        funk = self.bob.group_set.create(name='Funk')
+        self.assertSequenceEqual(self.bob.group_set.all(), [funk])
 
-        self.assertQuerysetEqual(
-            self.bob.group_set.all(),
-            []
-        )
-
-    def test_cannot_use_create_on_reverse_m2m_with_intermediary_model(self):
-        msg = 'Cannot use create() on a ManyToManyField which specifies an intermediary model'
-
-        with self.assertRaisesMessage(AttributeError, msg):
-            self.bob.group_set.create(name='Funk')
-
-        self.assertQuerysetEqual(
-            self.bob.group_set.all(),
-            []
-        )
-
-    def test_cannot_use_remove_on_reverse_m2m_with_intermediary_model(self):
+    def test_remove_on_reverse_m2m_with_intermediate_model(self):
         Membership.objects.create(person=self.bob, group=self.rock)
-        msg = 'Cannot use remove() on a ManyToManyField which specifies an intermediary model'
-
-        with self.assertRaisesMessage(AttributeError, msg):
-            self.bob.group_set.remove(self.rock)
+        self.bob.group_set.remove(self.rock)
+        self.assertSequenceEqual(self.bob.group_set.all(), [])
 
-        self.assertQuerysetEqual(
-            self.bob.group_set.all(),
-            ['Rock'],
-            attrgetter('name')
-        )
-
-    def test_cannot_use_setattr_on_reverse_m2m_with_intermediary_model(self):
-        msg = 'Cannot set values on a ManyToManyField which specifies an intermediary model'
+    def test_set_on_reverse_m2m_with_intermediate_model(self):
         members = list(Group.objects.filter(name__in=['Rock', 'Roll']))
-
-        with self.assertRaisesMessage(AttributeError, msg):
-            self.bob.group_set.set(members)
-
-        self.assertQuerysetEqual(
-            self.bob.group_set.all(),
-            []
-        )
+        self.bob.group_set.set(members)
+        self.assertSequenceEqual(self.bob.group_set.all(), [self.rock, self.roll])
 
     def test_clear_on_reverse_removes_all_the_m2m_relationships(self):
         Membership.objects.create(person=self.jim, group=self.rock)

tests/m2m_through_regress/tests.py

@@ -47,42 +47,6 @@ class M2MThroughTestCase(TestCase):
             ]
         )
 
-    def test_cannot_use_setattr_on_reverse_m2m_with_intermediary_model(self):
-        msg = (
-            "Cannot set values on a ManyToManyField which specifies an "
-            "intermediary model. Use m2m_through_regress.Membership's Manager "
-            "instead."
-        )
-        with self.assertRaisesMessage(AttributeError, msg):
-            self.bob.group_set.set([])
-
-    def test_cannot_use_setattr_on_forward_m2m_with_intermediary_model(self):
-        msg = (
-            "Cannot set values on a ManyToManyField which specifies an "
-            "intermediary model. Use m2m_through_regress.Membership's Manager "
-            "instead."
-        )
-        with self.assertRaisesMessage(AttributeError, msg):
-            self.roll.members.set([])
-
-    def test_cannot_use_create_on_m2m_with_intermediary_model(self):
-        msg = (
-            "Cannot use create() on a ManyToManyField which specifies an "
-            "intermediary model. Use m2m_through_regress.Membership's "
-            "Manager instead."
-        )
-        with self.assertRaisesMessage(AttributeError, msg):
-            self.rock.members.create(name="Anne")
-
-    def test_cannot_use_create_on_reverse_m2m_with_intermediary_model(self):
-        msg = (
-            "Cannot use create() on a ManyToManyField which specifies an "
-            "intermediary model. Use m2m_through_regress.Membership's "
-            "Manager instead."
-        )
-        with self.assertRaisesMessage(AttributeError, msg):
-            self.bob.group_set.create(name="Funk")
-
     def test_retrieve_reverse_m2m_items_via_custom_id_intermediary(self):
         self.assertQuerysetEqual(
             self.frank.group_set.all(), [