#25349: ModelForm regression when setting None into a ForeignKey
--------------------------+------------------------------------
Reporter: jpaulett | Owner:
Type: Bug | Status: new
Component: Forms | Version: 1.8
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
--------------------------+------------------------------------
Changes (by haxoza):
* owner: haxoza =>
* status: assigned => new
* has_patch: 0 => 1
Comment:
As I showed in tests from my previous comment the submitted bug is a
little bit more general. The main thing is that when model field with
`blank=False` is overridden by form field with `required=False` then
saving the model doesn't change the value in model instance nor in
database. This test is added
[https://github.com/django/django/pull/5658/files#diff-
74c67aea7429ebe5e21f206f24b4418bR266 here in my pull request].
I run git bisect tool to find which commit introduced this more general
bug. Tim was right on his first guess that commit
45e049937d2564d11035827ca9a9221b86215e45 broke the model forms behaviour
(#13776).
After further digging I came into the fact that in `_post_clean()` method
on `BaseModelForm` class `exclude` parameter in the following line:
{{{
self.instance = construct_instance(self, self.instance, opts.fields,
exclude)
}}}
has to be changed to `opts.exclude`. What's more the instance has to be
constructed once exclusions for further validation are prepared (because
`_get_validation_exclusions()` methods looks into `self._errors`). Here's
[https://github.com/django/django/pull/5658/files#diff-
70af885c2725fe87eb3b99a393268d10L379 the change I made].
The solution described above fixes the bug I found but it still doesn't
fix it for the bug originally described by jpaulett. The problem here is
that the None value cannot be assigned to foreign key field in a process
of model instance construction.
Long time ago Jacob
[https://github.com/haxoza/django/commit/1452d46240609ff39dacf9ea2f759ed600c2f09f
#diff-3010fc5a498b7171c342520f34507968R192 introduced a check for foreign
keys] which prevents `None` value assignment if the field is `null=False`.
In that time this decision was probably right.
However currently there are a few points against this check:
* Conceptually it is a programmer's responsibility to validate the data
assigned to foreign key in a right moment - not necessarily during
assignment to the model field.
* This behaviour is not symmetrical to other database-related constraints
as for example `unique=True` which is deferred to `save()` method (raising
IntegrityError) or `full_clean()` method (raising ValidationError).
* In #13776 the decision was made that None value for foreign key field
with `null=False` and corresponding form field with `required=False`
should be valid. It means that after model instantiation by such form the
value of given field is empty or just unset (not defined). Whatever the
state is, it doesn't matter. It shows only that currently discussed check
does **not** prevent in 100% against having foreign key field with
`null=False` set to `None`. It undermines the legitimacy of presence for
the discussed check.
As pointed above it has much more logical sense to defer validation of
non-nullable foreign key field to `save()` method and `full_clean()`
method. What's interesting deletion of the code introduced by Jacob
results in expected behaviour as highlighted in
[https://github.com/django/django/pull/5658/files#diff-
e4aa8dc49d7522ae55a932ffc7d09c1cR250 the tests I've proposed].
Having fixed foreign key field behaviour the solution for the submitted
issue is in place out of the box.
--
Ticket URL: <https://code.djangoproject.com/ticket/25349#comment:6>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
--
You received this message because you are subscribed to the Google Groups
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
To post to this group, send email to [email protected].
To view this discussion on the web visit
https://groups.google.com/d/msgid/django-updates/066.776c65ac60284e128e3e38ad2cef5e45%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.
