Allow missing fields option for inherited serializers. Closes #2388.

4 files changed, 64 insertions, 25 deletions

diff --git a/rest_framework/compat.py b/rest_framework/compat.py
index 17814136..766afaec 100644
--- a/rest_framework/compat.py
+++ b/rest_framework/compat.py
@@ -20,7 +20,7 @@ def unicode_repr(instance):
     # Get the repr of an instance, but ensure it is a unicode string
     # on both python 3 (already the case) and 2 (not the case).
     if six.PY2:
-        repr(instance).decode('utf-8')
+        return repr(instance).decode('utf-8')
     return repr(instance)
 
 
diff --git a/rest_framework/serializers.py b/rest_framework/serializers.py
index e373cd10..6320a075 100644
--- a/rest_framework/serializers.py
+++ b/rest_framework/serializers.py
@@ -253,7 +253,7 @@ class SerializerMetaclass(type):
         # If this class is subclassing another Serializer, add that Serializer's
         # fields.  Note that we loop over the bases in *reverse*. This is necessary
         # in order to maintain the correct order of fields.
-        for base in bases[::-1]:
+        for base in reversed(bases):
             if hasattr(base, '_declared_fields'):
                 fields = list(base._declared_fields.items()) + fields
 
@@ -880,8 +880,8 @@ class ModelSerializer(Serializer):
         # Retrieve metadata about fields & relationships on the model class.
         info = model_meta.get_field_info(model)
 
-        # Use the default set of field names if none is supplied explicitly.
         if fields is None:
+            # Use the default set of field names if none is supplied explicitly.
             fields = self._get_default_field_names(declared_fields, info)
             exclude = getattr(self.Meta, 'exclude', None)
             if exclude is not None:
@@ -891,6 +891,23 @@ class ModelSerializer(Serializer):
                         field_name
                     )
                     fields.remove(field_name)
+        else:
+            # Check that any fields declared on the class are
+            # also explicitly included in `Meta.fields`.
+
+            # Note that we ignore any fields that were declared on a parent
+            # class, in order to support only including a subset of fields
+            # when subclassing serializers.
+            declared_field_names = set(declared_fields.keys())
+            for cls in self.__class__.__bases__:
+                declared_field_names -= set(getattr(cls, '_declared_fields', []))
+
+            missing_fields = declared_field_names - set(fields)
+            assert not missing_fields, (
+                'Field `%s` has been declared on serializer `%s`, but '
+                'is missing from `Meta.fields`.' %
+                (list(missing_fields)[0], self.__class__.__name__)
+            )
 
         # Determine the set of model fields, and the fields that they map to.
         # We actually only need this to deal with the slightly awkward case
@@ -1024,17 +1041,6 @@ class ModelSerializer(Serializer):
                     (field_name, model.__class__.__name__)
                 )
 
-            # Check that any fields declared on the class are
-            # also explicitly included in `Meta.fields`.
-            missing_fields = set(declared_fields.keys()) - set(fields)
-            if missing_fields:
-                missing_field = list(missing_fields)[0]
-                raise ImproperlyConfigured(
-                    'Field `%s` has been declared on serializer `%s`, but '
-                    'is missing from `Meta.fields`.' %
-                    (missing_field, self.__class__.__name__)
-                )
-
             # Populate any kwargs defined in `Meta.extra_kwargs`
             extras = extra_kwargs.get(field_name, {})
             if extras.get('read_only', False):
diff --git a/rest_framework/utils/serializer_helpers.py b/rest_framework/utils/serializer_helpers.py
index f9960603..ab057862 100644
--- a/rest_framework/utils/serializer_helpers.py
+++ b/rest_framework/utils/serializer_helpers.py
@@ -105,3 +105,6 @@ class BindingDict(collections.MutableMapping):
 
     def __len__(self):
         return len(self.fields)
+
+    def __repr__(self):
+        return dict.__repr__(self.fields)
diff --git a/tests/test_model_serializer.py b/tests/test_model_serializer.py
index ee556dbc..247b309a 100644
--- a/tests/test_model_serializer.py
+++ b/tests/test_model_serializer.py
@@ -5,11 +5,14 @@ shortcuts for automatically creating serializers based on a given model class.
 These tests deal with ensuring that we correctly map the model fields onto
 an appropriate set of serializer fields for each case.
 """
+from __future__ import unicode_literals
 from django.core.exceptions import ImproperlyConfigured
 from django.core.validators import MaxValueValidator, MinValueValidator, MinLengthValidator
 from django.db import models
 from django.test import TestCase
+from django.utils import six
 from rest_framework import serializers
+from rest_framework.compat import unicode_repr
 
 
 def dedent(blocktext):
@@ -124,7 +127,7 @@ class TestRegularFieldMappings(TestCase):
                 url_field = URLField(max_length=100)
                 custom_field = ModelField(model_field=<tests.test_model_serializer.CustomField: custom_field>)
         """)
-        self.assertEqual(repr(TestSerializer()), expected)
+        self.assertEqual(unicode_repr(TestSerializer()), expected)
 
     def test_field_options(self):
         class TestSerializer(serializers.ModelSerializer):
