Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 1 commit into from Jun 21, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
10 changes: 10 additions & 0 deletions django/contrib/admin/checks.py
Expand Up @@ -533,6 +533,16 @@ def _check_filter_item(self, obj, field_name, label):
return must_be(
"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:
return []

Expand Down
7 changes: 4 additions & 3 deletions docs/ref/checks.txt
Expand Up @@ -637,9 +637,10 @@ with the admin site:
* **admin.E011**: The value of ``fieldsets[n][1]`` must contain the key
``fields``.
* **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.E015**: The value of ``exclude`` contains duplicate field(s).
* **admin.E016**: The value of ``form`` must inherit from ``BaseModelForm``.
Expand Down
43 changes: 42 additions & 1 deletion tests/modeladmin/test_checks.py
Expand Up @@ -4,10 +4,11 @@
from django.contrib.admin.options import VERTICAL, ModelAdmin, TabularInline
from django.contrib.admin.sites import AdminSite
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.forms.models import BaseModelFormSet
from django.test import SimpleTestCase
from django.test.utils import isolate_apps

from .models import Band, Song, User, ValidationTestInlineModel, ValidationTestModel

Expand Down Expand Up @@ -321,6 +322,26 @@ class TestModelAdmin(ModelAdmin):
"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):
class TestModelAdmin(ModelAdmin):
filter_vertical = ("users",)
Expand Down Expand Up @@ -363,6 +384,26 @@ class TestModelAdmin(ModelAdmin):
"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):
class TestModelAdmin(ModelAdmin):
filter_horizontal = ("users",)
Expand Down