Fixed #30421 -- Allowed symmetrical intermediate table for self-referential ManyToManyField.

AUTHORS

@@ -632,6 +632,7 @@ answer newbie questions, and generally made Django that much better:
     msundstr
     Mushtaq Ali <mushtaak@gmail.com>
     Mykola Zamkovoi <nickzam@gmail.com>
+    Nadège Michel <michel.nadege@gmail.com>
     Nagy Károly <charlie@rendszergazda.com>
     Nasimul Haque <nasim.haque@gmail.com>
     Nasir Hussain <nasirhjafri@gmail.com>

django/db/models/fields/related.py

@@ -1234,18 +1234,6 @@ class ManyToManyField(RelatedField):
                 to_model_name = to_model._meta.object_name
             relationship_model_name = self.remote_field.through._meta.object_name
             self_referential = from_model == to_model
-
-            # Check symmetrical attribute.
-            if (self_referential and self.remote_field.symmetrical and
-                    not self.remote_field.through._meta.auto_created):
-                errors.append(
-                    checks.Error(
-                        'Many-to-many fields with intermediate tables must not be symmetrical.',
-                        obj=self,
-                        id='fields.E332',
-                    )
-                )
-
             # Count foreign keys in intermediate model
             if self_referential:
                 seen_self = sum(

django/db/models/fields/related_descriptors.py

@@ -938,11 +938,14 @@ def create_forward_many_to_many_manager(superclass, rel, reverse):
                     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 must not be symmetrical.
+                # entry in the m2m table.
                 if self.symmetrical:
-                    self._add_items(self.target_field_name, self.source_field_name, *objs)
+                    self._add_items(
+                        self.target_field_name,
+                        self.source_field_name,
+                        *objs,
+                        through_defaults=through_defaults,
+                    )
         add.alters_data = True
 
         def remove(self, *objs):

docs/ref/checks.txt

@@ -222,7 +222,7 @@ Related fields
 * **fields.E331**: Field specifies a many-to-many relation through model
   ``<model>``, which has not been installed.
 * **fields.E332**: Many-to-many fields with intermediate tables must not be
-  symmetrical.
+  symmetrical. *This check appeared before Django 3.0.*
 * **fields.E333**: The model is used as an intermediate model by ``<model>``,
   but it has more than two foreign keys to ``<model>``, which is ambiguous.
   You must specify which two foreign keys Django should use via the

docs/ref/models/fields.txt

@@ -1537,6 +1537,11 @@ that control how the relationship functions.
     add the descriptor for the reverse relationship, allowing
     :class:`ManyToManyField` relationships to be non-symmetrical.
 
+    .. versionchanged:: 3.0
+
+        Specifying ``symmetrical=True`` for recursive many-to-many
+        relationships using an intermediary model was allowed.
+
 .. attribute:: ManyToManyField.through
 
     Django will automatically generate a table to manage many-to-many
@@ -1549,6 +1554,16 @@ that control how the relationship functions.
     :ref:`extra data with a many-to-many relationship
     <intermediary-manytomany>`.
 
+    .. note::
+
+        Recursive relationships using an intermediary model and defined as
+        symmetrical (that is, with :attr:`symmetrical=True
+        <ManyToManyField.symmetrical>`, which is the default) can't determine
+        the reverse accessors names, as they would be the same. You need to set
+        a :attr:`~ForeignKey.related_name` to at least one of them. If you'd
+        prefer Django not to create a backwards relation, set ``related_name``
+        to ``'+'``.
+
     If you don't specify an explicit ``through`` model, there is still an
     implicit ``through`` model class you can use to directly access the table
     created to hold the association. It has three fields to link the models.
@@ -1624,12 +1639,6 @@ that control how the relationship functions.
     foreign keys to the model, or you want to explicitly specify which two
     Django should use.
 
-    Recursive relationships using an intermediary model are always defined as
-    non-symmetrical -- that is, with :attr:`symmetrical=False <ManyToManyField.symmetrical>`
-    -- therefore, there is the concept of a "source" and a "target". In that
-    case ``'field1'`` will be treated as the "source" of the relationship and
-    ``'field2'`` as the "target".
-
 .. attribute:: ManyToManyField.db_table
 
     The name of the table to create for storing the many-to-many data. If this

docs/releases/3.0.txt

@@ -254,6 +254,9 @@ Models
 
 * :class:`~django.db.models.FilePathField` now accepts a callable ``path``.
 
+* Allowed symmetrical intermediate table for self-referential
+  :class:`~django.db.models.ManyToManyField`.
+
 Requests and Responses
 ~~~~~~~~~~~~~~~~~~~~~~
 

tests/invalid_models_tests/test_relative_fields.py

@@ -260,24 +260,6 @@ class RelativeFieldTests(SimpleTestCase):
         field = Group._meta.get_field('members')
         self.assertEqual(field.check(from_model=Group), [])
 
-    def test_symmetrical_self_referential_field(self):
-        class Person(models.Model):
-            # Implicit symmetrical=False.
-            friends = models.ManyToManyField('self', through="Relationship")
-
-        class Relationship(models.Model):
-            first = models.ForeignKey(Person, models.CASCADE, related_name="rel_from_set")
-            second = models.ForeignKey(Person, models.CASCADE, related_name="rel_to_set")
-
-        field = Person._meta.get_field('friends')
-        self.assertEqual(field.check(from_model=Person), [
-            Error(
-                'Many-to-many fields with intermediate tables must not be symmetrical.',
-                obj=field,
-                id='fields.E332',
-            ),
-        ])
-
     def test_too_many_foreign_keys_in_self_referential_model(self):
         class Person(models.Model):
             friends = models.ManyToManyField('self', through="InvalidRelationship", symmetrical=False)
@@ -301,52 +283,6 @@ class RelativeFieldTests(SimpleTestCase):
             ),
         ])
 
