Opened 7 hours ago
Closed 4 hours ago
#36668 closed Bug (invalid)
ModelForm validation skips UniqueConstraint if its condition refers to a field not present on the form
Reported by: | Baptiste Mispelon | Owned by: | |
---|---|---|---|
Component: | Database layer (models, ORM) | Version: | 5.2 |
Severity: | Normal | Keywords: | modelform, constraint, uniqueconstraint |
Cc: | Triage Stage: | Unreviewed | |
Has patch: | no | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
(sorry for this mouthful of a title π)
For starters, I'm not sure if this is a bug or a feature request but it feels like a bug to me so I went with that. Similarly, I'm unsure if this is a ModelForm
issue or a UniqueConstraint
one and went with the latter.
Now for the actual issue, consider this code (see attached patch for an actual testcase [1] ):
from django import forms from django.db import models class Page(models.Model): pass class Revision(models.Model): page = models.ForeignKey(Page, on_delete=models.CASCADE) status = models.IntegerField(default=1) class Meta: constraints = [ models.UniqueConstraint( name="unique_page_status_1", fields=["page"], condition=models.Q(status=1), ) ] class RevisionForm(forms.ModelForm): class Meta: model = Revision fields = ["page"] page = Page.objects.create() page.revision_set.create() form = RevisionForm(data={"page": page.pk}) # I would explain the form to be invalid because the revision that would be # created by saving the form would fail against the constraint. # But that's not what happens: assert not form.is_valid() # fails
(I've tested as far back as 4.1 and they all exhibit the same issue.)
I've investigated a little and here's what I think is happening: the ModelForm
ends up calling UniqueConstraint.validate()
and passing it an exclude
list that contains status
(because status
is a field on the model, but not on the form). In turn, the logic in UniqueConstraint.validate()
notices that status
is present in its condition
, and so decides to skip that validation.
I think that last step where UniqueConstraint.validate()
skips fields used in the constraint's condition
if they're listed in exclude
is a bug. I think this behavior makes sense for CheckConstraint
, but not for UniqueConstraint
. In my mind UniqueConstraint.validate()
should only check self.fields
and ignore self.condition
.
[1] Once the patch is applied with git apply ticket-xxx.diff
you can run it with uv run --with-editable=. --with-requirements=tests/requirements/py3.txt python tests/runtests.py aaaaaa
Attachments (1)
Change History (3)
by , 6 hours ago
Attachment: | ticket-36668-tests.diff added |
---|
comment:1 by , 5 hours ago
comment:2 by , 4 hours ago
Resolution: | β invalid |
---|---|
Status: | new β closed |
Happy to look at a documentation improvement here -- the closest thing I could find was (emphasis added):
You should only need to call a modelβs full_clean() method if you plan to handle validation errors yourself, or if you have excluded fields from the ModelForm that require validation.
This was discussed on the forum extensively so βI'll drop a link here.
The TL;DR is by design as
unique_together
has always worked this way (if a unique constraint refers to any excluded fields then it's not validated) so we followed the same approach forMeta.constraints
.