Force "required" fields to be included in a ModelForm

48 views
Skip to first unread message

Will Gordon

unread,
Apr 16, 2019, 8:34:12 PM4/16/19
to Django developers (Contributions to Django itself)
In the same way that editable fields are forced to not be included in a ModelForm (https://github.com/django/django/blob/master/django/forms/models.py#L146), I would like to propose that "required" fields (`blank=False`) be forced to be included in a ModelForm.

While I understand that a developer can force this inclusion themselves, but on a large project, it should not be necessary to always ensure that a Model and ModelForm are in sync.

Since this is probably a non-trivial patch (https://docs.djangoproject.com/en/dev/internals/contributing/writing-code/submitting-patches/#non-trivial-patches) I need to provide evidence that this has been discussed. As such, I'm open to any and all opinions!

James Bennett

unread,
Apr 16, 2019, 8:44:32 PM4/16/19
to django-d...@googlegroups.com
As the documentation points out, ModelForm avoids implicitly adding fields to a form when you haven't told it to, and does so for security reasons. Mass-assignment bugs have caused significant security issues in the past for other systems which *did* implicitly support/add fields, and I'd like to keep Django from doing that.

So if the proposal is just that any field with required=True is implicitly included, I'd be against it. Similarly, I think it would complicate the API too much to add support for some kind of "__all_required__" special declaration.

I would suggest there's value in figuring out a way to either have ModelForm raise some type of ignorable warning, or maybe having the system-check framework warn you, if you do have a ModelForm that doesn't include a required field, though.

Will Gordon

unread,
Apr 16, 2019, 8:56:07 PM4/16/19
to Django developers (Contributions to Django itself)
Sorry if I wasn't more clear, but using a warning was exactly what I was proposing. In the same way that a FieldError is raised when an editable field is listed in fields, I was essentially planning on doing the exact same check on the blank attribute. I agree that this should be ignorable, and I'm not sure that I've settled on a way to do that as of yet.

Tobias McNulty

unread,
Apr 16, 2019, 9:02:29 PM4/16/19
to django-developers
There are plenty of use cases when this behavior wouldn't be desired, such as editing the same model object with 2+ different model forms. Such a pattern might be more common when editing an existing object, but isn't out of the question for creating new objects, either (think of a multi-step form wizard).

Cheers,
Tobias

--
You received this message because you are subscribed to the Google Groups "Django developers (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email to django-develop...@googlegroups.com.
To post to this group, send email to django-d...@googlegroups.com.
Visit this group at https://groups.google.com/group/django-developers.
To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/980d8112-d51c-4e88-a17f-5e9a4a3577d2%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Will Gordon

unread,
Apr 16, 2019, 9:09:06 PM4/16/19
to Django developers (Contributions to Django itself)
I can certainly agree that there may be some use cases where it should be possible to disable this functionality, but I would argue that erroring on a missing, required field should be the default, and allow for a way to override...as opposed to allowing missing fields and requiring a workaround to ensure they're not missing.

Will Gordon

unread,
Apr 16, 2019, 9:14:16 PM4/16/19
to Django developers (Contributions to Django itself)
Would it be weird to just make it so that the "required" field must be present in exclude? This way, if you accidentally leave off a required field, you're quickly notified about it....but if you explicitly mark it as something to exclude, it makes it clear to everyone exactly what you're trying to do? The whole "explicit is better than implicit" and what not.

James Bennett

unread,
Apr 16, 2019, 9:16:55 PM4/16/19
to django-d...@googlegroups.com
Python has a warning system, and Django already uses it for things like deprecation notices.

I don't like error by default as an approach to this, but a warning (which is easy to ignore -- it doesn't break your code) would be fine.

Matthias Kestenholz

unread,
Apr 17, 2019, 2:34:03 AM4/17/19
to django-d...@googlegroups.com
Please don't do this.

There are very good reasons for NOT including blank=False fields by default such as batch-editing some field with a formset or inlineformset or just offering different forms to users with different access levels (as Tobias wrote).

Django started recommending using `fields` instead of `exclude` with good reasons. Why push people towards `exclude` again now? I think we should instead trust developers who specify `fields` that those actually are the fields they want in the form. Using `fields` *is* explicit already.

If it is hard to put this behavior into a base form class in your own project and use this base class everywhere then that would be a good reason to consider changing a thing or two in Django.

Thanks,
m.

On Wed, Apr 17, 2019 at 3:14 AM Will Gordon <wpg...@gmail.com> wrote:
Would it be weird to just make it so that the "required" field must be present in exclude? This way, if you accidentally leave off a required field, you're quickly notified about it....but if you explicitly mark it as something to exclude, it makes it clear to everyone exactly what you're trying to do? The whole "explicit is better than implicit" and what not.

--
You received this message because you are subscribed to the Google Groups "Django developers (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email to django-develop...@googlegroups.com.
To post to this group, send email to django-d...@googlegroups.com.
Visit this group at https://groups.google.com/group/django-developers.

For more options, visit https://groups.google.com/d/optout.


--
www.feinheit.chbera...@feinheit.ch — +41 555 11 11 41

Harro

unread,
Apr 17, 2019, 2:45:03 AM4/17/19
to Django developers (Contributions to Django itself)
I'm against, there are lots of cases where a modelform is used to edit an exitsting object and thus the required fields are already set and you don't want them to be editable.

If it's a trivial patch then you should think about extending modelform in your own project enforce it there and then use it instead of the normal modelform.

I also think a good test setup/protocol will catch missing fields pretty quickly as you won't be able to actually create the object.

PARTH PATIL

unread,
Apr 17, 2019, 3:02:36 AM4/17/19
to django-d...@googlegroups.com
Yes I agree with others, that this should not be implemented. As I find this too much specific to just your case.

There are many cases, like what if there is some required field which the developer wants to set by themselves and don't want user to edit that (transaction details maybe for a shopping site).

You may Inherit a class from modelform and overide the behaviour for your project, buy I dont think this should be included in Django.

Best Regards,
PARTH PATIL


--
You received this message because you are subscribed to the Google Groups "Django developers (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email to django-develop...@googlegroups.com.
To post to this group, send email to django-d...@googlegroups.com.
Visit this group at https://groups.google.com/group/django-developers.

Tobias Kunze

unread,
Apr 17, 2019, 4:00:38 AM4/17/19
to django-d...@googlegroups.com
On 19-04-16 17:34:11, Will Gordon wrote:
>In the same way that editable fields are forced to not be included in a
>ModelForm (
>https://github.com/django/django/blob/master/django/forms/models.py#L146),
>I would like to propose that "required" fields (`blank=False`) be forced to
>be included in a ModelForm.

Is there anything missing in Django that would prevent you from doing
this in a subclass of ModelForm? My projects tend to go with one or two
common base classes in between actual forms/views and Django's
forms/views, and I'd argue that this is not too much trouble for a
feature like this.

I'm also against adding a warning for this by default – I'd like to keep
warnings reserved for issues with more of an impact. I think for every
one of my larger projects, I'd have to silence this warning, which is
annoying (and keeping it there introduces warning fatigue, which Django
so far does a good job of avoiding).

Tobias
signature.asc

Will Gordon

unread,
Apr 17, 2019, 8:45:15 AM4/17/19
to Django developers (Contributions to Django itself)
Well shoot...this definitely seems like something only I'm running into. Appreciate y'alls feedback. Thanks 👍🏻
Reply all
Reply to author
Forward
0 new messages