django.newforms.extras.widgets.SelectDateWidget

[Camille Harang]

Hi,

django.newforms.extras.widgets.SelectDateWidget is not correctly passing the previous selected value to the HTML renderer because it sometimes receives the value as a datetime.date field or a unicode string. The code only handles it as unicode, it splits it and converts it to datetime.date, so when it receives it directly as a datetime.date try/except resets the default value as None :

    def render(self, name, value, attrs=None):
        try:
            value = datetime.date(*map(int, value.split('-')))
            year_val, month_val, day_val = value.year, value.month, value.day
        except (AttributeError, TypeError, ValueError):
            year_val = month_val = day_val = None

I added a type value test and now it works:

    def render(self, name, value, attrs=None):
        try:
            if type(value) == type(u''):
                value = datetime.date(*map(int, value.split('-')))
            year_val, month_val, day_val = value.year, value.month, value.day
        except (AttributeError, TypeError, ValueError):
            year_val = month_val = day_val = None

Regards.

Camille.

[Camille Harang <mammique@…>]

if type(value) == type(u''): => isinstance(value, unicode)

Sorry ;-/

[Camille Harang <mammique@…>]

Fix + ID's

[Camille Harang <mammique@…>]

Here is the patch. I alos added individual id_* to each <select> inspired by CheckboxSelectMultiple()'s way to do it, it works, i hope it fits Django coding guidelines.

Regards.

Camille.

[Camille Harang <mammique@…>]

Fix + ID's 2 (same but uses month_field = '%s_month', etc...)

[Camille Harang <mammique@…>]

Fix + ID's 3 (same as Fix + ID's 2 but uses isinstance, this is the good one, sorry for flooding)

[Camille Harang <mammique@…>]

Good one, forgget above :-/

[Camille Harang <mammique@…>]

Fix + ID's 5

[Martin Conte Mac Donell <Reflejo@…>]

This is IMHO a better way to do that

[Martin Conte Mac Donell <Reflejo@…>]

This is IMHO a better way to do that (Sorry for the other one)

[Martin Conte Mac Donell <Reflejo@…>]

This is IMHO a better way to do that (Err.. Sorry for the other ones)

[anonymous]

I suggest using isinstance(value, basestring), or even wrapping the value in unicode().

def render(self, name, value, attrs=None):
    try:
        if isinstance(value, basestring):
            value = datetime.date(*map(int, value.split('-')))
        year_val, month_val, day_val = value.year, value.month, value.day
    except (AttributeError, TypeError, ValueError):
        year_val = month_val = day_val = None

    # -- snip assumed --

def render(self, name, value, attrs=None):
    try:
       
        value = datetime.date(*map(int, unicode(value).split('-')))
        year_val, month_val, day_val = value.year, value.month, value.day
    except (AttributeError, TypeError, ValueError):
        year_val = month_val = day_val = None
 
    # -- snip assumed --

[Joseph Long <josephoenix@…>]

Change to render() for SelectDateWidget and test to ensure it works

[Joseph Long]

The file I just attached (wasn't logged in, oops) adds tests for SelectDateWidget rendering for this issue, and uses isinstance instead of type() in * to see whether the value is a string. Hopefully with these, this can be patched in the trunk. (I don't know about the IDs.. should those be patched for this ticket as well, or is that a separate issue?)

Joseph

[Chris Beaven]

I think that value_from_datadict should return a date anyway. But that's a side point from this bug, of which the latest patch looks good to go.

[Joseph Long]

Checks isinstance of string in render, and adds a test. Removes TypeError from exceptions, since isinstance means that can't be raised (as pointed out by SmileyChris)

[Chris Beaven]

Actually, I was wrong. TypeError could be raised still:
datetime.date(*map(int, '1-2'.split('-')))

[Joseph Long]

Nevermind removing TypeError. It could still be raised.. this is the better version of the patch

[ryan@…]

If "years" is not specified but initial date value is, perhaps self.years should be range(value.year, this_year+10)

Also, if date validation fails (eg. February 31), it would be nice if the widget would repopulate with the failed date, rather than resetting everything back to the first entry in the list.

Otherwise nice... :)

[Chris Beaven]

Replying to ryan@pressure.net.nz:

If "years" is not specified but initial date value is, perhaps self.years should be range(value.year, this_year+10)

This is a separate proposal to the fix here, feel free to open a new ticket.

Also, if date validation fails (eg. February 31), it would be nice if the widget would repopulate with the failed date, rather than resetting everything back to the first entry in the list.

While this is a separate issue (bug IMO), I'll fix it here since it's not that difficult.

Rolling back to Accepted and assigning to me.

[Chris Beaven]

Allow widget to receive a datetime and also render invalid dates (validation doesn't belong in the widget)

[Malcolm Tredinnick]

(In [7337]) Fixed #5917 -- More error robustness in date parsing in SelectDateWidget, plus
keep the original date selected on redisplay, even if it was bogus (e.g. 31
Feb). Patch from SmileyChris.