-    def test_symmetric_self_reference_with_intermediate_table(self):
-        class Person(models.Model):
-            # Explicit symmetrical=True.
-            friends = models.ManyToManyField('self', through="Relationship", symmetrical=True)
-
-        class Relationship(models.Model):
-            first = models.ForeignKey(Person, models.CASCADE, related_name="rel_from_set")
-            second = models.ForeignKey(Person, models.CASCADE, related_name="rel_to_set")
-
-        field = Person._meta.get_field('friends')
-        self.assertEqual(field.check(from_model=Person), [
-            Error(
-                'Many-to-many fields with intermediate tables must not be symmetrical.',
-                obj=field,
-                id='fields.E332',
-            ),
-        ])
-
-    def test_symmetric_self_reference_with_intermediate_table_and_through_fields(self):
-        """
-        Using through_fields in a m2m with an intermediate model shouldn't
-        mask its incompatibility with symmetry.
-        """
-        class Person(models.Model):
-            # Explicit symmetrical=True.
-            friends = models.ManyToManyField(
-                'self',
-                symmetrical=True,
-                through="Relationship",
-                through_fields=('first', 'second'),
-            )
-
-        class Relationship(models.Model):
-            first = models.ForeignKey(Person, models.CASCADE, related_name="rel_from_set")
-            second = models.ForeignKey(Person, models.CASCADE, related_name="rel_to_set")
-            referee = models.ForeignKey(Person, models.CASCADE, related_name="referred")
-
-        field = Person._meta.get_field('friends')
-        self.assertEqual(field.check(from_model=Person), [
-            Error(
-                'Many-to-many fields with intermediate tables must not be symmetrical.',
-                obj=field,
-                id='fields.E332',
-            ),
-        ])
-
     def test_foreign_key_to_abstract_model(self):
         class AbstractModel(models.Model):
             class Meta:

tests/m2m_recursive/models.py

@@ -22,7 +22,14 @@ from django.db import models
 class Person(models.Model):
     name = models.CharField(max_length=20)
     friends = models.ManyToManyField('self')
+    colleagues = models.ManyToManyField('self', symmetrical=True, through='Colleague')
     idols = models.ManyToManyField('self', symmetrical=False, related_name='stalkers')
 
     def __str__(self):
         return self.name
+
+
+class Colleague(models.Model):
+    first = models.ForeignKey(Person, models.CASCADE)
+    second = models.ForeignKey(Person, models.CASCADE, related_name='+')
+    first_meet = models.DateField()

