Opened 15 years ago

Closed 14 years ago

#12595 closed Bug (fixed)

Localflavor bad arguments handling

Reported by: Sayane Owned by: Jannis Leidel
Component: contrib.localflavor Version:
Severity: Normal Keywords:
Cc: say4ne@…, Mike Fogel 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

A lot of form fields in django.contrib.localflavor handles arguments to __init__() in a bad way.

    def __init__(self, *args, **kwargs):
        super(PLPESELField, self).__init__(r'^\d{11}$',
            max_length=None, min_length=None, *args, **kwargs)

Every line like first one should be fixed like this:

    def __init__(self, max_length=None, min_length=None, *args, **kwargs):

Because if someone will pass max_length or min_length argument to __init__() it will fail (multiple min_length/max_length arguments - additional one will be in kwargs).

It is a big problem, if i'm trying to do something like this:

class PostalCodeField(models.CharField):
    def formfield(self, **kwargs):
        kwargs.setdefault("form_class", PLPostalCodeField)
        return super(self.__class__, self).formfield(**kwargs)

because django.db.models.CharField sets max_length to self.max_length.

I'm attaching patch to fix polish localflavor.

Attachments (3)

localflavor_pl_fix.diff (1.6 KB ) - added by Sayane 15 years ago.
12595-1-wip.diff (5.6 KB ) - added by Claude Paroz 14 years ago.
12595-2.diff (32.0 KB ) - added by Claude Paroz 14 years ago.

Download all attachments as: .zip

Change History (15)

by Sayane, 15 years ago

Attachment: localflavor_pl_fix.diff added

comment:1 by Sayane, 15 years ago

Cc: say4ne@… added

comment:2 by Jannis Leidel, 15 years ago

Triage Stage: UnreviewedAccepted

comment:3 by Jannis Leidel, 15 years ago

Owner: changed from nobody to Jannis Leidel

comment:4 by Russell Keith-Magee, 15 years ago

milestone: 1.2
Version: SVN

comment:5 by anonymous, 14 years ago

Cc: Mike Fogel added

comment:6 by Jannis Leidel, 14 years ago

Owner: Jannis Leidel removed

comment:7 by Matt McClanahan, 14 years ago

Severity: Normal
Type: Bug

comment:8 by Julien Phalip, 14 years ago

Needs tests: set

comment:9 by Claude Paroz, 14 years ago

Easy pickings: unset
Patch needs improvement: set

I basically see what's the problem and what to do. Now there is a lot of similar changes to make, and I would like to know if I'm going in the right direction before giving more time to this.

See my patch, and tell me if it's OK (and good practice) to pass max_length and min_length to super as args (vs kwargs).
For tests, should I add field_kwargs to all tests, or add a second assert with field_kwargs to all tests, or simply add field_kwargs once per test case?

by Claude Paroz, 14 years ago

Attachment: 12595-1-wip.diff added

by Claude Paroz, 14 years ago

Attachment: 12595-2.diff added

comment:10 by Claude Paroz, 14 years ago

Needs tests: unset
Patch needs improvement: unset

OK, fixed now in all localflavors. Tested in utils.assertFieldOutput.

comment:11 by Jannis Leidel, 14 years ago

Triage Stage: AcceptedReady for checkin

comment:12 by Jannis Leidel, 14 years ago

Owner: set to Jannis Leidel
Resolution: fixed
Status: newclosed

In [16146]:

Fixed #12595 -- Fixed bad arguments handling in localflavor form fields.

Note: See TracTickets for help on using tickets.
Back to Top