Skip to content
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
2 changes: 2 additions & 0 deletions rest_framework/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -1496,6 +1496,8 @@ def get_uniqueness_extra_kwargs(self, field_names, declared_fields, extra_kwargs
default = unique_constraint_field.default
elif unique_constraint_field.null:
default = None
elif unique_constraint_field.blank:
default = ''
else:
default = empty
Comment on lines +1500 to 1502
Copy link

Copilot AI Dec 14, 2025

Choose a reason for hiding this comment

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

Consider adding a field type check before setting default to empty string. The Django blank attribute is primarily meaningful for text fields (CharField, TextField). For other field types with blank=True, setting default='' could cause type errors. While this is an edge case (as non-text fields with blank=True typically also have null=True or an explicit default, which are handled earlier in the condition chain), adding a check like isinstance(unique_constraint_field, (models.CharField, models.TextField)) would make the code more defensive and consistent with the pattern in utils/field_mapping.py lines 133-134.

Suggested change
default = ''
else:
default = empty
if isinstance(unique_constraint_field, (models.CharField, models.TextField)):
default = ''
else:
default = empty

Copilot uses AI. Check for mistakes.

Expand Down
66 changes: 66 additions & 0 deletions tests/test_validators.py
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,14 @@ class Meta:
unique_together = ('race_name', 'position')


class BlankUniquenessTogetherModel(models.Model):
race_name = models.CharField(max_length=100, blank=True)
position = models.IntegerField()

class Meta:
unique_together = ('race_name', 'position')

Comment on lines +152 to +158
Copy link

Copilot AI Dec 14, 2025

Choose a reason for hiding this comment

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

Consider adding a docstring to BlankUniquenessTogetherModel similar to NullUniquenessTogetherModel (lines 160-172) to explain the purpose and expected behavior of blank fields in unique_together constraints.

Copilot uses AI. Check for mistakes.

class NullUniquenessTogetherModel(models.Model):
"""
Used to ensure that null values are not included when checking
Expand Down Expand Up @@ -176,6 +184,12 @@ class Meta:
fields = '__all__'


class BlankUniquenessTogetherSerializer(serializers.ModelSerializer):
class Meta:
model = BlankUniquenessTogetherModel
fields = '__all__'


class NullUniquenessTogetherSerializer(serializers.ModelSerializer):
class Meta:
model = NullUniquenessTogetherModel
Expand Down Expand Up @@ -461,6 +475,34 @@ def test_do_not_ignore_validation_for_null_fields(self):
serializer = NullUniquenessTogetherSerializer(data=data)
assert not serializer.is_valid()

def test_validation_for_provided_blank_fields(self):
BlankUniquenessTogetherModel.objects.create(
position=1
)
data = {
'race_name': '',
'position': 1
}
serializer = BlankUniquenessTogetherSerializer(data=data)
assert not serializer.is_valid()

def test_validation_for_missing_blank_fields(self):
BlankUniquenessTogetherModel.objects.create(
position=1
)
data = {
'position': 1
}
serializer = BlankUniquenessTogetherSerializer(data=data)
assert not serializer.is_valid()

def test_ignore_validation_for_missing_blank_fields(self):
data = {
'position': 1
}
serializer = BlankUniquenessTogetherSerializer(data=data)
assert serializer.is_valid(), serializer.errors

Comment on lines +489 to +505
Copy link

Copilot AI Dec 14, 2025

Choose a reason for hiding this comment

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

The test names test_validation_for_missing_blank_fields and test_ignore_validation_for_missing_blank_fields are confusing because both refer to "missing blank fields" but have opposite expectations. Consider renaming to make the distinction clearer, such as test_validation_fails_for_duplicate_blank_defaults and test_validation_passes_when_no_duplicates.

Copilot uses AI. Check for mistakes.
def test_ignore_validation_for_unchanged_fields(self):
"""
If all fields in the unique together constraint are unchanged,
Expand Down Expand Up @@ -589,6 +631,18 @@ class Meta:
]


class UniqueConstraintBlankModel(models.Model):
title = models.CharField(max_length=100, blank=True)
age = models.IntegerField()
tag = models.CharField(max_length=100, blank=True)

class Meta:
constraints = [
# Unique constraint on 2 blank fields
Copy link

Copilot AI Dec 14, 2025

Choose a reason for hiding this comment

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

The comment "Unique constraint on 2 blank fields" is misleading. The constraint is on 'age' and 'tag', but 'age' is an IntegerField without blank=True. Only 'tag' has blank=True. Consider updating the comment to accurately reflect that the constraint includes one required field (age) and one blank field (tag).

Suggested change
# Unique constraint on 2 blank fields
# Unique constraint on one required field (age) and one blank field (tag)

Copilot uses AI. Check for mistakes.
models.UniqueConstraint(name='unique_constraint', fields=('age', 'tag'), condition=~models.Q(models.Q(title='') & models.Q(tag='True')))
Copy link
Contributor

Choose a reason for hiding this comment

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

Don't we have a char length limit here?

]


class UniqueConstraintNullableModel(models.Model):
title = models.CharField(max_length=100)
age = models.IntegerField(null=True)
Expand All @@ -607,6 +661,12 @@ class Meta:
fields = '__all__'


class UniqueConstraintBlankSerializer(serializers.ModelSerializer):
class Meta:
model = UniqueConstraintBlankModel
fields = ('title', 'age', 'tag')


class UniqueConstraintNullableSerializer(serializers.ModelSerializer):
class Meta:
model = UniqueConstraintNullableModel
Expand Down Expand Up @@ -714,6 +774,12 @@ def test_single_field_uniq_validators(self):
ids_in_qs = {frozenset(v.queryset.values_list(flat=True)) for v in validators if hasattr(v, "queryset")}
assert ids_in_qs == {frozenset([1]), frozenset([3])}

def test_blank_uqnique_constraint_fields_are_not_required(self):
Copy link
Contributor

Choose a reason for hiding this comment

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

Typo here

Copy link

Copilot AI Dec 14, 2025

Choose a reason for hiding this comment

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

Typo in test method name: "uqnique" should be "unique".

Suggested change
def test_blank_uqnique_constraint_fields_are_not_required(self):
def test_blank_unique_constraint_fields_are_not_required(self):

Copilot uses AI. Check for mistakes.
serializer = UniqueConstraintBlankSerializer(data={'age': 25})
self.assertTrue(serializer.is_valid(), serializer.errors)
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we split it into multiple assertions?

result = serializer.save()
self.assertIsInstance(result, UniqueConstraintBlankModel)

def test_nullable_unique_constraint_fields_are_not_required(self):
serializer = UniqueConstraintNullableSerializer(data={'title': 'Bob'})
self.assertTrue(serializer.is_valid(), serializer.errors)
Expand Down
Loading