Opened 4 weeks ago
Closed 4 weeks 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 , 4 weeks ago
| Attachment: | ticket-36668-tests.diff added |
|---|
comment:1 by , 4 weeks ago
comment:2 by , 4 weeks 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_togetherhas 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.