CRITICAL BUG in FormAlchemy v1.4.3: _sanitize_select_options() in helpers.py

[coredumperror]

I can't even begin to imagine what this thing is supposed to be doing, but what it's actually doing is destroying long <select> lists.

Here's the code for the function:

def _sanitize_select_options(options):
    if isinstance(options, (list, tuple)):
        if _only_contains_leaves(options) and len(options) >= 2:
            return (options[1], options[0])
        else:
            return [_sanitize_select_options(option) for option in options]
    return options

Notice that if the options variable is a list of length >= 2, with elements which are not also lists (e.g. a list of several strings), it converts options into (options[1], options[0]).

Why in the world does it do this? This results in every list of 2+ options being incorrectly rendered into a <select>.

I'm no expert when it comes to recursive functions, but it appears that (besides destroying every option list) this function doesn't do anything. When I made what appeared to be the most sensible fix the remove the bug, I realized that the function stopped having any effect.

I'd suggest either removing _sanitize_select_options(), or re-writing it to do what it was originally intended to do, whatever that was. Until that time, though, FormAlchemy v1.4.3 is completely worthless. I was forced to downgrade to v1.4.2.

[maxiberta]

Downgrading to 1.4.2 exposes issue #40 (cannot import exceptions from sqlalchemy), but at least there's a trivial fix for that.

Also noticed that several unit tests in formalchemy/tests/test_dates.py pass in 1.4.2 but fail in 1.4.3. In particular, _sanitize_select_options() breaks TimeFieldRenderer, and as a result only 2 options are shown in Hour, Minute and Second fields. Looks like 1.4.3 was released with little QA :-(

[maxiberta]

Commit a2d26dc13d5654ef4f34adc87e941ff3cd82f4fd (Support for webhelpers optgroups) introduced _sanitize_select_options(). There's a descriptive commit message but I can't figure out what it means.

[smurfix]

Apparently the author of this patch (a) never used "raw" options, but always (key,value) tuples, (b) didn't think, (c) didn't run the testsuite (it appears to have been broken before this commit …)

I fixed the formalchemy code to get the testsuite working again, and to always use tuples internally; see #60. I probably also should add a reversal of this patch (and a few more testcases for this) to my patchset, but time is limited. :-/