Opened 4 years ago

Closed 17 months ago

#15995 closed Bug (duplicate)

Problems in ModelForm._post_clean

Reported by: apollo13 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 apollo13 4 years ago. (798 bytes) - added by apollo13 4 years ago.

Download all attachments as: .zip

Change History (8)

Changed 4 years ago by apollo13

Changed 4 years ago by apollo13

comment:1 Changed 4 years ago by apollo13

  • 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 4 years ago by aaugustin

  • Triage Stage changed from Unreviewed to Accepted

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

comment:3 Changed 4 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 4 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 17 months ago by anubhav9042

  • Resolution set to duplicate
  • Status changed from new to closed

Duplicate of #13776

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