yourcelf / olwidget

Javascript library to replace textareas that contain WKT data with editable OpenLayers maps, and a django app using it for django forms and admin.
Other
91 stars 43 forks source link

required=False on MapField leads to TypeError #46

Closed dyve closed 13 years ago

dyve commented 13 years ago

I have a Django form (actually a ModelForm) with a MapField that is NOT required.

I'm trying ot pass required=False like this:

new_geom = MapField(required=False,
    fields=[
        EditableLayerField({'geometry': 'polygon', 'name': 'boundary'}),
    ],
    options={
        'overlay_style': {
            'fill_color': '#00ff00',
        },
    },
)

This results in TypeError at (my code) init() got multiple values for keyword argument 'required'

When passing required=False to EditableLayerField it seems to work, but not allowing required=True on MapField seems counter-intuitive

yourcelf commented 13 years ago

Thanks for the report. The TypeError bug should be fixed in commit eddf468d929c55529cb4.

As for getting a field to be properly not required, you should put "required=False" in the LayerField, not the MapField. The MapField is a multi-value field; all of the "required" evaluation happens in its children (the component fields). The following should work for an optional geometry field:

new_geom = MapField(
    fields=[
        EditableLayerField({'geometry': 'polygon', 'name': 'boundary'}, required=False),
    ],
    options={
        'overlay_style': {
            'fill_color': '#00ff00',
        },
    },
)

I've added some test cases which cover this. in the latest commit; please open a new issue if this doesn't work for you.

yourcelf commented 13 years ago

I considered making required=False in the MapField propagate to its children. This might seem intuitive; but if you're actually using multiple children, you might want one to be required and the others not. It gets messy to consider what happens if you put "required=False" in the parent but "required=True" in the child, as the parent doesn't have access to the child's kwargs until the child's constructor has already been called, and thus it can't distinguish between a child field where "required=True" has been specified and one where nothing was specified.

So I think for now leaving required=False to the child fields (the EditableLayerField) makes the most sense.

dyve commented 13 years ago

Thanks for the fix and the explanation. I agree leaving required=False to the child fields (the EditableLayerField) makes the most sense.