django-rest-framework icon indicating copy to clipboard operation
django-rest-framework copied to clipboard

Meta.read_only_fields does not affects declared fields

Open qwiglydee opened this issue 10 years ago • 11 comments

I believe that Meta.read_only_fields, Meta.fields, Meta.exclude should be applied to declared fields.

A serializer can be inherited, and some field could be declared in base serializer (either because they are not model-based, or because base serializer has some custom fields). To make such fields read_only in derived serializer - one have to copy-paste fields declaration from base just to specify read_only kwarg. The similar comes with include/exclude cases.

from django.db.models import Model, fields as dfields
from django.test import TestCase

from rest_framework import fields as sfields
from rest_framework.serializers import Serializer, ModelSerializer


class Base(Model):
    meta = { 'abstract': True }
    foo = dfields.IntegerField()

class Deriv(Base, Model):
    bar = dfields.IntegerField()


class PlainBaseSerializer(ModelSerializer):
    class Meta:
        model = Base

class PlainDerivSerializer(PlainBaseSerializer, ModelSerializer):
    class Meta:
        model = Deriv
        read_only_fields = ('foo',)


class TestPlainSerializers(TestCase):
    "ModelSerializers without custom fields -- work as expected"

    def test_readony_skipped(self):
        s = PlainDerivSerializer(data={'foo':"1", 'bar': "2"})
        self.assertTrue(s.is_valid())
        self.assertEqual(set(s.data.keys()), set(['bar']))


class CustomField(sfields.IntegerField):
    def to_internal_value(self, value):
        return super().to_internal_value(value)
    def to_representation(self, value):
        return super().to_representation(value)

class FuzzyBaseSerializer(ModelSerializer):
    class Meta:
        model = Base
    foo = CustomField()

class FuzzyDerivSerializer(FuzzyBaseSerializer, ModelSerializer):
    class Meta:
        model = Deriv
        read_only_fields = ('foo',)


class TestFuzzySerializers(TestCase):
    "ModelSerializers with custom fields -- do not work"

    def test_readony_skipped(self):
        s = FuzzyDerivSerializer(data={'foo':"1", 'bar': "2"})
        self.assertTrue(s.is_valid())
        self.assertEqual(set(s.data.keys()), set(['bar']))


class SimpleBaseSerializer(Serializer):
    foo = sfields.IntegerField()

class SimpleDerivSerializer(SimpleBaseSerializer, Serializer):
    class Meta:
        read_only_fields = ('foo',)
    bar = sfields.IntegerField()


class TestSimpleSerializers(TestCase):
    "non-model Serializers -- do not work"

    def test_readony_skipped(self):
        ser = SimpleDerivSerializer(data={'foo':"1", 'bar': "2"})
        ser.is_valid()
        self.assertEqual(set(ser.data.keys()), set(['bar']))

qwiglydee avatar Oct 22 '15 23:10 qwiglydee

Manage this branch in Squash

Test this branch here: https://engamirengbug-fixget-storage-hjxjd.squash.io

squash-labs[bot] avatar May 06 '23 15:05 squash-labs[bot]

Thanks @engAmirEng! Please could you add a test to cover this fix?

gasman avatar May 09 '23 17:05 gasman

Thanks @engAmirEng! Please could you add a test to cover this fix?

No problem, I really want to but I really don't know how, I came across this issue implementing the behavior that instantiates storage based on database values

engAmirEng avatar May 09 '23 17:05 engAmirEng

Could you provide a minimal example (maybe cut down from on https://medium.com/@hiteshgarg14/how-to-dynamically-select-storage-in-django-filefield-bc2e8f5883fd) that gives the incorrect result with the current code? Maybe we can then adapt that into a test.

gasman avatar May 11 '23 09:05 gasman

@gasman What do you think about this test? I think it's more general than providing a specific example

engAmirEng avatar May 13 '23 08:05 engAmirEng

Flagging for another review

lb- avatar Jun 03 '23 01:06 lb-