Opened 5 years ago

Closed 2 years ago

#15995 closed Bug (duplicate)

Problems in ModelForm._post_clean

Reported by: Florian Apolloner Owned by: nobody
Component: Forms Version: master
Severity: Normal Keywords:
Cc: Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no



I have a model with the following field:

    author = models.ForeignKey(User, on_delete=models.PROTECT)

The related modelform sets this field to required = False

This field should obviously pass form validation in the admin since required will be False! But then in _post_clean of the modelform it tries to construct the instance:

which immediately fails cause author can't be None:

Oddly enough, the code later on collects the fields for model validation: exclude = self._get_validation_exclusions(). This code would add author to the ignore list:

So model validation would be happyily exclude my field from validation, but it already fails in construct_instance. I think _post_clean should use the calculated exclude list instead of opts.exclude for the construct_instance call

Attachments (2) (893 bytes) - added by Florian Apolloner 5 years ago. (798 bytes) - added by Florian Apolloner 5 years ago.

Download all attachments as: .zip

Change History (8)

Changed 5 years ago by Florian Apolloner

Attachment: added

Changed 5 years ago by Florian Apolloner

Attachment: added

comment:1 Changed 5 years ago by Florian Apolloner

Needs documentation: unset
Needs tests: unset
Patch needs improvement: unset

Attached and My endgoal was to be able to set the author later on, without making the field nullable, If pass the excludes from _get_validation_excludes into construct_instance everything works…

comment:2 Changed 5 years ago by Aymeric Augustin

Triage Stage: UnreviewedAccepted

The idea makes sense. It would make the form validation slightly more tolerant, but that's not significantly backwards incompatible.

comment:3 Changed 5 years ago by anonymous

Hmm, my suggested solution won't work out. It works in my case, but if someone supplies real data it won't get set :( On the other way I fail to find an easy way around it *gg*

comment:4 Changed 5 years ago by kuba.janoszek@…

UI/UX: unset

Personally I think that using "construct_instance" function inside form's _post_clean method is not good idea.
Maybe it should me method? - If it would be a method it will be easy to change the way of instance construction.
Everybody who is unhappy with it will have opportunity to change it easily.

Here's my idea (should be part of django.forms.models.BaseModelForm class)


def _construct_instance(self):

opts = self._meta

return django.forms.models.construct_instance(self, self.instance, fields=opts.fields, exclude=opts.exclude)

def _post_clean(self):

self.instance = self._construct_instance()

# ... the rest of _post_clean method


comment:6 Changed 2 years ago by ANUBHAV JOSHI

Resolution: duplicate
Status: newclosed

Duplicate of #13776

Note: See TracTickets for help on using tickets.
Back to Top