From 43f7e6a4677a57ad8d24046f0450ecd92b17f0b3 Mon Sep 17 00:00:00 2001
From: Yohan Boniface <yohanboniface@free.fr>
Date: Thu, 6 Jun 2024 11:21:23 +0200
Subject: [PATCH] feat: allow to set the new layer name at import

---
 umap/static/umap/base.css                     |  2 +-
 umap/static/umap/js/modules/importer.js       | 37 +++++++++++++++----
 .../umap/js/modules/importers/communesfr.js   |  1 +
 .../umap/js/modules/importers/geodatamine.js  |  3 ++
 umap/static/umap/js/umap.js                   | 12 +++---
 umap/tests/integration/test_import.py         |  7 +++-
 6 files changed, 45 insertions(+), 17 deletions(-)

diff --git a/umap/static/umap/base.css b/umap/static/umap/base.css
index a8991951..2c3d0b2c 100644
--- a/umap/static/umap/base.css
+++ b/umap/static/umap/base.css
@@ -397,7 +397,7 @@ fieldset legend {
     content: attr(data-badge);
 }
 [hidden] {
-    display: none;
+    display: none!important;
 }
 
 /* Switch */
diff --git a/umap/static/umap/js/modules/importer.js b/umap/static/umap/js/modules/importer.js
index 22317817..e96c20c1 100644
--- a/umap/static/umap/js/modules/importer.js
+++ b/umap/static/umap/js/modules/importer.js
@@ -21,15 +21,16 @@ const TEMPLATE = `
       <legend class="counter" data-help="importFormats">${translate('Choose the data format')}</legend>
       <select name="format" onchange></select>
     </fieldset>
-    <fieldset class="destination formbox">
+    <fieldset id="destination" class="formbox">
       <legend class="counter">${translate('Choose the layer to import in')}</legend>
-      <select name="layer-id"></select>
-      <label>
+      <select name="layer-id" onchange></select>
+      <label id="clear">
         <input type="checkbox" name="clear" />
         ${translate('Replace layer content')}
       </label>
+      <input type="text" name="layer-name" placeholder="${translate('Layer name')}" />
     </fieldset>
-    <fieldset class="import-mode formbox">
+    <fieldset id="import-mode" class="formbox">
       <legend class="counter" data-help="importMode">${translate('Choose import mode')}</legend>
       <label>
         <input type="radio" name="action" value="copy" />
@@ -98,9 +99,25 @@ export default class Importer {
     return this.qs('[name=action]:checked').value
   }
 
+  get layerId() {
+    return this.qs('[name=layer-id]').value
+  }
+
+  set layerId(value) {
+    this.qs('[name=layer-id]').value = value
+  }
+
+  get layerName() {
+    return this.qs('[name=layer-name]').value
+  }
+
+  set layerName(name) {
+    this.qs('[name=layer-name]').value = name
+    this.onChange()
+  }
+
   get layer() {
-    const layerId = this.qs('[name=layer-id]').value
-    return this.map.datalayers[layerId] || this.map.createDataLayer()
+    return this.map.datalayers[this.layerId] || this.map.createDataLayer({name: this.layerName})
   }
 
   build() {
@@ -134,11 +151,13 @@ export default class Importer {
   }
 
   onChange() {
-    this.qs('.destination').toggleAttribute('hidden', this.format === 'umap')
-    this.qs('.import-mode').toggleAttribute(
+    this.qs('#destination').toggleAttribute('hidden', this.format === 'umap')
+    this.qs('#import-mode').toggleAttribute(
       'hidden',
       this.format === 'umap' || !this.url
     )
+    this.qs('[name=layer-name]').toggleAttribute('hidden', Boolean(this.layerId))
+    this.qs('#clear').toggleAttribute('hidden', !Boolean(this.layerId))
   }
 
   onFileChange(e) {
@@ -159,6 +178,7 @@ export default class Importer {
     this.qs('[type=file]').value = null
     this.url = null
     this.format = undefined
+    this.layerName = null
     const layerSelect = this.qs('[name="layer-id"]')
     layerSelect.innerHTML = ''
     this.map.eachDataLayerReverse((datalayer) => {
@@ -176,6 +196,7 @@ export default class Importer {
       value: '',
       textContent: translate('Import in a new layer'),
       parent: layerSelect,
+      selected: true
     })
   }
 
diff --git a/umap/static/umap/js/modules/importers/communesfr.js b/umap/static/umap/js/modules/importers/communesfr.js
index 845c2125..128423e5 100644
--- a/umap/static/umap/js/modules/importers/communesfr.js
+++ b/umap/static/umap/js/modules/importers/communesfr.js
@@ -28,6 +28,7 @@ export class Importer {
       on_select: (choice) => {
         importer.url = `https://geo.api.gouv.fr/communes?code=${choice.item.value}&format=geojson&geometry=contour`
         importer.format = 'geojson'
