Browse Source

Fixed #35676 -- Made BaseModelForm validate constraints that reference an InlineForeignKeyField.

Co-authored-by: Simon Charette <charette.s@gmail.com>
Clifford Gama 2 weeks ago
parent
commit
0ebea6e5c0

+ 1 - 0
AUTHORS

@@ -236,6 +236,7 @@ answer newbie questions, and generally made Django that much better:
     Chris Wilson <chris+github@qwirx.com>
     Ciaran McCormick <ciaran@ciaranmccormick.com>
     Claude Paroz <claude@2xlibre.net>
+    Clifford Gama <cliffygamy@gmail.com>
     Clint Ecker
     colin@owlfish.com
     Colin Wood <cwood06@gmail.com>

+ 23 - 5
django/forms/models.py

@@ -370,10 +370,12 @@ class BaseModelForm(BaseForm, AltersData):
         # if initial was provided, it should override the values from instance
         if initial is not None:
             object_data.update(initial)
-        # self._validate_unique will be set to True by BaseModelForm.clean().
-        # It is False by default so overriding self.clean() and failing to call
-        # super will stop validate_unique from being called.
+        # self._validate_(unique|constraints) will be set to True by
+        # BaseModelForm.clean(). It is False by default so overriding
+        # self.clean() and failing to call super will stop
+        # validate_(unique|constraints) from being called.
         self._validate_unique = False
