Consider warning when read_only_fields overlaps explicitly declared fields #10050
Unanswered
zainnadeem786
asked this question in
Ideas & Suggestions
Replies: 1 comment
|
I agree, we probably need something like this: diff --git a/rest_framework/serializers.py b/rest_framework/serializers.py
index adb6a65b..13da0b5b 100644
--- a/rest_framework/serializers.py
+++ b/rest_framework/serializers.py
@@ -1129,6 +1129,13 @@ class ModelSerializer(Serializer):
for field_name in field_names:
# If the field is explicitly declared on the class then use that.
if field_name in declared_fields:
+ try:
+ readonly = extra_kwargs[field_name]['read_only']
+ except KeyError:
+ pass
+ else:
+ if readonly and declared_fields[field_name].read_only != readonly:
+ warnings.warn('Declared field %s.%s is writable, to make it read only set read_only=True in declaration.' % (self.__class__.__name__, field_name))
fields[field_name] = declared_fields[field_name]
continue |
0 replies
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Uh oh!
There was an error while loading. Please reload this page.
Summary
I noticed that
Meta.read_only_fieldsdoes not apply when the same field is explicitly declared on aModelSerializer.For example:
In this case,
roleremains writable because the explicitly declared field takes precedence over the generated-field configuration.I understand that this is intentional behavior and is consistent with the existing DRF documentation:
read_only_fieldsis a shortcut for fields that would otherwise be automatically generated.Possible developer-experience improvement
Would it be useful for DRF to detect this configuration and provide a warning or configuration error?
For example, when a field is both:
Meta.read_only_fieldsDRF could inform the developer that
read_only=Trueneeds to be set directly on the declared field.For example:
This could help prevent configuration mistakes where a developer assumes that
read_only_fieldsis overriding the explicitly declared field.Why a warning may be preferable
I don't think the existing field-resolution behavior necessarily needs to change, since explicitly declared fields being authoritative appears to be an established design choice.
A warning/error could instead make the behavior explicit while preserving backwards compatibility.
I'm interested in hearing whether this is considered useful, or whether the current behavior and documentation are considered sufficiently clear.
All reactions