-
-
Notifications
You must be signed in to change notification settings - Fork 6.9k
Fix read_only + default unique_together validation. #5922
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix read_only + default unique_together validation. #5922
Conversation
@carltongibson Yup, seems to work. 🏆 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yup, looks pretty reasonable to me.
v3.8.2 is now available on PyPI. Thanks all. |
default = field.get_default() | ||
except SkipField: | ||
continue | ||
defaults[field.field_name] = default |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if we are using a field with field_name different from the field in model that is used as source then this is failing and it should be
defaults[field.source] = default
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I somehow missed your comment on this, but you're correct. The code above specifically checks for a compatible field source, but uses the serializer field_name
instead of the source
model field.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @anveshagarwal. I'm currently following up on that PR. That's what prompted my comment. 😄
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For posterity's sake, my reasoning above was slightly wrong. The source
checks are an indicator that we should be operating on field.source
instead of field.field_name
, however the way to determine this is by looking at run_validation
. Before it calls run_validators
(then _read_only_defaults
), it calls to_internal_value
. At this point, the data has been converted to its source structure. So, it makes sense that the read-only defaults would also map to its source structure, instead of the raw field names.
* Add test for read_only + default unique_together validation. * Fix read_only + default validation
In order to validate a
unique_together
relation on aread_only
field, you need to specify adefault
on the field to be used for validation purposes.e.g.:
Where
owner
is part of aunique_together
relation on the model.#5886 unwittingly broke this behaviour:
UniqueTogetherValidator
was not added to the serialiser.default
value used in validation.This PR:
UniqueTogetherValidator
is correctly generated