tests/m2m_recursive/tests.py

@@ -1,3 +1,5 @@
+import datetime
+
 from django.test import TestCase
 
 from .models import Person
@@ -59,3 +61,59 @@ class RecursiveM2MTests(TestCase):
         self.a.idols.add(self.a)
         self.assertSequenceEqual(self.a.idols.all(), [self.a])
         self.assertSequenceEqual(self.a.stalkers.all(), [self.a])
+
+
+class RecursiveSymmetricalM2MThroughTests(TestCase):
+    @classmethod
+    def setUpTestData(cls):
+        cls.a, cls.b, cls.c, cls.d = [
+            Person.objects.create(name=name)
+            for name in ['Anne', 'Bill', 'Chuck', 'David']
+        ]
+        cls.a.colleagues.add(cls.b, cls.c, through_defaults={
+            'first_meet': datetime.date(2013, 1, 5),
+        })
+        # Add m2m for Anne and Chuck in reverse direction.
+        cls.d.colleagues.add(cls.a, cls.c, through_defaults={
+            'first_meet': datetime.date(2015, 6, 15),
+        })
+
+    def test_recursive_m2m_all(self):
+        for person, colleagues in (
+            (self.a, [self.b, self.c, self.d]),
+            (self.b, [self.a]),
+            (self.c, [self.a, self.d]),
+            (self.d, [self.a, self.c]),
+        ):
+            with self.subTest(person=person):
+                self.assertSequenceEqual(person.colleagues.all(), colleagues)
+
+    def test_recursive_m2m_reverse_add(self):
+        # Add m2m for Anne in reverse direction.
+        self.b.colleagues.add(self.a, through_defaults={
+            'first_meet': datetime.date(2013, 1, 5),
+        })
+        self.assertSequenceEqual(self.a.colleagues.all(), [self.b, self.c, self.d])
+        self.assertSequenceEqual(self.b.colleagues.all(), [self.a])
+
+    def test_recursive_m2m_remove(self):
+        self.b.colleagues.remove(self.a)
+        self.assertSequenceEqual(self.a.colleagues.all(), [self.c, self.d])
+        self.assertSequenceEqual(self.b.colleagues.all(), [])
+
+    def test_recursive_m2m_clear(self):
+        # Clear m2m for Anne.
+        self.a.colleagues.clear()
+        self.assertSequenceEqual(self.a.friends.all(), [])
+        # Reverse m2m relationships is removed.
+        self.assertSequenceEqual(self.c.colleagues.all(), [self.d])
+        self.assertSequenceEqual(self.d.colleagues.all(), [self.c])
+
+    def test_recursive_m2m_set(self):
+        # Set new relationships for Chuck.
+        self.c.colleagues.set([self.b, self.d], through_defaults={
+            'first_meet': datetime.date(2013, 1, 5),
+        })
+        self.assertSequenceEqual(self.c.colleagues.order_by('name'), [self.b, self.d])
+        # Reverse m2m relationships is removed.
+        self.assertSequenceEqual(self.a.colleagues.order_by('name'), [self.b, self.d])

tests/m2m_through/models.py

@@ -72,6 +72,7 @@ class TestNoDefaultsOrNulls(models.Model):
 class PersonSelfRefM2M(models.Model):
     name = models.CharField(max_length=5)
     friends = models.ManyToManyField('self', through="Friendship", symmetrical=False)
+    sym_friends = models.ManyToManyField('self', through='SymmetricalFriendship', symmetrical=True)
 
     def __str__(self):
         return self.name
@@ -83,6 +84,12 @@ class Friendship(models.Model):
     date_friended = models.DateTimeField()
 
 
+class SymmetricalFriendship(models.Model):
+    first = models.ForeignKey(PersonSelfRefM2M, models.CASCADE)
+    second = models.ForeignKey(PersonSelfRefM2M, models.CASCADE, related_name='+')
+    date_friended = models.DateField()
+
+
 # Custom through link fields
 class Event(models.Model):
     title = models.CharField(max_length=50)

tests/m2m_through/tests.py

