Merge pull request #854 from ryankask/issue-775

All relations pointing to ManyToManyFields should default to read-only

docs/api-guide/relations.md

@@ -381,6 +381,15 @@ Note that reverse generic keys, expressed using the `GenericRelation` field, can
 
 For more information see [the Django documentation on generic relations][generic-relations].
 
+## ManyToManyFields with a Through Model
+
+By default, relational fields that target a ``ManyToManyField`` with a
+``through`` model specified are set to read-only.
+
+If you exlicitly specify a relational field pointing to a
+``ManyToManyField`` with a through model, be sure to set ``read_only``
+to ``True``.
+
 ## Advanced Hyperlinked fields
 
 If you have very specific requirements for the style of your hyperlinked relationships you can override `HyperlinkedRelatedField`. 

rest_framework/serializers.py

@@ -591,11 +591,16 @@ class ModelSerializer(Serializer):
         forward_rels += [field for field in opts.many_to_many if field.serialize]
 
         for model_field in forward_rels:
+            has_user_through_model = False
+
             if model_field.rel:
                 to_many = isinstance(model_field,
                                      models.fields.related.ManyToManyField)
                 related_model = model_field.rel.to
 
+                if to_many and not model_field.rel.through._meta.auto_created:
+                    has_user_through_model = True
+
             if model_field.rel and nested:
                 if len(inspect.getargspec(self.get_nested_field).args) == 2:
                     warnings.warn(
@@ -624,6 +629,9 @@ class ModelSerializer(Serializer):
                 field = self.get_field(model_field)
 
             if field:
+                if has_user_through_model:
+                    field.read_only = True
+
                 ret[model_field.name] = field
 
         # Deal with reverse relationships
@@ -641,6 +649,12 @@ class ModelSerializer(Serializer):
                 continue
             related_model = relation.model
             to_many = relation.field.rel.multiple
+            has_user_through_model = False
+            is_m2m = isinstance(relation.field,
+                                models.fields.related.ManyToManyField)
+
+            if is_m2m and not relation.field.rel.through._meta.auto_created:
+                has_user_through_model = True
 
             if nested:
                 field = self.get_nested_field(None, related_model, to_many)
@@ -648,6 +662,9 @@ class ModelSerializer(Serializer):
                 field = self.get_related_field(None, related_model, to_many)
 
             if field:
+                if has_user_through_model:
+                    field.read_only = True
+
                 ret[accessor_name] = field
 
         # Add the `read_only` flag to any fields that have bee specified

rest_framework/tests/relations_pk.py

@@ -1,4 +1,5 @@
 from __future__ import unicode_literals
+from django.db import models
 from django.test import TestCase
 from rest_framework import serializers
 from rest_framework.tests.models import (
@@ -127,6 +128,7 @@ class PKManyToManyTests(TestCase):
         # Ensure source 4 is added, and everything else is as expected
         queryset = ManyToManySource.objects.all()
         serializer = ManyToManySourceSerializer(queryset, many=True)
+        self.assertFalse(serializer.fields['targets'].read_only)
         expected = [
             {'id': 1, 'name': 'source-1', 'targets': [1]},
             {'id': 2, 'name': 'source-2', 'targets': [1, 2]},
@@ -138,6 +140,7 @@ class PKManyToManyTests(TestCase):
     def test_reverse_many_to_many_create(self):
         data = {'id': 4, 'name': 'target-4', 'sources': [1, 3]}
         serializer = ManyToManyTargetSerializer(data=data)
+        self.assertFalse(serializer.fields['sources'].read_only)
         self.assertTrue(serializer.is_valid())
         obj = serializer.save()
         self.assertEqual(serializer.data, data)
@@ -426,8 +429,69 @@ class PKNullableOneToOneTests(TestCase):
         self.assertEqual(serializer.data, expected)
 
 
+# The below models and tests ensure that serializer fields corresponding
+# to a ManyToManyField field with a user-specified ``through`` model are
+# set to read only
+
+
+class ManyToManyThroughTarget(models.Model):
+    name = models.CharField(max_length=100)
+
+
+class ManyToManyThrough(models.Model):
+    source = models.ForeignKey('ManyToManyThroughSource')
+    target = models.ForeignKey(ManyToManyThroughTarget)
+
+
+class ManyToManyThroughSource(models.Model):
+    name = models.CharField(max_length=100)
+    targets = models.ManyToManyField(ManyToManyThroughTarget,
+                                     related_name='sources',
+                                     through='ManyToManyThrough')
+
+
+class ManyToManyThroughTargetSerializer(serializers.ModelSerializer):
+    class Meta:
+        model = ManyToManyThroughTarget
+        fields = ('id', 'name', 'sources')
+
+
+class ManyToManyThroughSourceSerializer(serializers.ModelSerializer):
+    class Meta:
+        model = ManyToManyThroughSource
+        fields = ('id', 'name', 'targets')
+
+
+class PKManyToManyThroughTests(TestCase):
+    def setUp(self):
+        self.source = ManyToManyThroughSource.objects.create(
+            name='through-source-1')
+        self.target = ManyToManyThroughTarget.objects.create(
+            name='through-target-1')
+
+    def test_many_to_many_create(self):
+        data = {'id': 2, 'name': 'source-2', 'targets': [self.target.pk]}
+        serializer = ManyToManyThroughSourceSerializer(data=data)
+        self.assertTrue(serializer.fields['targets'].read_only)
+        self.assertTrue(serializer.is_valid())
+        obj = serializer.save()
+        self.assertEqual(obj.name, 'source-2')
+        self.assertEqual(obj.targets.count(), 0)
+
+    def test_many_to_many_reverse_create(self):
+        data = {'id': 2, 'name': 'target-2', 'sources': [self.source.pk]}
+        serializer = ManyToManyThroughTargetSerializer(data=data)
+        self.assertTrue(serializer.fields['sources'].read_only)
+        self.assertTrue(serializer.is_valid())
+        serializer.save()
+        obj = serializer.save()
+        self.assertEqual(obj.name, 'target-2')
+        self.assertEqual(obj.sources.count(), 0)
+
+
 # Regression tests for #694 (`source` attribute on related fields)
 
+
 class PrimaryKeyRelatedFieldSourceTests(TestCase):
     def test_related_manager_source(self):
         """