+        self._validate_constraints = False
         super().__init__(
             data,
             files,
@@ -436,6 +438,7 @@ class BaseModelForm(BaseForm, AltersData):
 
     def clean(self):
         self._validate_unique = True
+        self._validate_constraints = True
         return self.cleaned_data
 
     def _update_errors(self, errors):
@@ -495,13 +498,17 @@ class BaseModelForm(BaseForm, AltersData):
             self._update_errors(e)
 
         try:
-            self.instance.full_clean(exclude=exclude, validate_unique=False)
+            self.instance.full_clean(
+                exclude=exclude, validate_unique=False, validate_constraints=False
+            )
         except ValidationError as e:
             self._update_errors(e)
 
-        # Validate uniqueness if needed.
+        # Validate uniqueness and constraints if needed.
         if self._validate_unique:
             self.validate_unique()
+        if self._validate_constraints:
+            self.validate_constraints()
 
     def validate_unique(self):
         """
@@ -514,6 +521,17 @@ class BaseModelForm(BaseForm, AltersData):
         except ValidationError as e:
             self._update_errors(e)
 
+    def validate_constraints(self):
+        """
+        Call the instance's validate_constraints() method and update the form's
+        validation errors if any were raised.
+        """
+        exclude = self._get_validation_exclusions()
+        try:
+            self.instance.validate_constraints(exclude=exclude)
+        except ValidationError as e:
+            self._update_errors(e)
+
     def _save_m2m(self):
         """
         Save the many-to-many fields and generic relations for this form.

+ 12 - 8
docs/topics/forms/modelforms.txt

@@ -242,9 +242,13 @@ when calling :meth:`~django.forms.Form.is_valid()` or accessing the
 ``full_clean()``, although you will typically not use the latter method in
 practice.
 
-``Model`` validation (:meth:`Model.full_clean()
-<django.db.models.Model.full_clean()>`) is triggered from within the form
-validation step, right after the form's ``clean()`` method is called.
+``Model`` validation is triggered from within the form validation step right
+after the form's ``clean()`` method is called. First, the model's
+:meth:`~django.db.models.Model.full_clean()` is called with
+``validate_unique=False`` and ``validate_constraints=False``, then the model's
+:meth:`~django.db.models.Model.validate_unique()` and
+:meth:`~django.db.models.Model.validate_constraints()` methods are called in
+order.
 
 .. warning::
 
@@ -267,10 +271,10 @@ attribute that gives its methods access to that specific model instance.
 
 .. warning::
 
-    The ``ModelForm.clean()`` method sets a flag that makes the :ref:`model
+    The ``ModelForm.clean()`` method sets flags that make the :ref:`model
     validation <validating-objects>` step validate the uniqueness of model
     fields that are marked as ``unique``, ``unique_together`` or
-    ``unique_for_date|month|year``.
+    ``unique_for_date|month|year``, as well as constraints.
 
     If you would like to override the ``clean()`` method and maintain this
     validation, you must call the parent class's ``clean()`` method.
@@ -284,9 +288,9 @@ If you have excluded any model fields, validation will not be run on those
 fields. See the :doc:`form validation </ref/forms/validation>` documentation
 for more on how field cleaning and validation work.
 
-The model's ``clean()`` method will be called before any uniqueness checks are
-made. See :ref:`Validating objects <validating-objects>` for more information
-on the model's ``clean()`` hook.
+The model's ``clean()`` method will be called before any uniqueness or
+constraint checks are made. See :ref:`Validating objects <validating-objects>`
+for more information on the model's ``clean()`` hook.
 
 .. _considerations-regarding-model-errormessages:
 

+ 5 - 0
tests/inline_formsets/models.py

@@ -15,6 +15,11 @@ class Child(models.Model):
     school = models.ForeignKey(School, models.CASCADE)
     name = models.CharField(max_length=100)
 
+    class Meta:
+        constraints = [
+            models.UniqueConstraint("mother", "father", name="unique_parents"),
+        ]
+
 
 class Poet(models.Model):
     name = models.CharField(max_length=100)

+ 35 - 0
tests/inline_formsets/tests.py

@@ -215,3 +215,38 @@ class InlineFormsetFactoryTest(TestCase):
         )
         formset = PoemFormSet(None, instance=poet)
         formset.forms  # Trigger form instantiation to run the assert above.
+
+
+class InlineFormsetConstraintsValidationTests(TestCase):
+    def test_constraint_refs_inline_foreignkey_field(self):
+        """
+        Constraints that reference an InlineForeignKeyField should not be
+        skipped from validation (#35676).
+        """
+        ChildFormSet = inlineformset_factory(
+            Parent,
+            Child,
+            fk_name="mother",
+            fields="__all__",
+            extra=1,
+        )
+        father = Parent.objects.create(name="James")
+        school = School.objects.create(name="Hogwarts")
+        mother = Parent.objects.create(name="Lily")
+        Child.objects.create(name="Harry", father=father, mother=mother, school=school)
+        data = {
+            "mothers_children-TOTAL_FORMS": "1",
+            "mothers_children-INITIAL_FORMS": "0",
+            "mothers_children-MIN_NUM_FORMS": "0",
+            "mothers_children-MAX_NUM_FORMS": "1000",
+            "mothers_children-0-id": "",
+            "mothers_children-0-father": str(father.pk),
+            "mothers_children-0-school": str(school.pk),
+            "mothers_children-0-name": "Mary",
+        }
+        formset = ChildFormSet(instance=mother, data=data, queryset=None)
+        self.assertFalse(formset.is_valid())
+        self.assertEqual(
+            formset.errors,
+            [{"__all__": ["Constraint “unique_parents” is violated."]}],
+        )

+ 21 - 0
tests/model_forms/models.py

@@ -521,3 +521,24 @@ class Dice(models.Model):
         through=NumbersToDice,
         limit_choices_to=models.Q(value__gte=1),
     )
+
+
+class ConstraintsModel(models.Model):
+    name = models.CharField(max_length=100)
+    category = models.CharField(max_length=50, default="uncategorized")
+    price = models.DecimalField(max_digits=10, decimal_places=2, default=0)
+
+    class Meta:
+        constraints = [
+            models.UniqueConstraint(
+                "name",
+                "category",
+                name="unique_name_category",
+                violation_error_message="This product already exists.",
+            ),
+            models.CheckConstraint(
+                condition=models.Q(price__gt=0),
+                name="price_gte_zero",
+                violation_error_message="Price must be greater than zero.",
+            ),
+        ]

+ 38 - 0
tests/model_forms/tests.py

@@ -40,6 +40,7 @@ from .models import (
     Character,
     Colour,
     ColourfulItem,
+    ConstraintsModel,
     CustomErrorMessage,
     CustomFF,
     CustomFieldForExclusionModel,
@@ -3718,3 +3719,40 @@ class ModelToDictTests(TestCase):
         # If data were a QuerySet, it would be reevaluated here and give "red"
         # instead of the original value.
         self.assertEqual(data, [blue])
+
+
+class ConstraintValidationTests(TestCase):
+    def test_unique_constraint_refs_excluded_field(self):
+        obj = ConstraintsModel.objects.create(name="product", price="1.00")
+        data = {
+            "id": "",
+            "name": obj.name,
+            "price": "1337.00",
+            "category": obj.category,
+        }
+        ConstraintsModelForm = modelform_factory(ConstraintsModel, fields="__all__")
+        ExcludeCategoryForm = modelform_factory(ConstraintsModel, exclude=["category"])
+        full_form = ConstraintsModelForm(data)
+        exclude_category_form = ExcludeCategoryForm(data)
+        self.assertTrue(exclude_category_form.is_valid())
+        self.assertFalse(full_form.is_valid())
+        self.assertEqual(
+            full_form.errors, {"__all__": ["This product already exists."]}
+        )
+
+    def test_check_constraint_refs_excluded_field(self):
+        data = {
+            "id": "",
+            "name": "priceless",
+            "price": "0.00",
+            "category": "category 1",
+        }
+        ConstraintsModelForm = modelform_factory(ConstraintsModel, fields="__all__")
+        ExcludePriceForm = modelform_factory(ConstraintsModel, exclude=["price"])
+        full_form = ConstraintsModelForm(data)
+        exclude_price_form = ExcludePriceForm(data)
+        self.assertTrue(exclude_price_form.is_valid())
+        self.assertFalse(full_form.is_valid())
+        self.assertEqual(
+            full_form.errors, {"__all__": ["Price must be greater than zero."]}
+        )