Opened 10 years ago

Closed 9 years ago

Last modified 8 years ago

#3896 closed (fixed)

pass value to field specific clean function

Reported by: Henrik Vendelbo <info@…> Owned by: nobody
Component: Forms Version: master
Severity: Keywords:
Cc: tailofthesun@…, hv@… Triage Stage: Design decision needed
Has patch: yes Needs documentation: yes
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description (last modified by James Bennett)

newforms has this bit of code in the full_clean function:

            try:
                value = field.clean(value)
                self.clean_data[name] = value
                if hasattr(self, 'clean_%s' % name):
                    value = getattr(self, 'clean_%s' % name)()
                self.clean_data[name] = value
            except ValidationError, e:
                errors[name] = e.messages

Why is value not passed to the clean_%s function? Since it is required to return the accepted value, it is odd that it is forced to look it up. Basicly a dictionary read and assignment is required with no obvious gain. Logically I find it odd as well that clean_data contains values that are not cleaned fully yet.

Attachments (2)

3896.diff (690 bytes) - added by Gary Wilson <gary.wilson@…> 9 years ago.
Pass the value to clean_XXX method and don't save to clean_data first.
3896.2.diff (2.0 KB) - added by Gary Wilson <gary.wilson@…> 9 years ago.
Only the passing of value to clean_XXX method, with test fixes.

Download all attachments as: .zip

Change History (10)

comment:1 Changed 10 years ago by James Bennett

Description: modified (diff)
Needs documentation: unset
Needs tests: unset
Patch needs improvement: unset

(cleaning up code formatting in description)

comment:2 Changed 9 years ago by Gary Wilson <gary.wilson@…>

Triage Stage: UnreviewedDesign decision needed

Seems logical to me. I think it is certainly a bug that clean_data will retain the value even if clean_XXX raises a ValidationError. So even if it is decided that value shouldn't be passed to clean_XXX, we at least need to fix the clean_data problem.

Changed 9 years ago by Gary Wilson <gary.wilson@…>

Attachment: 3896.diff added

Pass the value to clean_XXX method and don't save to clean_data first.

comment:3 Changed 9 years ago by Gary Wilson <gary.wilson@…>

Has patch: set
Needs documentation: set
Needs tests: set

Changed 9 years ago by Gary Wilson <gary.wilson@…>

Attachment: 3896.2.diff added

Only the passing of value to clean_XXX method, with test fixes.

comment:4 Changed 9 years ago by Gary Wilson <gary.wilson@…>

If this change goes in, then also consider removing the line specified in the first patch, which would fix #4391.

comment:5 Changed 9 years ago by Gary Wilson <gary.wilson@…>

Needs tests: unset

comment:6 Changed 9 years ago by anonymous

Cc: tailofthesun@… added

comment:7 Changed 9 years ago by Philippe Raoult

Resolution: fixed
Status: newclosed

it's really an implementation detail, but none the less fixed in [5346].

comment:8 Changed 8 years ago by Thomas Güttler

Cc: hv@… added
Note: See TracTickets for help on using tickets.
Back to Top