+        importer.layerName = choice.item.label
         importer.dialog.close()
       },
     }
diff --git a/umap/static/umap/js/modules/importers/geodatamine.js b/umap/static/umap/js/modules/importers/geodatamine.js
index 0a46b828..7e5b9898 100644
--- a/umap/static/umap/js/modules/importers/geodatamine.js
+++ b/umap/static/umap/js/modules/importers/geodatamine.js
@@ -37,6 +37,7 @@ export class Importer {
 
   async open(importer) {
     let boundary = null
+    let boundaryName = null
     const container = DomUtil.create('div')
     container.innerHTML = TEMPLATE
     const response = await importer.map.request.get(`${this.baseUrl}/themes`)
@@ -59,11 +60,13 @@ export class Importer {
       url: `${this.baseUrl}/boundaries/search?text={q}`,
       on_select: (choice) => {
         boundary = choice.item.value
+        boundaryName = choice.item.label
       },
     })
     const confirm = () => {
       importer.url = `${this.baseUrl}/data/${select.value}/${boundary}?format=geojson&aspoint=${asPoint.checked}`
       importer.format = 'geojson'
+      importer.layerName = `${boundaryName} — ${select.options[select.selectedIndex].textContent}`
       importer.dialog.close()
     }
     L.DomUtil.createButton('', container, 'OK', confirm)
diff --git a/umap/static/umap/js/umap.js b/umap/static/umap/js/umap.js
index 3401f4b5..8a1559f5 100644
--- a/umap/static/umap/js/umap.js
+++ b/umap/static/umap/js/umap.js
@@ -799,16 +799,14 @@ U.Map = L.Map.extend({
     return L.Map.prototype.setMaxBounds.call(this, bounds)
   },
 
-  createDataLayer: function (datalayer, sync) {
-    datalayer = datalayer || {
-      name: `${L._('Layer')} ${this.datalayers_index.length + 1}`,
-    }
-    const dl = new U.DataLayer(this, datalayer, sync)
+  createDataLayer: function (options = {}, sync) {
+    options.name = options.name || `${L._('Layer')} ${this.datalayers_index.length + 1}`
+    const datalayer = new U.DataLayer(this, options, sync)
 
     if (sync !== false) {
-      dl.sync.upsert(dl.options)
+      datalayer.sync.upsert(datalayer.options)
     }
-    return dl
+    return datalayer
   },
 
   newDataLayer: function () {
diff --git a/umap/tests/integration/test_import.py b/umap/tests/integration/test_import.py
index a124b50c..dbf14cb7 100644
--- a/umap/tests/integration/test_import.py
+++ b/umap/tests/integration/test_import.py
@@ -218,6 +218,8 @@ def test_can_import_in_existing_datalayer(live_server, datalayer, page, openmap)
     path = Path(__file__).parent.parent / "fixtures/test_upload_data.csv"
     textarea.fill(path.read_text())
     page.locator('select[name="format"]').select_option("csv")
+    page.locator('select[name="layer-id"]').select_option(datalayer.name)
+    expect(page.locator('input[name=layer-name]')).to_be_hidden()
     page.get_by_role("button", name="Add data", exact=True).click()
     # No layer has been created
     expect(layers).to_have_count(1)
@@ -237,6 +239,7 @@ def test_can_replace_datalayer_data(live_server, datalayer, page, openmap):
     path = Path(__file__).parent.parent / "fixtures/test_upload_data.csv"
     textarea.fill(path.read_text())
     page.locator('select[name="format"]').select_option("csv")
+    page.locator('select[name="layer-id"]').select_option(datalayer.name)
     page.get_by_label("Replace layer content").check()
     page.get_by_role("button", name="Add data", exact=True).click()
     # No layer has been created
@@ -258,10 +261,12 @@ def test_can_import_in_new_datalayer(live_server, datalayer, page, openmap):
     textarea.fill(path.read_text())
     page.locator("select[name=format]").select_option("csv")
     page.locator("[name=layer-id]").select_option(label="Import in a new layer")
+    page.locator("[name=layer-name]").fill("My new layer name")
     page.get_by_role("button", name="Add data", exact=True).click()
     # A new layer has been created
     expect(layers).to_have_count(2)
     expect(markers).to_have_count(3)
+    expect(page.get_by_text("My new layer name")).to_be_visible()
 
 
 def test_should_remove_dot_in_property_names(live_server, page, settings, tilelayer):
@@ -504,7 +509,7 @@ def test_import_geojson_from_url(page, live_server, tilelayer):
             }
         )
 
-    # Intercept the route to the proxy
+    # Intercept the route
     page.route("https://remote.org/data.json", handle)
     page.goto(f"{live_server.url}/map/new/")
     expect(page.locator(".leaflet-marker-icon")).to_be_hidden()