[Django] #35367: Multi-field constraint are not correctly handled when a form has one field marked as read-only

[Django]

#35367: Multi-field constraint are not correctly handled when a form has one field
marked as read-only
--------------------------------------+------------------------
Reporter: David | Owner: nobody
Type: Bug | Status: new
Component: Forms | Version: 4.2
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
--------------------------------------+------------------------
When a model defines a constraint which relates two or more fields and one
of those field is excluded from the related model form the validation
provide a false positive and then the form tries to store an invalid
record on the database, raising an `IntegrityError`.

Here is a code sample which reflects the situation

{{{#!python
# models.py
from django.db import models
from django.forms import modelform_factory


class MyModel(models.Model):
field_1 = models.IntegerField(null=True)
field_2 = models.IntegerField()

class Meta:
constraints = [
models.CheckConstraint(
name="mycheck",
check=models.Q(field_1__lt=models.F("field_2")) |
models.Q(field_1__isnull=True),
)
]


# this is like it would be done in admin if 'field_1' is set a readonly
Form = modelform_factory(MyModel, fields=['field_2'], exclude=['field_1'])
# this is ok for validation: 10 < 20
instance = MyModel(field_1=10, field_2=20)
# this does not respect the constraint: 10 > 5
form_instance = Form({'field_2': 5}, instance=instance)
form.is_valid()
# > True
form.save()
# > IntegrityError
}}}


This is somehow similar and related to #12627
--
Ticket URL: <https://code.djangoproject.com/ticket/35367>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

[Django]

#35367: Multi-field constraint are not correctly handled when a form has one field
marked as read-only

------------------------+--------------------------------------
Reporter: David | Owner: nobody
Type: Bug | Status: closed
Component: Forms | Version: 4.2
Severity: Normal | Resolution: wontfix

Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0

------------------------+--------------------------------------
Changes (by David Sanders):

* resolution: => wontfix
* status: new => closed

Comment:

Form & model validation are separate concerns though & I'd argue this is
desired behaviour. In fact the code explicitly leaves out the constraint
validation because it includes field_1.

If you want the constraint to be respected then you'd need to add an
additional line to validate the model instance in its entirety:

{{{
if form.is_valid():
instance = form.save(commit=False)
instance.full_clean() # full_clean() within form.is_valid() excludes
field_1
instance.save()
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/35367#comment:1>

[Django]

#35367: Multi-field constraint are not correctly handled when a form has one field
marked as read-only
------------------------+--------------------------------------
Reporter: David | Owner: nobody
Type: Bug | Status: closed
Component: Forms | Version: 4.2
Severity: Normal | Resolution: wontfix
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
------------------------+--------------------------------------

Comment (by Sarah Boyce):

Hi David, to reiterate what David Sanders said, this is expected behaviour
and documented. See:
https://docs.djangoproject.com/en/5.0/ref/models/instances/#validating-
objects
In particular:
> When you use a ModelForm, the call to is_valid() will perform these
validation steps for all the fields that are included on the form. See the
ModelForm documentation for more information. 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.

You need to call full_clean() as you have excluded a field required for
validation purposes:
https://docs.djangoproject.com/en/5.0/ref/models/instances/#django.db.models.Model.validate_constraints
--
Ticket URL: <https://code.djangoproject.com/ticket/35367#comment:2>

[Django]

#35367: Multi-field constraint are not correctly handled when a form has one field
marked as read-only
------------------------+--------------------------------------
Reporter: David | Owner: nobody
Type: Bug | Status: closed
Component: Forms | Version: 4.2
Severity: Normal | Resolution: wontfix
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
------------------------+--------------------------------------

Comment (by David):

I undestand that this may be a design choice, the part which worries me is
that when the form creation is delegated to `ModelAdmin` this behaviour is
masked.

I came across this problem with an admin like:

{{{#!python
@admin.register(MyModel)
class MyModelAdmin(admin.ModelAdmin):
readonly_fields = ('field_1',)
}}}

My expectation was to get warned before changing the other field that the
constraint was going to be violated rather than getting an error.

Currently there is no warning in the admin documentation which refers to
possible problems which may be caused by read only fields and data
validation performed by the ORM:
https://docs.djangoproject.com/en/5.0/ref/contrib/admin/#django.contrib.admin.ModelAdmin.readonly_fields
--
Ticket URL: <https://code.djangoproject.com/ticket/35367#comment:3>

[Django]

#35367: Multi-field constraint are not correctly handled when a form has one field
marked as read-only
------------------------+--------------------------------------
Reporter: David | Owner: nobody
Type: Bug | Status: closed
Component: Forms | Version: 4.2
Severity: Normal | Resolution: wontfix
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
------------------------+--------------------------------------
Comment (by David):

Maybe an other way to express what I belive is: if there is any constraint
which involves more than one field it is not correct to allow excluding a
subset of those fields from validation while keeping the others in the
validation workflow.

With the provided code sample the validation of `field_1` **cannot** be
done without `field_2` (and vice-versa).
--
Ticket URL: <https://code.djangoproject.com/ticket/35367#comment:4>

[Django]

#35367: Multi-field constraint are not correctly handled when a form has one field
marked as read-only
------------------------+--------------------------------------
Reporter: David | Owner: nobody
Type: Bug | Status: closed
Component: Forms | Version: 4.2
Severity: Normal | Resolution: wontfix
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
------------------------+--------------------------------------

Comment (by Sarah Boyce):

I think #12627 covers your previous comment about `readonly_fields` in the
admin.

Replying to [comment:4 David]:

> Maybe an other way to express what I belive is: if there is any
constraint which involves more than one field it is not correct to allow
excluding a subset of those fields from validation while keeping the
others in the validation workflow.
>
> With the provided code sample the validation of `field_1` **cannot** be
done without `field_2` (and vice-versa).

If you want to discuss changing the design, you should propose this in the
[https://forum.djangoproject.com/c/internals/5 Django forum] and see if
you can get agreement from the community for this change. As this would
impact a lot of existing projects, we need strong community agreement
after which we can accept a ticket 🙂
--
Ticket URL: <https://code.djangoproject.com/ticket/35367#comment:5>