Code

Opened 5 years ago

Closed 5 years ago

Last modified 3 years ago

#10872 closed (fixed)

Support of 'list_editable' in admin when using GeoDjango geometry fields

Reported by: springmeyer Owned by: nobody
Component: GIS Version: master
Severity: Keywords:
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: yes
Easy pickings: UI/UX:

Description

The post data sent from a GeoDjango geometry field currently fails when 'list_editable' is used in the Django Admin (when using the GeoDjango OSMGeoAdmin)

The problem occurs for two small reasons:

1) Because the geometry field is a textarea whose name is contructed from {{ field }} rather than {{ id }} the post data does not get prepended with the 'form-row' information.

2) Because (I think) the geometry widget textarea is hidden the geometry field {{ id }} is prefixed with 'id_' unlike other fields I've noticed.

So, any easy fix for #1 is to use {{ id }} instead of {{ field }} like:

Index: django/contrib/gis/templates/gis/admin/openlayers.html
===================================================================
--- django/contrib/gis/templates/gis/admin/openlayers.html	(revision 10604)
+++ django/contrib/gis/templates/gis/admin/openlayers.html	(working copy)
@@ -32,6 +32,6 @@
 <div id="{{ id }}_map"{% if LANGUAGE_BIDI %} dir="ltr"{% endif %}></div>
 <a href="javascript:{{ module }}.clearFeatures()">Delete all Features</a>
 {% if display_wkt %}<p> WKT debugging window:</p>{% endif %}
-<textarea id="{{ id }}" class="vWKTField required" cols="150" rows="10" name="{{ field_name }}">{{ wkt }}</textarea>
+<textarea id="{{ id }}" class="vWKTField required" cols="150" rows="10" name="{{ id }}">{{ wkt }}</textarea>
 <script type="text/javascript">{% block init_function %}{{ module }}.init();{% endblock %}</script>
 </span>

But that raises/creates issue #2 which effectively breaks the successful posting of data in both the list_editable changelist view and on the normal record view page (due to the 'id_' prefixing).

I've been looking through the commit (r10077) and the django forms trying to track down how and where and why this 'id_' gets prefixed. I can't quite suss out where it is happening well enough to feel like I know the right place to tackle this.

So, my best fix at this point is to strip the 'id_' off of the field id inside the geodjango form widget. The attached patch does that in a way to gets the list_editable working and still works great with the normal record edit/view page. So, a classic 'this patch works (seemingly) perfectly, but sucks : )

I'm concerned about some solution to this because I anticipate that once users running 1.1 discover this 'list_editable' feature I'm sure many using GeoDjango folks will want to try it out and will hit this.

Attachments (2)

geodjango_widget_list_editable_support.patch (3.1 KB) - added by springmeyer 5 years ago.
Hack to get geodjango admin map widget working with post data
geodjango_widget_list_editable_support2.patch (1.9 KB) - added by springmeyer 5 years ago.
New patch to solve problem more cleanly using the widget attr 'name'

Download all attachments as: .zip

Change History (6)

Changed 5 years ago by springmeyer

Hack to get geodjango admin map widget working with post data

comment:1 Changed 5 years ago by Alex

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement set
  • Triage Stage changed from Unreviewed to Accepted

Changed 5 years ago by springmeyer

New patch to solve problem more cleanly using the widget attr 'name'

comment:2 Changed 5 years ago by springmeyer

Alex wisely caught the deeper issue here that we should be using 'name' instead of 'field' from the attr's for the unique field name. Doing so, along with using 'name' for the namespacing of each map's javascript gets things working nicely and more cleanly for both the changelist view and the record view.

New patch attached. Didn't need to alter the openlayers.js at all (therefore left the reference to {{ field }} in that file - we can clean that up whenever).

Thanks Alex,

-dane

comment:3 Changed 5 years ago by jbronn

  • Resolution set to fixed
  • Status changed from new to closed

(In [10636]) Fixed #10872 -- Geographic admin now supports new list_editable option. Thanks to Dane Springmeyer and Alex Gaynor for the solution.

comment:4 Changed 3 years ago by jacob

  • milestone 1.1 deleted

Milestone 1.1 deleted

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.