OpenLayersWidget's 'Delete all Features' control doesn't respect GeoModelAdmin's modifiable attribute

[Flavio Curella]

When GeoModelAdmin's modiable attribute is set to False, the controls for editing the geometry are not present on the change object page, but the 'Delete all Features' control is still there.

[Claude Paroz]

Why do you test wkt == '' in your patch?

[Flavio Curella]

@claudep: I was testing 'wkt' as a way to determine if the geometry was new or if it was an already existing one that needs to modified.

After running some tests I've realize that's the wrong attribute to check. On my new patch I pass the field's value to the template and I'm checking directly against that.

[Claude Paroz]

Oh, now I understand that modifiable allows to create a feature, but not modify it afterwards. I didn't get it at first, and the docs are not clear either about this. This is something we should also fix.

Could you also enlighten me about the difference between testing wkt or value? (it's already late :-) )

[Flavio Curella]

wkt is 'a string representation of the geometry in WKT format' (​https://docs.djangoproject.com/en/1.4/ref/contrib/gis/gdal/#django.contrib.gis.gdal.OGRGeometry.wkt), while value is the actual python object, whose __str__ (or __unicode__) method returns something like POLYGON((0 0, 5 0, 5 5, 0 5)).

I'd rather use value because wrt involves some geographical transformation, and could result in being '' due to errors while converting. (See ​https://github.com/django/django/blob/master/django/contrib/gis/admin/widgets.py#L67)

[Claude Paroz]

Flavio, sorry for such a long time without news. I revisited this issue today, and I think we should set the modifiable param to True when the widget has no existing value (instead of adding the value). If you can add a new feature, there are no reason you cannot edit it before saving. Thoughts?

[Flavio Curella]

@claudep

if i'm reading your comment correctly, are you proposing to have the editing tools (including the 'delete all features' button) only on the 'add_view', but not in the 'change_view'?

That would make sense, since 'modifiable' has a different meaning than 'editable'. I think we may need to make this semantic difference more explicit in the docs.

[Flavio Curella]

sorry, I see what you're saying now. you're proposing to set self.params['modifiable'] = True instead of self.params['value'] = value in widgets.py, and then just use {% if modifiable %} in template, right?

If that's the case, I'm conflicted. On one hand, that's a simpler implementation. On the other hand, feels like hijacking a value explicitly set by the user. Maybe we could set self.params['editable'] = True and check against that?

[Claude Paroz]

modifiable has already the meaning: can add new value, cannot edit existing value. Quoting the docs:

Defaults to True. When set to False, disables editing of existing geometry fields in the admin.

So we are simply pushing that logic a bit further down to the JS code.

[Tim Graham]

Patch no longer applies cleanly.

[Flavio Curella]

Updated PR at ​https://github.com/django/django/pull/3019

[Tim Graham]

Claude, as someone with more experience with GIS, could you check this?

[Claude Paroz <claude@…>]

In ce078ef976d83395dc19c9a1576529a5a0678514:

Fixed #19139 -- Made OpenLayersWidget follow GeoModelAdmin's modifiable attribute

Thanks Tim Graham for the review.