fix(blank/unique): add `default` handling for `blank=True` field #9751

sshishov · 2025-08-02T12:48:20Z

Description

This MR should fix the issue with blank=True field in UniqueTogether constraint. It should not become required all of a sudden.

Fixes #9750

Signed-off-by: Sergei Shishov <[email protected]>

sshishov · 2025-08-02T12:58:25Z

I have added tests similar to the tests added in #9531

auvipy

did you check this pr #9725 ?

deepakangadi · 2025-08-05T00:59:02Z

tests/test_validators.py

+    class Meta:
+        constraints = [
+            # Unique constraint on 2 blank fields
+            models.UniqueConstraint(name='unique_constraint', fields=('age', 'tag'), condition=~models.Q(models.Q(title='') & models.Q(tag='True')))


Don't we have a char length limit here?

deepakangadi · 2025-08-05T00:59:28Z

tests/test_validators.py


+    def test_blank_uqnique_constraint_fields_are_not_required(self):
+        serializer = UniqueConstraintBlankSerializer(data={'age': 25})
+        self.assertTrue(serializer.is_valid(), serializer.errors)


Can we split it into multiple assertions?

deepakangadi · 2025-08-05T00:59:33Z

tests/test_validators.py

        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):


mahdighadiriii · 2025-09-27T14:41:57Z

Hi @sshishov @auvipy @deepakangadi,
I'm excited to contribute to DRF as my first contribution! I noticed this PR (#9751) addresses a critical issue (#9750) with blank=True fields in UniqueConstraints, which aligns with my experience in Django/DRF serializers and API optimization.

I can help unblock this by:

Fixing the typo in test_blank_uqnique_constraint_fields_are_not_required (uqnique → unique).
Splitting assertions in the test for clarity, as suggested by @deepakangadi.
Adding char length limit (e.g., max_length=255) to the constraint.
Resolving conflicts by rebasing on main.
Checking for overlap with PR docs: unique constraints cause 'required=True' #9725, as mentioned by @auvipy.

If this sounds good, I can start working on these changes. Should I update this branch (if permitted) or create a new PR? Thanks for your guidance!

stale · 2025-10-18T07:30:46Z

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

Copilot

Pull request overview

This PR fixes an issue where blank=True fields in unique_together constraints were incorrectly becoming required. The fix adds special handling for blank fields by setting their default value to an empty string, similar to how null=True fields default to None. This ensures that blank fields in uniqueness constraints remain optional while still properly validating uniqueness.

Key Changes

Added default='' handling for blank fields in unique constraints in get_uniqueness_extra_kwargs
Added comprehensive test coverage for both unique_together and UniqueConstraint with blank fields
Tests verify that blank fields are not required, while still enforcing uniqueness validation

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.

File	Description
rest_framework/serializers.py	Added conditional logic to set `default=''` for blank fields in the uniqueness constraint handling, following the same pattern as null field handling
tests/test_validators.py	Added new test models (`BlankUniquenessTogetherModel`, `UniqueConstraintBlankModel`) and test cases to verify blank fields in uniqueness constraints are not required but still validate correctly

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-12-14T13:51:46Z

tests/test_validators.py

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


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 · 2025-12-14T13:51:46Z

tests/test_validators.py

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


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 · 2025-12-14T13:51:47Z

rest_framework/serializers.py

+                default = ''
            else:
                default = empty


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 · 2025-12-14T13:51:47Z

tests/test_validators.py

        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):


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 · 2025-12-14T13:51:47Z

tests/test_validators.py

+    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
+


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.

auvipy

please also fix the merge conflicts

fix(blank/unique): add default handling for blank=True field

8de6a6c

Signed-off-by: Sergei Shishov <[email protected]>

sshishov force-pushed the ss/fix/allow-blank-unique-constraint branch from 3bcd191 to 8de6a6c Compare August 2, 2025 12:56

auvipy reviewed Aug 2, 2025

View reviewed changes

deepakangadi suggested changes Aug 5, 2025

View reviewed changes

stale bot added the stale label Oct 18, 2025

browniebroke removed the stale label Oct 20, 2025

auvipy requested review from auvipy and Copilot December 14, 2025 13:46

Copilot started reviewing on behalf of auvipy December 14, 2025 13:47 View session

Copilot AI reviewed Dec 14, 2025

View reviewed changes

auvipy requested changes Dec 14, 2025

View reviewed changes

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

	def test_blank_uqnique_constraint_fields_are_not_required(self):
	def test_blank_unique_constraint_fields_are_not_required(self):

Uh oh!

fix(blank/unique): add default handling for blank=True field #9751

Are you sure you want to change the base?

fix(blank/unique): add default handling for blank=True field #9751

Conversation

sshishov commented Aug 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Uh oh!

sshishov commented Aug 2, 2025

Uh oh!

auvipy left a comment

Choose a reason for hiding this comment

Uh oh!

deepakangadi Aug 5, 2025

Choose a reason for hiding this comment

Uh oh!

deepakangadi Aug 5, 2025

Choose a reason for hiding this comment

Uh oh!

deepakangadi Aug 5, 2025

Choose a reason for hiding this comment

Uh oh!

mahdighadiriii commented Sep 27, 2025

Uh oh!

stale bot commented Oct 18, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Key Changes

Reviewed changes

Uh oh!

Copilot AI Dec 14, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 14, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 14, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 14, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 14, 2025

Choose a reason for hiding this comment

Uh oh!

auvipy left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

fix(blank/unique): add `default` handling for `blank=True` field #9751

fix(blank/unique): add `default` handling for `blank=True` field #9751

sshishov commented Aug 2, 2025 •

edited

Loading