From 36ab17738fdfec06a9e82d94fc1aa1fdba842811 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Ricardo=20Louren=C3=A7o?= Date: Thu, 28 Oct 2021 11:48:14 +0100 Subject: [PATCH] Warn when extra_field_kwargs includes fields explicitly declared (related to #3460) --- rest_framework/serializers.py | 18 ++++++++++++++++-- tests/test_model_serializer.py | 23 +++++++++++++++++++++++ 2 files changed, 39 insertions(+), 2 deletions(-) diff --git a/rest_framework/serializers.py b/rest_framework/serializers.py index 389680517..7ca2d57ab 100644 --- a/rest_framework/serializers.py +++ b/rest_framework/serializers.py @@ -1062,12 +1062,26 @@ class ModelSerializer(Serializer): fields = OrderedDict() for field_name in field_names: + extra_field_kwargs = extra_kwargs.get(field_name, {}) + # If the field is explicitly declared on the class then use that. if field_name in declared_fields: - fields[field_name] = declared_fields[field_name] + assert not extra_field_kwargs, ( + "The field '{field_name}' was declared on serializer " + "{serializer_class}, but has also been included in " + "extra_fields_kwargs ({extra_field_kwargs}). " + "This must be a mistake, because extra_field_kwargs are " + "not applied to declared fields. For example, if you want " + "the field to be read-only, you must explicitly pass " + "read_only=True when declaring, instead of including the " + "field in read_only_fields.".format( + field_name=field_name, + serializer_class=self.__class__.__name__, + extra_field_kwargs=extra_field_kwargs + ) + ) continue - extra_field_kwargs = extra_kwargs.get(field_name, {}) source = extra_field_kwargs.get('source', '*') if source == '*': source = field_name diff --git a/tests/test_model_serializer.py b/tests/test_model_serializer.py index 7da1b41ae..2bb7b372a 100644 --- a/tests/test_model_serializer.py +++ b/tests/test_model_serializer.py @@ -1333,3 +1333,26 @@ class Issue6751Test(TestCase): serializer.save() self.assertEqual(instance.char_field, 'value changed by signal') + +class ExtraKwargsForDeclaredFieldsModel(models.Model): + test_field = models.CharField(max_length=64) + +class TestExtraKwargsForDeclaredFields(TestCase): + """ + Passing extra_kwargs for a declared field should throw an AssertionError. + Particularly relevant for read_only_fields. See + https://github.com/encode/django-rest-framework/issues/3460. + """ + def test_extra_kwargs_for_declared_fields(self): + class TestSerializer(serializers.ModelSerializer): + test_field = serializers.CharField() + + class Meta: + model = ExtraKwargsForDeclaredFieldsModel + fields = ('test_field', ) + read_only_fields = ('test_field', ) + + with pytest.raises(AssertionError) as e_info: + TestSerializer(data={'test_field': 'abc'}).fields + + assert 'extra_field_kwargs' in str(e_info.value)