@@ -142,7 +145,14 @@ class TestRegularFieldMappings(TestCase):
                 descriptive_field = IntegerField(help_text='Some help text', label='A label')
                 choices_field = ChoiceField(choices=[('red', 'Red'), ('blue', 'Blue'), ('green', 'Green')])
         """)
-        self.assertEqual(repr(TestSerializer()), expected)
+        if six.PY2:
+            # This particular case is too awkward to resolve fully across
+            # both py2 and py3.
+            expected = expected.replace(
+                "('red', 'Red'), ('blue', 'Blue'), ('green', 'Green')",
+                "(u'red', u'Red'), (u'blue', u'Blue'), (u'green', u'Green')"
+            )
+        self.assertEqual(unicode_repr(TestSerializer()), expected)
 
     def test_method_field(self):
         """
@@ -221,7 +231,7 @@ class TestRegularFieldMappings(TestCase):
                 model = RegularFieldsModel
                 fields = ('auto_field',)
 
-        with self.assertRaises(ImproperlyConfigured) as excinfo:
+        with self.assertRaises(AssertionError) as excinfo:
             TestSerializer().fields
         expected = (
             'Field `missing` has been declared on serializer '
@@ -229,6 +239,26 @@ class TestRegularFieldMappings(TestCase):
         )
         assert str(excinfo.exception) == expected
 
+    def test_missing_superclass_field(self):
+        """
+        Fields that have been declared on a parent of the serializer class may
+        be excluded from the `Meta.fields` option.
+        """
+        class TestSerializer(serializers.ModelSerializer):
+            missing = serializers.ReadOnlyField()
+
+            class Meta:
+                model = RegularFieldsModel
+
+        class ChildSerializer(TestSerializer):
+            missing = serializers.ReadOnlyField()
+
+            class Meta:
+                model = RegularFieldsModel
+                fields = ('auto_field',)
+
+        ChildSerializer().fields
+
 
 # Tests for relational field mappings.
 # ------------------------------------
@@ -276,7 +306,7 @@ class TestRelationalFieldMappings(TestCase):
                 many_to_many = PrimaryKeyRelatedField(many=True, queryset=ManyToManyTargetModel.objects.all())
                 through = PrimaryKeyRelatedField(many=True, read_only=True)
         """)
-        self.assertEqual(repr(TestSerializer()), expected)
+        self.assertEqual(unicode_repr(TestSerializer()), expected)
 
     def test_nested_relations(self):
         class TestSerializer(serializers.ModelSerializer):
@@ -300,7 +330,7 @@ class TestRelationalFieldMappings(TestCase):
                     id = IntegerField(label='ID', read_only=True)
                     name = CharField(max_length=100)
         """)
-        self.assertEqual(repr(TestSerializer()), expected)
+        self.assertEqual(unicode_repr(TestSerializer()), expected)
 
     def test_hyperlinked_relations(self):
         class TestSerializer(serializers.HyperlinkedModelSerializer):
@@ -315,7 +345,7 @@ class TestRelationalFieldMappings(TestCase):
                 many_to_many = HyperlinkedRelatedField(many=True, queryset=ManyToManyTargetModel.objects.all(), view_name='manytomanytargetmodel-detail')
                 through = HyperlinkedRelatedField(many=True, read_only=True, view_name='throughtargetmodel-detail')
         """)
-        self.assertEqual(repr(TestSerializer()), expected)
+        self.assertEqual(unicode_repr(TestSerializer()), expected)
 
     def test_nested_hyperlinked_relations(self):
         class TestSerializer(serializers.HyperlinkedModelSerializer):
@@ -339,7 +369,7 @@ class TestRelationalFieldMappings(TestCase):
                     url = HyperlinkedIdentityField(view_name='throughtargetmodel-detail')
                     name = CharField(max_length=100)
         """)
-        self.assertEqual(repr(TestSerializer()), expected)
+        self.assertEqual(unicode_repr(TestSerializer()), expected)
 
     def test_pk_reverse_foreign_key(self):
         class TestSerializer(serializers.ModelSerializer):
@@ -353,7 +383,7 @@ class TestRelationalFieldMappings(TestCase):
                 name = CharField(max_length=100)
                 reverse_foreign_key = PrimaryKeyRelatedField(many=True, queryset=RelationalModel.objects.all())
         """)
-        self.assertEqual(repr(TestSerializer()), expected)
+        self.assertEqual(unicode_repr(TestSerializer()), expected)
 
     def test_pk_reverse_one_to_one(self):
         class TestSerializer(serializers.ModelSerializer):
@@ -367,7 +397,7 @@ class TestRelationalFieldMappings(TestCase):
                 name = CharField(max_length=100)
                 reverse_one_to_one = PrimaryKeyRelatedField(queryset=RelationalModel.objects.all())
         """)
-        self.assertEqual(repr(TestSerializer()), expected)
+        self.assertEqual(unicode_repr(TestSerializer()), expected)
 
     def test_pk_reverse_many_to_many(self):
         class TestSerializer(serializers.ModelSerializer):
@@ -381,7 +411,7 @@ class TestRelationalFieldMappings(TestCase):
                 name = CharField(max_length=100)
                 reverse_many_to_many = PrimaryKeyRelatedField(many=True, queryset=RelationalModel.objects.all())
         """)
-        self.assertEqual(repr(TestSerializer()), expected)
+        self.assertEqual(unicode_repr(TestSerializer()), expected)
 
     def test_pk_reverse_through(self):
         class TestSerializer(serializers.ModelSerializer):
@@ -395,7 +425,7 @@ class TestRelationalFieldMappings(TestCase):
                 name = CharField(max_length=100)
                 reverse_through = PrimaryKeyRelatedField(many=True, read_only=True)
         """)
-        self.assertEqual(repr(TestSerializer()), expected)
+        self.assertEqual(unicode_repr(TestSerializer()), expected)
 
 
 class TestIntegration(TestCase):