From cebaad5b5fc7d156852279296ca0d44bbd6af1de Mon Sep 17 00:00:00 2001
From: Valentin Deniaud <vdeniaud@entrouvert.com>
Date: Thu, 4 Nov 2021 17:24:52 +0100
Subject: [PATCH] maps: move layer request parameter options to separate form
 (#57761)

---
 combo/apps/maps/forms.py                      | 17 +++++++++-----
 combo/apps/maps/manager_views.py              | 10 ++++++++-
 .../maps/templates/maps/map_layer_form.html   | 11 +++++++++-
 .../map_layer_request_parameters_form.html    | 11 ++++++++++
 combo/apps/maps/urls.py                       |  5 +++++
 tests/test_maps_manager.py                    | 22 +++++++++++--------
 6 files changed, 60 insertions(+), 16 deletions(-)
 create mode 100644 combo/apps/maps/templates/maps/map_layer_request_parameters_form.html

diff --git a/combo/apps/maps/forms.py b/combo/apps/maps/forms.py
index 707ebf47..69477789 100644
--- a/combo/apps/maps/forms.py
+++ b/combo/apps/maps/forms.py
@@ -37,7 +37,12 @@ class MapLayerForm(forms.ModelForm):
 
     class Meta:
         model = MapLayer
-        exclude = ('kind',)
+        exclude = (
+            'kind',
+            'include_user_identifier',
+            'geojson_query_parameter',
+            'geojson_accepts_circle_param',
+        )
         widgets = {
             'marker_colour': ColourOrTextInput,
             'icon_colour': forms.TextInput(attrs={'type': 'color'}),
@@ -49,7 +54,6 @@ class MapLayerForm(forms.ModelForm):
             # new instance, delete some fields
             del self.fields['slug']
             del self.fields['cache_duration']
-            del self.fields['include_user_identifier']
         else:
             # new widget for icon field
             self.fields['icon'].widget = IconRadioSelect()
@@ -65,9 +69,6 @@ class MapLayerForm(forms.ModelForm):
                 'icon',
                 'icon_colour',
                 'cache_duration',
-                'include_user_identifier',
-                'geojson_query_parameter',
-                'geojson_accepts_circle_param',
             ]
         for field in todelete_fields:
             if field in self.fields:
@@ -83,6 +84,12 @@ class MapLayerForm(forms.ModelForm):
         return cleaned_data
 
 
+class MapLayerRequestParametersForm(forms.ModelForm):
+    class Meta:
+        model = MapLayer
+        fields = ('include_user_identifier', 'geojson_query_parameter', 'geojson_accepts_circle_param')
+
+
 class MapLayerOptionsForm(forms.ModelForm):
     class Meta:
         model = MapLayerOptions
diff --git a/combo/apps/maps/manager_views.py b/combo/apps/maps/manager_views.py
index 624e717c..a8ab0249 100644
--- a/combo/apps/maps/manager_views.py
+++ b/combo/apps/maps/manager_views.py
@@ -22,7 +22,7 @@ from django.views.generic import CreateView, DeleteView, ListView, UpdateView
 
 from combo.data.models import CellBase, PageSnapshot
 
-from .forms import MapLayerForm, MapLayerOptionsForm
+from .forms import MapLayerForm, MapLayerOptionsForm, MapLayerRequestParametersForm
 from .models import Map, MapLayer, MapLayerOptions
 
 
@@ -56,6 +56,14 @@ class LayerEditView(MapLayerMixin, UpdateView):
     template_name = 'maps/map_layer_form.html'
 
 
+class LayerEditRequestParametersView(MapLayerMixin, UpdateView):
+    form_class = MapLayerRequestParametersForm
+    template_name = 'maps/map_layer_request_parameters_form.html'
+
+    def get_success_url(self):
+        return reverse('maps-manager-layer-edit', kwargs={'slug': self.object.slug})
+
+
 class LayerDeleteView(MapLayerMixin, DeleteView):
     template_name = 'maps/map_layer_confirm_delete.html'
 
diff --git a/combo/apps/maps/templates/maps/map_layer_form.html b/combo/apps/maps/templates/maps/map_layer_form.html
index e11a5e45..d2ce6f35 100644
--- a/combo/apps/maps/templates/maps/map_layer_form.html
+++ b/combo/apps/maps/templates/maps/map_layer_form.html
@@ -4,6 +4,11 @@
 {% block appbar %}
 {% if object.id %}
 <h2>{% trans "Edit Map Layer" %}</h2>
+{% if object.kind == 'geojson' %}
+<span class="actions">
+<a rel="popup" href="{% url 'maps-manager-layer-edit-request-parameters' slug=object.slug %}">{% trans 'Request parameters' %}</a>
+</span>
+{% endif %}
 {% else %}
 <h2>{% trans "New Map Layer" %}</h2>
 {% endif %}
@@ -11,7 +16,11 @@
 
 {% block breadcrumb %}
 {{ block.super }}
-<a href="{% url 'maps-manager-homepage' %}">{% trans 'Maps' %}</a>
+{% if object.id %}
+<a href="{% url 'maps-manager-layer-edit' slug=object.slug %}">{% trans 'Edit Map Layer' %}</a>
+{% else %}
+<a href="#">{% trans "New Map Layer" %}</a>
+{% endif %}
 {% endblock %}
 
 {% block content %}
diff --git a/combo/apps/maps/templates/maps/map_layer_request_parameters_form.html b/combo/apps/maps/templates/maps/map_layer_request_parameters_form.html
new file mode 100644
index 00000000..d7d0bea7
--- /dev/null
+++ b/combo/apps/maps/templates/maps/map_layer_request_parameters_form.html
@@ -0,0 +1,11 @@
+{% extends "maps/map_layer_form.html" %}
+{% load i18n %}
+
+{% block appbar %}
+<h2>{% trans "Edit request parameters" %}</h2>
+{% endblock %}
+
+{% block breadcrumb %}
+{{ block.super }}
+<a rel="popup" href="{% url 'maps-manager-layer-edit-request-parameters' slug=object.slug %}">{% trans "Edit request parameters" %}</a>
+{% endblock %}
diff --git a/combo/apps/maps/urls.py b/combo/apps/maps/urls.py
index b2d2d6b7..a32b9487 100644
--- a/combo/apps/maps/urls.py
+++ b/combo/apps/maps/urls.py
@@ -33,6 +33,11 @@ maps_manager_urls = [
         manager_views.LayerEditView.as_view(),
         name='maps-manager-layer-edit',
     ),
+    url(
+        r'^layers/(?P<slug>[\w-]+)/edit/request-parameters/$',
+        manager_views.LayerEditRequestParametersView.as_view(),
+        name='maps-manager-layer-edit-request-parameters',
+    ),
     url(
         r'^layers/(?P<slug>[\w-]+)/delete/$',
         manager_views.LayerDeleteView.as_view(),
diff --git a/tests/test_maps_manager.py b/tests/test_maps_manager.py
index 22d4581a..4117c94e 100644
--- a/tests/test_maps_manager.py
+++ b/tests/test_maps_manager.py
@@ -53,7 +53,6 @@ def test_add_geojson_layer(app, admin_user):
     resp = resp.click('New GeoJSON layer')
     assert 'slug' not in resp.context['form'].fields
     assert 'cache_duration' not in resp.context['form'].fields
-    assert 'include_user_identifier' not in resp.context['form'].fields
     assert 'tiles_template_url' not in resp.context['form'].fields
     assert 'tiles_attribution' not in resp.context['form'].fields
     assert 'tiles_default' not in resp.context['form'].fields
@@ -64,16 +63,12 @@ def test_add_geojson_layer(app, admin_user):
     resp.form['icon'] = 'bicycle'
     assert resp.form['icon_colour'].value == '#000000'
     resp.form['icon_colour'] = '#FFFFFF'
-    resp.form['geojson_query_parameter'] = 'foobar'
-    resp.form['geojson_accepts_circle_param'] = True
     resp = resp.forms[0].submit()
     assert resp.location.endswith('/manage/maps/')
     assert MapLayer.objects.count() == 1
     layer = MapLayer.objects.get()
     assert layer.label == 'Test'
     assert layer.slug == 'test'
-    assert layer.geojson_query_parameter == 'foobar'
-    assert layer.geojson_accepts_circle_param is True
     assert layer.kind == 'geojson'
 
 
@@ -83,13 +78,10 @@ def test_add_tiles_layer(app, admin_user):
     resp = resp.click('New tiles layer')
     assert 'slug' not in resp.context['form'].fields
     assert 'cache_duration' not in resp.context['form'].fields
-    assert 'include_user_identifier' not in resp.context['form'].fields
     assert 'geojson_url' not in resp.context['form'].fields
     assert 'marker_colour' not in resp.context['form'].fields
     assert 'icon' not in resp.context['form'].fields
     assert 'icon_colour' not in resp.context['form'].fields
-    assert 'geojson_query_parameter' not in resp.context['form'].fields
-    assert 'geojson_accepts_circle_param' not in resp.context['form'].fields
     resp.forms[0]['label'] = 'Test'
     resp.forms[0]['tiles_template_url'] = 'http://somedomain.com/blabla/{z}/{x}/{y}{r}.png'
     resp.forms[0]['tiles_attribution'] = 'Foo bar'
@@ -138,17 +130,29 @@ def test_edit_geojson_layer(app, admin_user, layer):
     layer.refresh_from_db()
     assert layer.geojson_url == 'http://example.net/new_geojson'
 
+    resp = resp.follow().click('Test')
+    resp = resp.click('Request parameters')
+    resp.form['geojson_query_parameter'] = 'foobar'
+    resp.form['geojson_accepts_circle_param'] = True
+    resp.form['include_user_identifier'] = True
+    resp = resp.form.submit()
+    assert resp.location.endswith('/manage/maps/layers/%s/edit/' % layer.slug)
+    layer.refresh_from_db()
+    assert layer.geojson_query_parameter == 'foobar'
+    assert layer.geojson_accepts_circle_param is True
+    assert layer.include_user_identifier is True
+
 
 def test_edit_tiles_layer(app, admin_user, tiles_layer):
     app = login(app)
     resp = app.get('/manage/maps/', status=200)
     resp = resp.click('Test')
     assert 'cache_duration' not in resp.context['form']
-    assert 'include_user_identifier' not in resp.context['form']
     assert 'geojson_url' not in resp.context['form']
     assert 'marker_colour' not in resp.context['form']
     assert 'icon' not in resp.context['form']
     assert 'icon_colour' not in resp.context['form']
+    assert 'Request parameters' not in resp.text
     resp.forms[0]['tiles_default'] = False
     resp = resp.forms[0].submit()
     assert resp.location.endswith('/manage/maps/')
-- 
2.30.2