-
-
Notifications
You must be signed in to change notification settings - Fork 7k
UniqueTogheterValidator fails with partial unique indexes with ForeignKey #9779
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -4,6 +4,7 @@ | |||||
|
|
||||||
| import pytest | ||||||
| from django import VERSION as django_version | ||||||
| from django.core.exceptions import ValidationError as DjangoValidationError | ||||||
| from django.db import DataError, models | ||||||
| from django.test import TestCase | ||||||
|
|
||||||
|
|
@@ -779,6 +780,75 @@ class Meta: | |||||
| assert serializer.is_valid() | ||||||
|
|
||||||
|
|
||||||
| class UniqueConstraintWithRelationModel(models.Model): | ||||||
| value = models.IntegerField() | ||||||
| first_related = models.ForeignKey(IntegerFieldModel, related_name='+', on_delete=models.CASCADE, null=True, blank=True) | ||||||
| second_related = models.ForeignKey(IntegerFieldModel, related_name='+', on_delete=models.CASCADE, null=True, blank=True) | ||||||
|
|
||||||
| class Meta: | ||||||
| constraints = [ | ||||||
| models.UniqueConstraint(fields=('value', 'first_related'), condition=models.Q(second_related__isnull=True), name='unique_constraint_with_relationa'), | ||||||
| models.UniqueConstraint(fields=('value', 'second_related'), condition=models.Q(first_related__isnull=True), name='unique_constraint_with_relationb'), | ||||||
|
||||||
| ] | ||||||
|
|
||||||
|
|
||||||
| class UniqueConstraintWithRelationModelSerializer(serializers.ModelSerializer): | ||||||
| class Meta: | ||||||
| model = UniqueConstraintWithRelationModel | ||||||
| fields = serializers.ALL_FIELDS | ||||||
|
||||||
| fields = serializers.ALL_FIELDS | |
| fields = '__all__' |
Copilot
AI
Dec 14, 2025
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.
The test suite is missing a positive test case to verify that the unique constraint validation passes when the constraint conditions are not violated. Consider adding a test to verify that creating a new instance with different values (e.g., value=2) or when the condition doesn't apply (e.g., both first_related and second_related are set) is allowed and passes validation.
Copilot
AI
Dec 14, 2025
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.
The test suite is missing a test case for updating an existing instance. Following the pattern established in other validator tests (e.g., test_updated_instance_excluded_from_unique_together at line 239), there should be a test to verify that when updating an instance with the same values, it doesn't fail validation against itself.
Copilot
AI
Dec 14, 2025
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.
The test should also verify the case where both first_related and second_related are non-null. Based on the constraint conditions (second_related__isnull=True and first_related__isnull=True), when both fields have values, neither constraint should apply and the validation should pass. This is an important edge case that demonstrates the constraint conditions are being properly evaluated.
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.
The constraint name 'unique_constraint_with_relationa' appears to be truncated or missing an underscore. Based on Django naming conventions and the pattern in line 791 ('unique_constraint_with_relationb'), this should likely be 'unique_constraint_with_relation_a' or extend to something more descriptive like 'unique_constraint_with_relation_first'.