Skip to content

Fix serializer method field hidden unique together (#9700) #9710

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
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
12 changes: 10 additions & 2 deletions rest_framework/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -1483,6 +1483,12 @@ def get_uniqueness_extra_kwargs(self, field_names, declared_fields, extra_kwargs
hidden_fields = {}
uniqueness_extra_kwargs = {}

# Identify declared SerializerMethodFields to prevent them from becoming HiddenFields
serializer_method_field_names = {
name for name, field_instance in declared_fields.items()
if isinstance(field_instance, SerializerMethodField)
}

for unique_constraint_name in unique_constraint_names:
# Get the model field that is referred too.
unique_constraint_field = model._meta.get_field(unique_constraint_name)
Expand All @@ -1507,8 +1513,10 @@ def get_uniqueness_extra_kwargs(self, field_names, declared_fields, extra_kwargs
elif default is not empty:
# The corresponding field is not present in the
# serializer. We have a default to use for it, so
# add in a hidden field that populates it.
hidden_fields[unique_constraint_name] = HiddenField(default=default)
# add in a hidden field that populates it,
# unless it's a SerializerMethodField.
if unique_constraint_name not in serializer_method_field_names:
hidden_fields[unique_constraint_name] = HiddenField(default=default)

# Update `extra_kwargs` with any new options.
for key, value in uniqueness_extra_kwargs.items():
Expand Down
90 changes: 90 additions & 0 deletions tests/test_model_serializer.py
Copy link
Member

Choose a reason for hiding this comment

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

Please tidy the diff in this file. It's clearly AI generated file (even the commit is not hiding that fact), with extra verbose comments. It should follow the style of the surrounding code

Original file line number Diff line number Diff line change
Expand Up @@ -1346,6 +1346,31 @@ class Meta:
fields = ('name',)


class UniqueTestModel(models.Model):
name = models.CharField(max_length=100)
description = models.CharField(max_length=100, null=True, blank=True)
other_field = models.CharField(max_length=100, default="default_value")

class Meta:
unique_together = [("name", "description")]
Copy link
Member

Choose a reason for hiding this comment

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

why not using unique constraint here?


def __str__(self):
return f"{self.name} - {self.description or 'No description'}"


class UniqueTestModelSerializer(serializers.ModelSerializer):
description = serializers.SerializerMethodField()

class Meta:
model = UniqueTestModel
fields = ["name", "description", "other_field"]

def get_description(self, obj):
if obj.description:
return f"Serialized: {obj.description}"
return "Serialized: No description provided"


class Issue6110Test(TestCase):
def test_model_serializer_custom_manager(self):
instance = Issue6110ModelSerializer().create({'name': 'test_name'})
Expand Down Expand Up @@ -1395,3 +1420,68 @@ class Meta:
serializer.save()

self.assertEqual(instance.char_field, 'value changed by signal')


class TestSerializerMethodFieldInUniqueTogether(TestCase):
def test_serializer_method_field_not_hidden_in_unique_together(self):
"""
Tests that a SerializerMethodField named the same as a model field
in a unique_together constraint is not treated as a HiddenField and
that unique_together validation still functions correctly.
"""
serializer = UniqueTestModelSerializer()

self.assertFalse(
isinstance(serializer.fields['description'], serializers.HiddenField),
"Field 'description' should not be a HiddenField."
)
self.assertTrue(
isinstance(serializer.fields['description'], serializers.SerializerMethodField),
"Field 'description' should be a SerializerMethodField."
)

instance = UniqueTestModel.objects.create(name="TestName", description="TestDesc")
serializer_output = UniqueTestModelSerializer(instance).data
self.assertIn("description", serializer_output)
self.assertEqual(serializer_output["description"], "Serialized: TestDesc")

instance_no_desc = UniqueTestModel.objects.create(name="TestNameNoDesc")
serializer_output_no_desc = UniqueTestModelSerializer(instance_no_desc).data
self.assertEqual(serializer_output_no_desc["description"], "Serialized: No description provided")

UniqueTestModel.objects.create(name="UniqueName", description="UniqueDesc")
invalid_data = {"name": "UniqueName", "description": "UniqueDesc", "other_field": "some_value"}
serializer_invalid = UniqueTestModelSerializer(data=invalid_data)
with self.assertRaises(serializers.ValidationError) as context:
serializer_invalid.is_valid(raise_exception=True)
self.assertIn("non_field_errors", context.exception.detail)
self.assertTrue(any("unique test model with this name and description already exists" in str(err)
for err_list in context.exception.detail.values() for err in err_list))

UniqueTestModel.objects.create(name="UniqueNameNull", description=None)
invalid_data_null = {"name": "UniqueNameNull", "description": None, "other_field": "some_value"}
serializer_invalid_null = UniqueTestModelSerializer(data=invalid_data_null)
with self.assertRaises(serializers.ValidationError) as context_null:
serializer_invalid_null.is_valid(raise_exception=True)
self.assertIn("non_field_errors", context_null.exception.detail)
self.assertTrue(any("unique test model with this name and description already exists" in str(err)
for err_list in context_null.exception.detail.values() for err in err_list))

valid_data = {"name": "NewName", "description": "NewDesc", "other_field": "another_value"}
serializer_valid = UniqueTestModelSerializer(data=valid_data)
self.assertTrue(serializer_valid.is_valid(raise_exception=True))
self.assertEqual(serializer_valid.validated_data['name'], "NewName")

valid_data_no_desc = {"name": "NameOnly"}
serializer_valid_no_desc = UniqueTestModelSerializer(data=valid_data_no_desc)
self.assertTrue(serializer_valid_no_desc.is_valid(raise_exception=True))
self.assertIsNone(serializer_valid_no_desc.validated_data.get('description'))
self.assertEqual(serializer_valid_no_desc.validated_data['other_field'], "default_value")

saved_instance = serializer_valid_no_desc.save()
self.assertEqual(saved_instance.name, "NameOnly")
self.assertIsNone(saved_instance.description)
self.assertEqual(saved_instance.other_field, "default_value")

output_after_save = UniqueTestModelSerializer(saved_instance).data
self.assertEqual(output_after_save['description'], "Serialized: No description provided")
Loading