#33040 closed Uncategorized (invalid)
BaseModelForm.is_valid() should document how it modifies the BaseModelForm.instance
Reported by: | Danang Rahmatullah | Owned by: | nobody |
---|---|---|---|
Component: | Documentation | Version: | 3.2 |
Severity: | Normal | Keywords: | BaseModelForm is_valid |
Cc: | Triage Stage: | Unreviewed | |
Has patch: | no | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
BasedModelForm
extends BaseForm
. When BaseModelForm().is_valid()
is called, its super is called which in turn calls BaseForm.errors
(property method). The issue is when there are no errors (self._errors is None): BaseForm
calls full_clean()
which runs the _post_clean()
hook defined in BaseModelForm
, finally modifying BaseModelForm.instance
.
Given the abstraction level of subclasses of BaseModelForm
such as forms.ModelForm
, it is very unreasonable to expect that a seemingly simple function like is_valid()
to create such a big change to a property as important as instance
. I hope this side-effect is clearly explained in the documentation or have the code refactored to separate the modifying effect of is_valid()
function from the form validity check
# form.instance.myfield = False form = MyModelForm(instance=myinstance, data={"myfield": True}) # form.instance.myfield = False form.is_valid() # form.instance.myfield = True
Change History (2)
comment:1 by , 3 years ago
Easy pickings: | unset |
---|---|
Resolution: | → invalid |
Status: | new → closed |
comment:2 by , 3 years ago
Thanks for the link attached. I was mainly looking at the Forms API page which currently has no references to the link you have posted. In that case, I think adding a small reference link to the validation page from the forms API page would greatly help others.
Yes, this is explained in detail in docs. I'm not sure what change you're proposing.
If you're having trouble understanding how Django works, see TicketClosingReasons/UseSupportChannels for ways to get help.