Ticket #12203: 12203-m2m-w-through-modeladmin-fields-option2.diff

django/contrib/admin/validation.py

@@ -149 +149 @@
             validate_inline(inline, cls, model)
 
 def validate_inline(cls, parent, parent_model):
-    
+
     # model is already verified to exist and be a Model
     if cls.fk_name: # default value is None
         f = get_field(cls, cls.model, cls.model._meta, 'fk_name', cls.fk_name)
@@ -200 +200 @@
             raise ImproperlyConfigured('Both fieldsets and fields are specified in %s.' % cls.__name__)
         if len(cls.fields) > len(set(cls.fields)):
             raise ImproperlyConfigured('There are duplicate field(s) in %s.fields' % cls.__name__)
+        for idx, field in enumerate(cls.fields):
+            f = get_field(cls, model, opts, 'fields', field)
+            if isinstance(f, models.ManyToManyField) and f.rel.through and not f.rel.through._meta.auto_created:
+                if f.rel.through:
+                    validate_fields_m2m_through_model(cls, model, field, f, idx)
 
     # fieldsets
     if cls.fieldsets: # default value is None
@@ -272 +277 @@
             for idx, f in enumerate(val):
                 get_field(cls, model, opts, "prepopulated_fields['%s'][%d]" % (field, idx), f)
 
+def validate_fields_m2m_through_model(cls, model, field, f, idx):
+    from_model, to_model, pk, uniquet = None, None, False, False
+    for i, through_model_fld in enumerate(f.rel.through._meta.fields):
+        if isinstance(through_model_fld, models.ForeignKey):
+            rel_to = through_model_fld.rel.to
+            if rel_to == model:
+                from_model = through_model_fld.name
+            elif rel_to == f.rel.to:
+                to_model = through_model_fld.name
+            else:
+                break
+        elif through_model_fld.primary_key: # XXX hasattr?
+            if pk:
+                break
+            pk = True
+    if hasattr(f.rel.through._meta, 'unique_together'):
+        unique_together = f.rel.through._meta.unique_together
+        if unique_together and isinstance(unique_together, (list, tuple)):
+            unique_together = unique_together[0]
+        if from_model in unique_together and \
+            to_model in unique_together and len(unique_together) == 2:
+                uniquet = True
+    if not (i == 2 and from_model and to_model and pk and uniquet):
+        raise ImproperlyConfigured("%s.fields: Included '%s' ManyToManyField field uses 'through' option but specifies a non suitable intermediate model (%s)." % (cls.__name__, field, f.rel.through.__name__))
+
 def check_isseq(cls, label, obj):
     if not isinstance(obj, (list, tuple)):
         raise ImproperlyConfigured("'%s.%s' must be a list or tuple." % (cls.__name__, label))

tests/regressiontests/admin_validation/models.py

@@ -25 +25 @@
     album2 = models.ForeignKey(Album, related_name="album2_set")
     e = models.CharField(max_length=1)
 
+# Validation of inclusion of m2m fields in ModelAdmin.fields. See #12203
 
+class Author(models.Model):
+    name = models.CharField(max_length=30)
+
+# Missing unique_together in the intermediate model:
+
+class Book(models.Model):
+    name = models.CharField(max_length=20)
+    authors = models.ManyToManyField(Author, through='AuthorsBooks')
+
+class AuthorsBooks(models.Model):
+    author = models.ForeignKey(Author)
+    book = models.ForeignKey(Book)
+
+# Extra data in the intermediate model:
+
+class Ballad(models.Model):
+    name = models.CharField(max_length=20)
+    authors = models.ManyToManyField(Author, through='AuthorsBallads')
+
+class AuthorsBallads(models.Model):
+    author = models.ForeignKey(Author)
+    ballad = models.ForeignKey(Ballad)
+    extra_data = models.IntegerField()
+
+    class Meta:
+        unique_together = ('author', 'ballad')
+
+# Intermediate model PK field isn't an independent one:
+
+class Act(models.Model):
+    name = models.CharField(max_length=20)
+    authors = models.ManyToManyField(Author, through='AuthorsActs')
+
+class AuthorsActs(models.Model):
+    author = models.ForeignKey(Author, primary_key=True)
+    act = models.ForeignKey(Act)
+
+# This through-using m2m complies with all the requirements needed to be
+# listed in ModelAdmin.fields, should validate succesfully:
+
+class Paper(models.Model):
+    name = models.CharField(max_length=20)
+    authors = models.ManyToManyField(Author, through='AuthorsPapers')
+
+
+class AuthorsPapers(models.Model):
+    author = models.ForeignKey(Author)
+    paper = models.ForeignKey(Paper)
+
+    class Meta:
+        unique_together = ('author', 'paper')
+
+# Intermediate model has a manually set PK, should work too:
+
+class Post(models.Model):
+    name = models.CharField(max_length=20)
+    authors = models.ManyToManyField(Author, through='AuthorsPosts')
+
+
+class AuthorsPosts(models.Model):
+    mypk = models.AutoField(primary_key=True)
+    author = models.ForeignKey(Author)
+    post = models.ForeignKey(Post)
+
+    class Meta:
+        unique_together = ('author', 'post')
+
+# Recursive m2m with through, should work too:
+
+class Writer(models.Model):
+    name = models.CharField(max_length=30)
+    similar = models.ManyToManyField('self', through='Influence', symmetrical=False)
+
+
+class Influence(models.Model):
+    inspirator = models.ForeignKey(Writer, related_name='influenced_by')
+    inspired = models.ForeignKey(Writer, related_name='influenced')
 
 __test__ = {'API_TESTS':"""
 
@@ -95 +173 @@
 
 >>> validate_inline(TwoAlbumFKAndAnEInline, None, Album)
 
+# Regression test for #12203 - Fail more gracefully when a M2M field that
+# specifies the 'through' option is included in the 'fields' ModelAdmin option.
+
+>>> class BookAdmin(admin.ModelAdmin):
+...     fields = ['authors']
+
+>>> validate(BookAdmin, Book)
+Traceback (most recent call last):
+    ...
+ImproperlyConfigured: BookAdmin.fields: Included 'authors' ManyToManyField field uses 'through' option but specifies a non suitable intermediate model (AuthorsBooks).
+
+>>> class BalladAdmin(admin.ModelAdmin):
+...     fields = ['authors']
+
+>>> validate(BalladAdmin, Ballad)
+Traceback (most recent call last):
+    ...
+ImproperlyConfigured: BalladAdmin.fields: Included 'authors' ManyToManyField field uses 'through' option but specifies a non suitable intermediate model (AuthorsBallads).
+
+>>> class ActAdmin(admin.ModelAdmin):
+...     fields = ['authors']
+
+>>> validate(ActAdmin, Act)
+Traceback (most recent call last):
+    ...
+ImproperlyConfigured: ActAdmin.fields: Included 'authors' ManyToManyField field uses 'through' option but specifies a non suitable intermediate model (AuthorsActs).
+
+>>> class PaperAdmin(admin.ModelAdmin):
+...     fields = ['authors']
+
+>>> validate(PaperAdmin, Paper)
+
+>>> class PostAdmin(admin.ModelAdmin):
+...     fields = ['authors']
+
+>>> validate(PostAdmin, Post)
+
 """}