Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

✨ Add new geocoding provider: Bing #646

Merged
merged 44 commits into from
Nov 24, 2023
Merged
Show file tree
Hide file tree
Changes from 15 commits
Commits
Show all changes
44 commits
Select commit Hold shift + click to select a range
b4e1702
Add GEOCODING_PROVIDERS setting.
Nov 8, 2023
0097084
Add geocoding_providers to qdjango.project model.
Nov 8, 2023
fbd085b
Update base settings GEOCODING_PROVIDERS.
Nov 8, 2023
c4d0067
Add qdjango utility: get_geocoding_providers.
Nov 8, 2023
73f1334
Add geocoding_provider CRUD.
Nov 8, 2023
469b402
Change initconfig API REST proeperty group.mapcontrols from list to d…
Nov 8, 2023
c9e04b1
Fix for update projects with geocoding_providers empty or None.
Nov 8, 2023
2218022
Check for geocoding_providers is empty.
Nov 8, 2023
3f493f5
Update geoconding_providers qdjango.project field for backward compat…
Nov 9, 2023
04c50f2
Fix test.
Nov 9, 2023
5e3aab4
Merge branch 'dev' into Set_geocoding_provider
Nov 9, 2023
b62488f
Add sql to migration for change mapcontrol nominatim in geocoding
Nov 9, 2023
803aca9
Update mapcontrol structure
Nov 9, 2023
bd392a3
Change from 'provider' to 'providers' inside the options of mapcontro…
Nov 10, 2023
fc1f9b0
Change from 'provider' to 'providers' inside the options of mapcontro…
Nov 10, 2023
9354137
:arrow_up: Client:
volterra79 Nov 10, 2023
18c4005
:arrow_up: Client:
volterra79 Nov 14, 2023
4600cf7
:arrow_up: Client:
volterra79 Nov 14, 2023
a5e397c
Small fix.
Nov 9, 2023
b6ed5dd
Typo.
Nov 9, 2023
6a58099
Add email exists check on recovery username form.
Nov 9, 2023
81b6078
Refactoring and add settings to reset password and user recovery emai…
Nov 10, 2023
77fe30d
:sparkles: Change password at first login (#650)
wlorenzetti Nov 13, 2023
b7c8443
:sparkles: Add support to qgis `'postgresraster'` layer (#652)
wlorenzetti Nov 14, 2023
a2d91ef
Fix get filter layer saved with anonymous user. (#654)
wlorenzetti Nov 14, 2023
b85e3ef
Typo
Nov 14, 2023
8ac9b33
Fix importing project with RelationReference widget
Nov 15, 2023
6260f05
Fix migrations.
Nov 15, 2023
524a63a
:arrow_up: Client:
volterra79 Nov 15, 2023
f6d32f8
:arrow_up: Client:
volterra79 Nov 15, 2023
3609ef4
:arrow_up: Client:
volterra79 Nov 15, 2023
90db595
:arrow_up: Client:
volterra79 Nov 16, 2023
347b7a6
:arrow_up: Client:
volterra79 Nov 16, 2023
8e982b7
Merge branch 'dev' into Set_geocoding_provider
Nov 17, 2023
da81786
:arrow_up: Client:
volterra79 Nov 17, 2023
536cb10
:arrow_up: Client:
volterra79 Nov 17, 2023
bfc45e4
:arrow_up: Client:
volterra79 Nov 20, 2023
d0fb196
:arrow_up: Client:
volterra79 Nov 20, 2023
33444f3
:arrow_up: Client:
volterra79 Nov 20, 2023
4395af0
mispelling
Raruto Nov 21, 2023
54c73c1
:arrow_up: Client:
volterra79 Nov 21, 2023
3cdd053
Remove 'Bing' from default value of 'GEOCODING_PROVIDERS' setting.
Nov 22, 2023
15be9ac
:arrow_up: Client:
volterra79 Nov 22, 2023
60dd474
:arrow_up: Client:
volterra79 Nov 22, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions g3w-admin/base/settings/base_geo_settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -46,3 +46,15 @@
}
}

# Geocoding providers
GEOCONDING_PROVIDERS = {
"nominatim": {
"label": "Nominatim (OSM)",
"url": "https://nominatim.openstreetmap.org/search"
},
"bing": {
"label": "Bing",
"url": "https://dev.virtualearth.net/REST/v1/LocalSearch/"
}
}

2 changes: 1 addition & 1 deletion g3w-admin/client/tests/test_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ def testGroupConfigApiView(self):
'extent': [-180.0, -90.0, 180.0, 90.0]
})
print(resp["group"]["mapcontrols"])
self.assertEqual(resp["group"]["mapcontrols"], ['zoom', 'zoombox', 'zoomtoextent', 'query', 'querybbox', 'querybypolygon', 'overview', 'scaleline', 'geolocation', 'streetview', 'nominatim', 'addlayers', 'length', 'area', 'mouseposition', 'scale'])
self.assertEqual(resp["group"]["mapcontrols"], {'zoom': {}, 'zoombox': {}, 'zoomtoextent': {}, 'query': {}, 'querybbox': {}, 'querybypolygon': {}, 'overview': {}, 'scaleline': {}, 'geolocation': {}, 'streetview': {}, 'nominatim': {}, 'addlayers': {}, 'length': {}, 'area': {}, 'mouseposition': {}, 'scale': {}})
self.assertEqual(resp["group"]["header_logo_img"], "logo_img/qgis-logo.png")
self.assertEqual(resp["group"]["name"], "Gruppo 1")
self.assertIsNone(resp["group"]["header_logo_link"])
Expand Down
20 changes: 17 additions & 3 deletions g3w-admin/core/api/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@

