#5355 closed (fixed)
DecimalField may "clean" itself and then raise an Exception
Reported by: | Owned by: | Philippe Raoult | |
---|---|---|---|
Component: | Forms | Version: | dev |
Severity: | Keywords: | newforms, sprintsept14 | |
Cc: | Triage Stage: | Ready for checkin | |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
DecimalField may "clean" itself and then raise an Exception. In my case, it happens when I put a DecimalField in a django.newforms.Form class which will be used to "Edit" the model.
I suppose it would be necessary to check the type of parameter value like some of the other "Field" classes, such as DateField.
super(DecimalField, self).clean(value) if not self.required and value in EMPTY_VALUES: return None value = value.strip() # raise AttributeError, 'Decimal' object has no attribute 'strip' try: value = Decimal(value)
Attachments (2)
Change History (9)
by , 17 years ago
Attachment: | fields.py.patch added |
---|
comment:1 by , 17 years ago
Has patch: | unset |
---|---|
Keywords: | newforms added |
Triage Stage: | Unreviewed → Design decision needed |
comment:2 by , 17 years ago
Keywords: | sprintsept14 added |
---|---|
Owner: | changed from | to
Triage Stage: | Design decision needed → Accepted |
comment:3 by , 17 years ago
Has patch: | set |
---|---|
Status: | new → assigned |
comment:4 by , 17 years ago
Triage Stage: | Accepted → Ready for checkin |
---|
comment:5 by , 17 years ago
So, for reference, there were a few problems with this patch.
- The first version didn't check the precision.
- The second version didn't work on input like " 3.14".
- The test in the second version showed that IntegerField was cleaning 3.14 (the number) to 3, rather than raising an error.
I'm committing something that fixes all of these.
comment:6 by , 17 years ago
Resolution: | → fixed |
---|---|
Status: | assigned → closed |
comment:7 by , 17 years ago
There were an extensive discussion last night (european time); I simply assumed that tests in the final patch matched what was agreed upon.
There's no reason value would already be a decimal unless the data was a decimal in the first place. If you use a form with string input this will never happen. Maybe we should check in form inputs that the values are really strings and/or enforce this ?