ModelForm.is_valid() and ModelForm.errors have backwards-incompatible side effects since model validation
|Reported by:||Luke Plant||Owned by:||jkocherhans|
|Cc:||Triage Stage:||Design decision needed|
|Has patch:||no||Needs documentation:||no|
|Needs tests:||no||Patch needs improvement:||no|
Since r12098 (model validation merge),
ModelForm.errors have the side effect of modifying the bound instance.
Attached is a (rather lazily written) patch to the tests which demonstrates the problem - it ought to pass, and does pass before r12098 (noting that the patch doesn't apply cleanly before r12098), but not after.
This is a significant backwards incompatibility, but rather subtle too - in my case it was caught only due to fairly extensive tests in one of my projects. It is also quite counter-intuitive that either
form.is_valid should have any side effects. At the very least this needs to be clearly documented. I'm guessing it might be tricky to fix, given earlier problems surrounding the order of events in model validation.
For your information, my use case is detecting that a change has occurred to a certain field:
if form.is_valid(): orig_val = obj.val obj = form.save() if obj.val != orig_val: do_something()
The side effects of
form.is_valid() mean that
do_something() is no longer called.
Change History (8)
comment:1 Changed 7 years ago by
|Patch needs improvement:||unset|
comment:2 Changed 7 years ago by
|Component:||Uncategorized → Forms|
|Triage Stage:||Unreviewed → Design decision needed|