Browse Source

Fixed #34345 -- Added system check for ManyToManyFields with intermediate tables in ModelAdmin.filter_horizontal/vertical.

Hrushikesh 1 year ago
parent
commit
107865780a
3 changed files with 56 additions and 4 deletions
  1. 10 0
      django/contrib/admin/checks.py
  2. 4 3
      docs/ref/checks.txt
  3. 42 1
      tests/modeladmin/test_checks.py

+ 10 - 0
django/contrib/admin/checks.py

@@ -533,6 +533,16 @@ class BaseModelAdminChecks:
                 return must_be(
                 return must_be(
                     "a many-to-many field", option=label, obj=obj, id="admin.E020"
                     "a many-to-many field", option=label, obj=obj, id="admin.E020"
                 )
                 )
+            elif not field.remote_field.through._meta.auto_created:
+                return [
+                    checks.Error(
+                        f"The value of '{label}' cannot include the ManyToManyField "
+                        f"'{field_name}', because that field manually specifies a "
+                        f"relationship model.",
+                        obj=obj.__class__,
+                        id="admin.E013",
+                    )
+                ]
             else:
             else:
                 return []
                 return []
 
 

+ 4 - 3
docs/ref/checks.txt

@@ -637,9 +637,10 @@ with the admin site:
 * **admin.E011**: The value of ``fieldsets[n][1]`` must contain the key
 * **admin.E011**: The value of ``fieldsets[n][1]`` must contain the key
   ``fields``.
   ``fields``.
 * **admin.E012**: There are duplicate field(s) in ``fieldsets[n][1]``.
 * **admin.E012**: There are duplicate field(s) in ``fieldsets[n][1]``.
-* **admin.E013**: The value of ``fields[n]/fieldsets[n][m]`` cannot include the
-  ``ManyToManyField`` ``<field name>``, because that field manually specifies a
-  relationship model.
+* **admin.E013**: The value of
+  ``fields[n]/filter_horizontal[n]/filter_vertical[n]/fieldsets[n][m]`` cannot
+  include the ``ManyToManyField`` ``<field name>``, because that field manually
+  specifies a relationship model.
 * **admin.E014**: The value of ``exclude`` must be a list or tuple.
 * **admin.E014**: The value of ``exclude`` must be a list or tuple.
 * **admin.E015**: The value of ``exclude`` contains duplicate field(s).
 * **admin.E015**: The value of ``exclude`` contains duplicate field(s).
 * **admin.E016**: The value of ``form`` must inherit from ``BaseModelForm``.
 * **admin.E016**: The value of ``form`` must inherit from ``BaseModelForm``.

+ 42 - 1
tests/modeladmin/test_checks.py

@@ -4,10 +4,11 @@ from django.contrib.admin import BooleanFieldListFilter, SimpleListFilter
 from django.contrib.admin.options import VERTICAL, ModelAdmin, TabularInline
 from django.contrib.admin.options import VERTICAL, ModelAdmin, TabularInline
 from django.contrib.admin.sites import AdminSite
 from django.contrib.admin.sites import AdminSite
 from django.core.checks import Error
 from django.core.checks import Error
-from django.db.models import CASCADE, F, Field, ForeignKey, Model
+from django.db.models import CASCADE, F, Field, ForeignKey, ManyToManyField, Model
 from django.db.models.functions import Upper
 from django.db.models.functions import Upper
 from django.forms.models import BaseModelFormSet
 from django.forms.models import BaseModelFormSet
 from django.test import SimpleTestCase
 from django.test import SimpleTestCase
+from django.test.utils import isolate_apps
 
 
 from .models import Band, Song, User, ValidationTestInlineModel, ValidationTestModel
 from .models import Band, Song, User, ValidationTestInlineModel, ValidationTestModel
 
 
@@ -321,6 +322,26 @@ class FilterVerticalCheckTests(CheckTestCase):
             "admin.E020",
             "admin.E020",
         )
         )
 
 
+    @isolate_apps("modeladmin")
+    def test_invalid_m2m_field_with_through(self):
+        class Artist(Model):
+            bands = ManyToManyField("Band", through="BandArtist")
+
+        class BandArtist(Model):
+            artist = ForeignKey("Artist", on_delete=CASCADE)
+            band = ForeignKey("Band", on_delete=CASCADE)
+
+        class TestModelAdmin(ModelAdmin):
+            filter_vertical = ["bands"]
+
+        self.assertIsInvalid(
+            TestModelAdmin,
+            Artist,
+            "The value of 'filter_vertical[0]' cannot include the ManyToManyField "
+            "'bands', because that field manually specifies a relationship model.",
+            "admin.E013",
+        )
+
     def test_valid_case(self):
     def test_valid_case(self):
         class TestModelAdmin(ModelAdmin):
         class TestModelAdmin(ModelAdmin):
             filter_vertical = ("users",)
             filter_vertical = ("users",)
@@ -363,6 +384,26 @@ class FilterHorizontalCheckTests(CheckTestCase):
             "admin.E020",
             "admin.E020",
         )
         )
 
 
+    @isolate_apps("modeladmin")
+    def test_invalid_m2m_field_with_through(self):
+        class Artist(Model):
+            bands = ManyToManyField("Band", through="BandArtist")
+
+        class BandArtist(Model):
+            artist = ForeignKey("Artist", on_delete=CASCADE)
+            band = ForeignKey("Band", on_delete=CASCADE)
+
+        class TestModelAdmin(ModelAdmin):
+            filter_horizontal = ["bands"]
+
+        self.assertIsInvalid(
+            TestModelAdmin,
+            Artist,
+            "The value of 'filter_horizontal[0]' cannot include the ManyToManyField "
+            "'bands', because that field manually specifies a relationship model.",
+            "admin.E013",
+        )
+
     def test_valid_case(self):
     def test_valid_case(self):
         class TestModelAdmin(ModelAdmin):
         class TestModelAdmin(ModelAdmin):
             filter_horizontal = ("users",)
             filter_horizontal = ("users",)