from qgis.core import QgsCoordinateReferenceSystem, QgsCoordinateTransform, QgsCoordinateTransformContext
from copy import copy
import json


def update_serializer_data(serializer_data, data):
Expand Down Expand Up @@ -116,9 +117,6 @@ def to_representation(self, instance):
'extent': extent
}

# map controls
ret['mapcontrols'] = [mapcontrol.name for mapcontrol in instance.mapcontrols.all()]

# add projects to group
ret['projects'] = []
self.projects = {}
Expand Down Expand Up @@ -258,6 +256,22 @@ def to_representation(self, instance):
if layout_right_panel:
ret['layout']['rightpanel'] = layout_right_panel

# Mapcontrols
ret['mapcontrols'] = {}
for mapcontrol in instance.mapcontrols.all():
options = {}
if mapcontrol.name in ('nominatim', 'geocoding') and self.project.geocoding_providers:
options = {'providers': {}}
for gp in json.loads(self.project.geocoding_providers):
if gp in settings.GEOCONDING_PROVIDERS:
options['providers'].update({
gp: settings.GEOCONDING_PROVIDERS[gp]
})

ret['mapcontrols'].update({
mapcontrol.name: options
})

return ret

class Meta:
Expand Down
16 changes: 16 additions & 0 deletions g3w-admin/qdjango/forms/projects.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,10 @@
from qdjango.models import *
from qdjango.utils.data import QgisProject
from qdjango.utils.validators import ProjectExists
from qdjango.utils.models import get_geocoding_providers

import shutil
import json


class QdjangoProjectFormMixin(object):
Expand Down Expand Up @@ -160,6 +162,8 @@ class QdjangoProjectForm(TranslationModelForm, QdjangoProjectFormMixin, G3WFormM

description = BleachField(required=False)

geocoding_providers = forms.MultipleChoiceField(choices=get_geocoding_providers,required=False)

def __init__(self, *args, **kwargs):

if 'instance' in kwargs and hasattr(kwargs['instance'], 'url_alias'):
Expand Down Expand Up @@ -268,6 +272,7 @@ def __init__(self, *args, **kwargs):
'multilayer_query',
'multilayer_querybybbox',
'multilayer_querybypolygon',
Field('geocoding_providers', css_class='select2', style="width:100%;"),
css_class='box-body',

),
Expand Down Expand Up @@ -321,12 +326,21 @@ class Meta:
'use_map_extent_as_init_extent',
'context_base_legend',
'title_ur',
'geocoding_providers'
)
field_classes = dict(
qgis_file=UploadedFileField,
thumbnail=UploadedFileField
)

def clean_geocoding_providers(self):
"""
Make the cleaned data for geocoding_providers:
make it a Json serializable
"""

return json.dumps(self.cleaned_data['geocoding_providers'])

def _setEditorUserQueryset(self):
"""
Set query set for editors chosen fields
Expand Down Expand Up @@ -383,6 +397,8 @@ def save(self, commit=True):

self._save_url_alias()



# add permission to Editor level 1 and 2 if current user is Editor level 1 or 2
if userHasGroups(self.request.user, [G3W_EDITOR1, G3W_EDITOR2]):
self.instance.addPermissionsToEditor(self.request.user)
Expand Down
26 changes: 26 additions & 0 deletions g3w-admin/qdjango/migrations/0115_project_geocoding_providers.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
# Generated by Django 3.2.23 on 2023-11-08 10:35

from django.db import migrations, models


class Migration(migrations.Migration):

dependencies = [
('qdjango', '0114_alter_sessiontokenfilter_token'),
]

operations = [
migrations.AddField(
model_name='project',
name='geocoding_providers',
field=models.TextField(blank=True, null=True, verbose_name='Geocoding providers'),
),

# Rename map control nominatim in geocoding
migrations.RunSQL(
"UPDATE core_mapcontrol SET name='geocoding' WHERE name='nominatim'"
),
migrations.RunSQL(
"UPDATE qdjango_project SET geocoding_providers='[\"nominatim\"]'"
),
]
1 change: 1 addition & 0 deletions g3w-admin/qdjango/models/projects.py
Original file line number Diff line number Diff line change
Expand Up @@ -278,6 +278,7 @@ class Project(G3WProjectMixins, G3WACLModelMixins, TimeStampedModel):

order = models.PositiveIntegerField(_('Fields to se order'), default=0, blank=True, null=True)

geocoding_providers = models.TextField(_('Geocoding providers'), blank=True, null=True)

class Meta:
verbose_name = _('Project')
Expand Down
17 changes: 16 additions & 1 deletion g3w-admin/qdjango/utils/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -163,4 +163,19 @@ def get_view_layer_ids(user, project):
set([l.qgs_layer_id for l in get_objects_for_user(get_anonymous_user(), 'qdjango.view_layer', Layer).
filter(project=project)])
)
)
)

def get_geocoding_providers():
"""
Return al tuple for qdjango project form field geocoding_providers

