Skip to content
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 unique_together validation with source #9482

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 7 additions & 6 deletions rest_framework/validators.py
Original file line number Diff line number Diff line change
Expand Up @@ -159,17 +159,18 @@ def __call__(self, attrs, serializer):
queryset = self.filter_queryset(attrs, queryset, serializer)
queryset = self.exclude_current_instance(attrs, queryset, serializer.instance)

checked_names = [
serializer.fields[field_name].source for field_name in self.fields
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since fields is a dict it is more readable to iterate over values, also it should be a little faster.

Suggested change
serializer.fields[field_name].source for field_name in self.fields
field.source for field in self.fields.values()

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @sevdog thanks for the comment, but I believe self.fields is a list of strings, not dict.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, sorry I was considering to be in an other class/file.

]
# Ignore validation if any field is None
if serializer.instance is None:
checked_values = [
value for field, value in attrs.items() if field in self.fields
]
checked_values = [attrs[field_name] for field_name in checked_names]
else:
# Ignore validation if all field values are unchanged
checked_values = [
value
for field, value in attrs.items()
if field in self.fields and value != getattr(serializer.instance, field)
attrs[field_name]
for field_name in checked_names
if attrs[field_name] != getattr(serializer.instance, field_name)
]

if checked_values and None not in checked_values and qs_exists(queryset):
Expand Down
22 changes: 22 additions & 0 deletions tests/test_validators.py
Original file line number Diff line number Diff line change
Expand Up @@ -469,6 +469,28 @@ def test_ignore_validation_for_unchanged_fields(self):
assert serializer.is_valid()
assert not mock.called

@patch("rest_framework.validators.qs_exists")
def test_unique_together_with_source(self, mock_qs_exists):
class UniqueTogetherWithSourceSerializer(serializers.ModelSerializer):
name = serializers.CharField(source="race_name")
pos = serializers.IntegerField(source="position")

class Meta:
model = UniquenessTogetherModel
fields = ["name", "pos"]

data = {"name": "Paris Marathon", "pos": 1}
instance = UniquenessTogetherModel.objects.create(
race_name="Paris Marathon", position=1
)
serializer = UniqueTogetherWithSourceSerializer(data=data)
assert not serializer.is_valid()
assert mock_qs_exists.called
mock_qs_exists.reset_mock()
serializer = UniqueTogetherWithSourceSerializer(data=data, instance=instance)
assert serializer.is_valid()
assert not mock_qs_exists.called

def test_filter_queryset_do_not_skip_existing_attribute(self):
"""
filter_queryset should add value from existing instance attribute
Expand Down
Loading