Document that explicitly declared fields will not have their attributes overridden by `read_only_fields` shortcut.

[TheProdigyFilippo]

class PersonSerializer(serializers.ModelSerializer):
    info = serializers.Field(source='info')

    class Meta:
        model = Person
        read_only_fields = ('info',)

In this case read_only_fields is not working (info field is not read only) and we have to add read_only=True to the field. It's very confusing.

[jpadilla]

@TheProdigyFilippo I'm pretty sure you'll also need to set fields = ('info', ) as the example in the docs. Also, please note that the discussion group is the best place to take this discussion and other usage questions. Thanks!

[TheProdigyFilippo]

@jpadilla Thank for your interest, but you're wrong. Firstly, in the documentation there's no written anywhere that read_only_fields should work only in conjuction with fields. And it shouldn't. Example is EXAMPLE only. If there would be additionaly some crappy global variable in the docs does it mean, that read_only_fields works only when you also use this global variable?

read_only_fields is not working when you add field explicitly to the serializer class and it is a bug. Even adding this field to fields, what you proposed (but what is stupid) is not working. Real life example:

class UserTransferSerializer(serializers.ModelSerializer):
    date = serializers.DateTimeField(format='%s')

    class Meta:
        model = Transfer
        fields = ('date', 'transfer_cost')
        read_only_fields = ('transfer_cost', 'date')

And the result:

s = UserTransferSerializer()
>>> s.fields
{'date': DateTimeField(format='%s'), 'transfer_cost': DecimalField(decimal_places=8, max_digits=18, read_only=True)}

But why the necessity of adding explicitly created Field to the field tuple is stupid? Because you can use exclude which is mutually exclusive with fields and read_only_fields would be useless.

I hope that now you understand the bug. If you have any more questions, I'll do my best to answer, give few more examples and so on. Of course you can also use the discussion group.

[xordoquy]

@TheProdigyFilippo please refrain from using aggressive tone. A few things to keep in mind:

1. This is no paid support and we can't afford to spend hours on each issue.
2. We do see on regular basis examples of DRF bugs that turns out to be user's bug.
3. If you want to speed up this issue, feel free to provide a failing test case we can work on out of the box.

[TheProdigyFilippo]

Dear @xordoquy @jpadilla, excuse me for my aggresive tone. It was response for treating me like a ingoramus and sending away to the docs (I've read it), discussion group and superfast closing this issue by Jose. I'm very sorry again for that. I wanted to help you guys, DRS still is great and your work also is great :)

[xordoquy]

The fact that we close pretty fast issues is linked with our bandwidth. Examples help but still require a lot of our time which is limited. A failing test case definitively helps understanding the issue and figure whether or not it's a DRF bug and whether or not it needs to be fixed.

[lovelydinosaur]

In this case read_only_fields is not working

Correct - if you've specified a field explicitly it doesn't make sense to have it in read_only_fields. We could as an enhancement error loudly?

[TheProdigyFilippo]

if you've specified a field explicitly it doesn't make sense to have it in read_only_fields

Can't agree with you. It would be definitely better (for a coding style) to take advantage of configuration in one place. In my example I'm forced to set read_only property in two places (in field and read_only_fields) or redefine all model fields in serializer and not use read_only_fields to achieve consistency. But it would be great to set read_only property for all wished fields in read_only_fields.

[xordoquy]

I think it'd be more consistent if read_only_fields would also apply to explicitly defined fields.
After all, we do need to list them to the fields list so they are taken into account.
The major issue I'd see to this change would be the following case:

class PersonSerializer(serializers.ModelSerializer):
    info = serializers.Field(source='info', read_only=False)

    class Meta:
        model = Person
        read_only_fields = ('info',)

[carltongibson]

We had this conversation when the API for 3.0 was being discussed. The summary of it was this:

1. All these convenience flags would be removed, in order to promote a cleaner API.
2. We'd make a special exception for read_only_fields — because it was such a common use-case.

The use-case is when you have a ModelSerializer where you are not explicitly defining fields but quickly want to mark a field as read-only: the obvious example of this is a user field, where you set the user based on request.user and not on submitted data.

We decided that once users stepped onto the path of explicitly defining fields they would be responsible for moving the read-only declaration there, in line with 1 above.

[lovelydinosaur]

It would be definitely better (for a coding style) to take advantage of configuration in one place.

Agree. And if the field has been defined explicitly - that's where.

1. Drop read_only_fields entirely, because it's such an ugly little shortcut.
2. Complain if an explicit field is in read_only_fields.
3. Do nothing.

But having an explicitly defined field that does not match the definition its been given isn't going to happen.

[carltongibson]

Drop read_only_fields entirely, because it's such an ugly little shortcut.

Just on this. It is (an ugly little shortcut) — but it's handy for newcomers to DRF: you can get up and running with ModelSerializer, and get some great results without learning too much. Then you realise need to make your user reference read-only and, without the shortcut, there's instantly a whole load of unfamiliar API that you need to take-in. With the shortcut you can keep progressing and loop back to the Fields API, a good while later.

[lovelydinosaur]

Indeed, so we're not taking (1), and had really already made a decision on that. From my POV that leaves (2) and (3) as valid options.

[twbagustin]

So.. This ain't fixed yet right? Came here through Google.. I don't agree with dropping the shortcut, it's as it was pointed out a "common scenario". If you have a bunch of fields customized say 15 it feels kinda silly to place a read_only flag on each one vs using a shortcut that was actually created for this purpose... Specific use-case: I have a django live production model with a lot of fields, you need the api to throw camelCase field names, so you have to explicitly set each field to rename it, and mark them read_only.. (this points out a feature request maybe, labels dirs on Meta)

For me either go with (1) drop the shortcut if it's not going to work as a shorcut, or (2) properly support the use-case when you have a bunch of explicitly defined fields on a ModelSerializer

[lovelydinosaur]

This ain't fixed yet

It's not that it "ain't fixed", it's that it's not something I'd want to support.

If there's a field explicitly declared on the serializer, then that's what it'll use. You don't need additional shortcuts if you're explicitly declaring it.

If it's the case that you want a subclass of that serializer where you modify a field to be read only then add the field explicitly on the subclass and mark it read only. Job done.