:return: tuple of items for a django form select
"""

ret = []
for gp, p in settings.GEOCONDING_PROVIDERS.items():
if gp == 'bing' and 'bing' not in settings.VENDOR_KEYS:
Comment on lines +177 to +178
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@wlorenzetti hiding it can become difficult to use (and discover), I would recommend showing an informational message instead, eg:

image

<div class="help-block" style="color: red">
  ⚠️ No <a href="https://g3w-suite.readthedocs.io/en/latest/settings.html#vendors-settings" target="_blank"><code>VENDOR_KEYS</code></a> found for "Bing" provider
</div>

Correct me if I'm wrong, only Nominatim provider will be activated by default for each project (so it wouldn't send invalid provider configurations to the client anyway):

image

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Raruto for me is correct not show Bing if a a vendor key for it is not set inside the settings. It could be useful add a description for the find int he form writing a thing like this: "For use Bing geoding is necessary have a Bing API key "

Correct me if I'm wrong, only Nominatim provider will be activated by default for each project (so it wouldn't send invalid provider configurations to the client anyway):

No why? at most you will have a map control without provider in the configuration API

Copy link
Contributor

@Raruto Raruto Nov 10, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hiding it can become difficult to use (and discover), I would recommend showing an informational message instead

for me is correct not show Bing if a a vendor key for it is not set inside the settings

I said that because it complicates development a bit.

API keys (and other server configurations) can also be added on the fly via JS.

If we also have to check the docker configuration it becomes an extra step (a "duplicated" step) which would require a dedicated documentation..

Even on the server side, a sysadmin might even be happy to simply do that:

GEOCONDING_PROVIDERS = {
  "bing": {
    "label": "Bing",
    "url": "https://dev.virtualearth.net/REST/v1/LocalSearch/?key=my.super.secret.key"
  }
}

which is equivalent to the wordy version:

VENDOR_KEYS = {
 'bing': 'my.super.secret.key'
}

GEOCONDING_PROVIDERS = {
  "bing": {
    "label": "Bing",
    "url": "https://dev.virtualearth.net/REST/v1/LocalSearch/"
  }
}

But without the risk to lose them in configuration files.

In essence, let's leave people a little more freedom on how to handle it.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Raruto ok I can understand what you are saying me, but the VENDORS_KEYS are used also for other features i.e. base layers .. so I think if better leave one place where to set the VENDOR key.

continue
ret.append((gp, p['label']))

return ret
17 changes: 17 additions & 0 deletions g3w-admin/qdjango/views/projects.py
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,14 @@ class QdjangoProjectCreateView(QdjangoProjectCUViewMixin, G3WGroupViewMixin, G3W
def dispatch(self, *args, **kwargs):
return super(QdjangoProjectCreateView, self).dispatch(*args, **kwargs)

def get_initial(self):
initial = super().get_initial()

# Get intial for geocoding_providers: default 'nominatim'
initial['geocoding_providers'] = ["nominatim"]

return initial


class QdjangoProjectUpdateView(QdjangoProjectCUViewMixin, G3WGroupViewMixin, G3WRequestViewMixin, G3WACLViewMixin,
UpdateView):
Expand All @@ -113,6 +121,15 @@ class QdjangoProjectUpdateView(QdjangoProjectCUViewMixin, G3WGroupViewMixin, G3W
def dispatch(self, *args, **kwargs):
return super(QdjangoProjectUpdateView, self).dispatch(*args, **kwargs)

def get_initial(self):
initial = super().get_initial()

# Get intial for geocoding_providers
if self.object.geocoding_providers:
initial['geocoding_providers'] = json.loads(self.object.geocoding_providers)

return initial

def get_context_data(self, **kwargs):
context = super(QdjangoProjectUpdateView,
self).get_context_data(**kwargs)
Expand Down
Loading