#11303: changed_data works incorrectly with BooleanField
-------------------------------+--------------------------------------------
Reporter: oduvan | Owner: nobody
Status: new | Milestone:
Component: Forms | Version: 1.1
Resolution: | Keywords:
Stage: Accepted | Has_patch: 1
Needs_docs: 0 | Needs_tests: 0
Needs_better_patch: 0 |
-------------------------------+--------------------------------------------
Changes (by semenov):
* version: 1.1-beta-1 => 1.1
* summary: changed_data wrong work with BooleanField => changed_data
works incorrectly with BooleanField
Comment:
I agree with @margieroginski. The current show_hidden_input implementation
is just broken when it comes to booleans/checkboxes. @russellm, this isn't
an "edge case annoyance...in the admin" at all. The show_hidden_input
logic is '''very''' useful when you allow to edit a single (shared) model
instance to many users simultaneously; they won't be posting "old" data
and rewrite all changes made by the other collaborators. You shouldn't
underestimate the importance, as this is actually a very common use case -
that would be useful even in ''this Django bug tracker'', since that's not
a rare situation when someone's changes to Summary or Triage Stage is
erroneously "reverted" by the next poster - a perfect example where hidden
inputs should have been added.
I found this ticket because I first developed ''my own'' generic
implementation of exactly the same logic - I was saving pristine values
along with the form and then checking them on submit. Then I discovered
the show_hidden_input parameter and rewrote my app to use it instead. The
funny thing is that in my implementation, I didn't have the discussed bug
since I was testing on a form which had BooleanFields from the very
beginning so I specifically added a workaround. You can imagine my
disappointment when my code suddenly broke when switched to the "original"
Django solution.[[br]][[br]]
> In general it seems very difficult to compare all these values such as
False, 'False', u'0', u'1', True, 'True' and get it right.
I can't agree with that. Overestimating the complexity of the problem is
no better than underestimating its importance :)[[br]]
There's no such things as 'False'. How did you get that? When a python
value is rendered, it gets through force_unicode() and False can only
become '0'. Therefore, the logic is very simple:[[br]]
1) no data, !'' and '0' is False[[br]]
2) all the rest is True
The bug can be traced to this code in forms.Form:
{{{
#!python
def _get_changed_data(self):
# ...
for name, field in self.fields.items():
# ...
if field.widget._has_changed(initial_value, data_value):
self._changed_data.append(name)
}}}
The initial_data here is taken from hidden_widget (which is filled with
'0' for False when a form is first rendered) which is then casted to True
by bool() inside CheckboxInput._has_changed.
Therefore, the two possible fixes could be:[[br]]
1) make CheckboxInput._has_changed smarter and treat initial='0' as
False[[br]]
2) add a custom BooleanField.hidden_widget (so called BooleanHiddenInput)
which would store !'' for False instead of '0'
I prefer the second approach as it allows to get rid of hard coded
comparisons against string '0'.
--
Ticket URL: <http://code.djangoproject.com/ticket/11303#comment:10>
Django <http://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
--
You received this message because you are subscribed to the Google Groups
"Django updates" group.
To post to this group, send email to django-upda...@googlegroups.com.
To unsubscribe from this group, send email to
django-updates+unsubscr...@googlegroups.com.
For more options, visit this group at
http://groups.google.com/group/django-updates?hl=en.
