diff --git a/django/db/models/fields/related_descriptors.py b/django/db/models/fields/related_descriptors.py index 8b9ddf0fb4..9f6c6858ec 100644 --- a/django/db/models/fields/related_descriptors.py +++ b/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 ]) diff --git a/docs/ref/models/relations.txt b/docs/ref/models/relations.txt index 353504a58e..6274506a87 100644 --- a/docs/ref/models/relations.txt +++ b/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 ` 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 ` 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 ` 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 - ` 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. diff --git a/docs/releases/2.2.txt b/docs/releases/2.2.txt index 5d1333ad95..d3c8e9abcc 100644 --- a/docs/releases/2.2.txt +++ b/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 ~~~~~~~~~~~~~~~~~~~~~~ diff --git a/docs/topics/db/examples/many_to_many.txt b/docs/topics/db/examples/many_to_many.txt index ed4a093964..746fb2f550 100644 --- a/docs/topics/db/examples/many_to_many.txt +++ b/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 -` 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``:: diff --git a/docs/topics/db/models.txt b/docs/topics/db/models.txt index 1a340a4567..f895db89fa 100644 --- a/docs/topics/db/models.txt +++ b/docs/topics/db/models.txt @@ -511,37 +511,31 @@ the intermediate model:: >>> beatles.members.all() , ]> -Unlike normal many-to-many fields, you *can't* use ``add()``, ``create()``, -or ``set()`` to create relationships:: +You can also use ``add()``, ``create()``, or ``set()`` to create relationships, +as long as your specify ``through_defaults`` for any required fields:: - >>> # The following statements will not work - >>> beatles.members.add(john) - >>> beatles.members.create(name="George Harrison") - >>> beatles.members.set([john, paul, ringo, george]) + >>> 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)}) -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. +You may prefer to create instances of the intermediate model directly. -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:: +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() , , ]> - >>> # 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() + ]> -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() -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') diff --git a/tests/m2m_through/models.py b/tests/m2m_through/models.py index a84608a530..141d02daf8 100644 --- a/tests/m2m_through/models.py +++ b/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): diff --git a/tests/m2m_through/tests.py b/tests/m2m_through/tests.py index 930f5e848c..2921a1a260 100644 --- a/tests/m2m_through/tests.py +++ b/tests/m2m_through/tests.py @@ -1,6 +1,7 @@ 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): + 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) + + 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') + + 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) - msg = 'Cannot use remove() on a ManyToManyField which specifies an intermediary model' + self.rock.members.remove(self.jim) + self.assertSequenceEqual(self.rock.members.all(), []) - with self.assertRaisesMessage(AttributeError, msg): - self.rock.members.remove(self.jim) + 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(), []) - self.assertQuerysetEqual( - self.rock.members.all(), - ['Jim'], - attrgetter("name") - ) - - 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_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' + self.bob.group_set.remove(self.rock) + self.assertSequenceEqual(self.bob.group_set.all(), []) - with self.assertRaisesMessage(AttributeError, msg): - self.bob.group_set.remove(self.rock) - - 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) diff --git a/tests/m2m_through_regress/tests.py b/tests/m2m_through_regress/tests.py index 7454bac99d..b0e12e2745 100644 --- a/tests/m2m_through_regress/tests.py +++ b/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(), [