From 0ebea6e5c07485a36862e9b6e2be18d1694ad2c5 Mon Sep 17 00:00:00 2001 From: Clifford Gama Date: Sat, 8 Mar 2025 15:46:58 +0200 Subject: [PATCH] Fixed #35676 -- Made BaseModelForm validate constraints that reference an InlineForeignKeyField. Co-authored-by: Simon Charette --- AUTHORS | 1 + django/forms/models.py | 28 ++++++++++++++++++----- docs/topics/forms/modelforms.txt | 20 ++++++++++------- tests/inline_formsets/models.py | 5 +++++ tests/inline_formsets/tests.py | 35 +++++++++++++++++++++++++++++ tests/model_forms/models.py | 21 ++++++++++++++++++ tests/model_forms/tests.py | 38 ++++++++++++++++++++++++++++++++ 7 files changed, 135 insertions(+), 13 deletions(-) diff --git a/AUTHORS b/AUTHORS index 9ac0d50bd9..5ddebb69fb 100644 --- a/AUTHORS +++ b/AUTHORS @@ -236,6 +236,7 @@ answer newbie questions, and generally made Django that much better: Chris Wilson Ciaran McCormick Claude Paroz + Clifford Gama Clint Ecker colin@owlfish.com Colin Wood diff --git a/django/forms/models.py b/django/forms/models.py index d220e3c90f..9acd2d7280 100644 --- a/django/forms/models.py +++ b/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. diff --git a/docs/topics/forms/modelforms.txt b/docs/topics/forms/modelforms.txt index e2a37296b7..ef916d882d 100644 --- a/docs/topics/forms/modelforms.txt +++ b/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() -`) 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 ` 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 ` 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 ` 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 ` +for more information on the model's ``clean()`` hook. .. _considerations-regarding-model-errormessages: diff --git a/tests/inline_formsets/models.py b/tests/inline_formsets/models.py index f4c06e8fac..a090387c42 100644 --- a/tests/inline_formsets/models.py +++ b/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) diff --git a/tests/inline_formsets/tests.py b/tests/inline_formsets/tests.py index 1ae9b3f760..0fe9766dc6 100644 --- a/tests/inline_formsets/tests.py +++ b/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."]}], + ) diff --git a/tests/model_forms/models.py b/tests/model_forms/models.py index f9441a4c77..4111fafd69 100644 --- a/tests/model_forms/models.py +++ b/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.", + ), + ] diff --git a/tests/model_forms/tests.py b/tests/model_forms/tests.py index a432f6ce43..837466f07f 100644 --- a/tests/model_forms/tests.py +++ b/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."]} + )