@@ -1,4 +1,4 @@
-from datetime import datetime
+from datetime import date, datetime
 from operator import attrgetter
 
 from django.db import IntegrityError
@@ -7,7 +7,7 @@ from django.test import TestCase
 from .models import (
     CustomMembership, Employee, Event, Friendship, Group, Ingredient,
     Invitation, Membership, Person, PersonSelfRefM2M, Recipe, RecipeIngredient,
-    Relationship,
+    Relationship, SymmetricalFriendship,
 )
 
 
@@ -401,7 +401,7 @@ class M2mThroughReferentialTests(TestCase):
             attrgetter("name")
         )
 
-    def test_self_referential_symmetrical(self):
+    def test_self_referential_non_symmetrical_both(self):
         tony = PersonSelfRefM2M.objects.create(name="Tony")
         chris = PersonSelfRefM2M.objects.create(name="Chris")
         Friendship.objects.create(
@@ -439,6 +439,71 @@ class M2mThroughReferentialTests(TestCase):
             attrgetter('name')
         )
 
+    def test_self_referential_symmetrical(self):
+        tony = PersonSelfRefM2M.objects.create(name='Tony')
+        chris = PersonSelfRefM2M.objects.create(name='Chris')
+        SymmetricalFriendship.objects.create(
+            first=tony, second=chris, date_friended=date.today(),
+        )
+        self.assertSequenceEqual(tony.sym_friends.all(), [chris])
+        # Manually created symmetrical m2m relation doesn't add mirror entry
+        # automatically.
+        self.assertSequenceEqual(chris.sym_friends.all(), [])
+        SymmetricalFriendship.objects.create(
+            first=chris, second=tony, date_friended=date.today()
+        )
+        self.assertSequenceEqual(chris.sym_friends.all(), [tony])
+
+    def test_add_on_symmetrical_m2m_with_intermediate_model(self):
+        tony = PersonSelfRefM2M.objects.create(name='Tony')
+        chris = PersonSelfRefM2M.objects.create(name='Chris')
+        date_friended = date(2017, 1, 3)
+        tony.sym_friends.add(chris, through_defaults={'date_friended': date_friended})
+        self.assertSequenceEqual(tony.sym_friends.all(), [chris])
+        self.assertSequenceEqual(chris.sym_friends.all(), [tony])
+        friendship = tony.symmetricalfriendship_set.get()
+        self.assertEqual(friendship.date_friended, date_friended)
+
+    def test_set_on_symmetrical_m2m_with_intermediate_model(self):
+        tony = PersonSelfRefM2M.objects.create(name='Tony')
+        chris = PersonSelfRefM2M.objects.create(name='Chris')
+        anne = PersonSelfRefM2M.objects.create(name='Anne')
+        kate = PersonSelfRefM2M.objects.create(name='Kate')
+        date_friended_add = date(2013, 1, 5)
+        date_friended_set = date.today()
+        tony.sym_friends.add(
+            anne, chris,
+            through_defaults={'date_friended': date_friended_add},
+        )
+        tony.sym_friends.set(
+            [anne, kate],
+            through_defaults={'date_friended': date_friended_set},
+        )
+        self.assertSequenceEqual(tony.sym_friends.all(), [anne, kate])
+        self.assertSequenceEqual(anne.sym_friends.all(), [tony])
+        self.assertSequenceEqual(kate.sym_friends.all(), [tony])
+        self.assertEqual(
+            kate.symmetricalfriendship_set.get().date_friended,
+            date_friended_set,
+        )
+        # Date is preserved.
+        self.assertEqual(
+            anne.symmetricalfriendship_set.get().date_friended,
+            date_friended_add,
+        )
+        # Recreate relationship.
+        tony.sym_friends.set(
+            [anne],
+            clear=True,
+            through_defaults={'date_friended': date_friended_set},
+        )
+        self.assertSequenceEqual(tony.sym_friends.all(), [anne])
+        self.assertSequenceEqual(anne.sym_friends.all(), [tony])
+        self.assertEqual(
+            anne.symmetricalfriendship_set.get().date_friended,
+            date_friended_set,
+        )
+
 
 class M2mThroughToFieldsTests(TestCase):
     @classmethod