entrouvert/wcs
entrouvert
/
wcs
14
1
Fork 0
Code Issues Pull Requests 37 Releases Activity

settings: add geocoding base URL to UI settings (#71997) #847

Merged
fpeters merged 1 commits from wip/71997-geocoding-settings into main 2023-11-26 12:47:51 +01:00
Conversation 2 Commits 1 Files Changed 3 +70 -4
3 changed files with 70 additions and 4 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

28
tests/admin_pages/test_settings.py
View File

@ -920,6 +920,34 @@ def test_settings_geolocation(pub):
resp = resp.form.submit().follow()
assert pub.cfg['misc']['default-zoom-level'] == '16'
resp = resp.click('Geolocation')
assert resp.form['geocoding-services-base-url'].value == ''
resp = resp.form.submit().follow()
assert pub.get_geocoding_service_url() == 'https://nominatim.entrouvert.org/search'
if not pub.site_options.has_section('options'):
pub.site_options.add_section('options')
pub.site_options.set('options', 'nominatim_url', 'https://nominatim.example.org')
with open(os.path.join(pub.app_dir, 'site-options.cfg'), 'w') as fd:
pub.site_options.write(fd)
assert pub.get_geocoding_service_url() == 'https://nominatim.example.org/search'
resp = resp.click('Geolocation')
assert resp.form['geocoding-services-base-url'].value == 'https://nominatim.example.org'
resp.form['geocoding-services-base-url'] = 'https://nominatim.org'
resp = resp.form.submit().follow()
resp = resp.click('Geolocation')
assert resp.form['geocoding-services-base-url'].value == 'https://nominatim.org'
assert pub.get_geocoding_service_url() == 'https://nominatim.org/search'
resp = resp.form.submit().follow()
pub.site_options.set('options', 'reverse_geocoding_service_url', 'https://nominatim.example.org/reverse')
with open(os.path.join(pub.app_dir, 'site-options.cfg'), 'w') as fd:
pub.site_options.write(fd)
resp = resp.click('Geolocation')
assert 'System settings are currently forcing geocoding URLs' in resp.text
def test_settings_permissions(pub):
create_superuser(pub)

34
wcs/admin/settings.py
View File

@ -627,7 +627,7 @@ class SettingsDirectory(AccessControlled, Directory):
if enabled('geolocation'):
r += htmltext('<dt><a href="geolocation">%s</a></dt> <dd>%s</dd>') % (
_('Geolocation'),
_('Configure geolocation'),
_('Configure geolocation and geocoding'),
)
if enabled('submission-channels'):
r += htmltext('<dt><a href="submission-channels">%s</a></dt> <dd>%s</dd>') % (
@ -1154,6 +1154,34 @@ class SettingsDirectory(AccessControlled, Directory):
required=False,
)
has_system_settings = bool(
get_publisher().get_site_option('reverse_geocoding_service_url')
or get_publisher().get_site_option('geocoding_service_url')
)
if has_system_settings:
form.widgets.append(
HtmlWidget(
'<div class="warningnotice"><p>%s</p>'
tnoel marked this conversation as resolved Outdated
tnoel commented 2023-11-19 17:21:41 +01:00
Outdated
Review
Copy Link

Mais il y a les possibles « self.get_site_option('geocoding_service_url') » et « self.get_site_option('reverse_geocoding_service_url') » qui peuvent exister et feront que cette option ne sera pas prise en charge.

Je me dis que si une des options geocoding_service_url / reverse_geocoding_service_url / nominatim_url existe, on devrait ne pas proposer de paramétrage, juste afficher les options et dire qu'elles sont configurées statiquement. Charge à celui qui voit ça de prévenir un tech qu'il faut retirer les affaires de site-options.cfg pour permettre une configuration correcte via l'UI.

Mais il y a les possibles « self.get_site_option('geocoding_service_url') » et « self.get_site_option('reverse_geocoding_service_url') » qui peuvent exister et feront que cette option ne sera pas prise en charge. Je me dis que si une des options geocoding_service_url / reverse_geocoding_service_url / nominatim_url existe, on devrait ne pas proposer de paramétrage, juste afficher les options et dire qu'elles sont configurées statiquement. Charge à celui qui voit ça de prévenir un tech qu'il faut retirer les affaires de site-options.cfg pour permettre une configuration correcte via l'UI.
fpeters commented 2023-11-19 17:34:03 +01:00
Outdated
Review
Copy Link

Oui mais geocoding_service_url/reverse_geocoding_service_url ne sont jamais utilisées (d'un grep rapide), donc j'ai préféré faire sans. Oui je pourrais ajouter un message / désactiver le champ si elles sont définies (édit : fait), par contre (réponse au point suivant) je préfère garder la priorité à ce qui est défini dans l'UI, ça fluidifie la transition.

Oui mais geocoding_service_url/reverse_geocoding_service_url ne sont jamais utilisées (d'un grep rapide), donc j'ai préféré faire sans. Oui je pourrais ajouter un message / désactiver le champ si elles sont définies (édit : fait), par contre (réponse au point suivant) je préfère garder la priorité à ce qui est défini dans l'UI, ça fluidifie la transition.
fpeters commented 2023-11-20 09:31:30 +01:00
Outdated
Review
Copy Link

priorité à ce qui est défini dans l'UI

Et pour un argument supplémentiare, c'est aussi ce qui est fait pour l'autre paramétrage de cette page, la position par défaut :

    def get_default_position(self):
        default_position = self.cfg.get('misc', {}).get('default-position', None)
        if not default_position:
            default_position = self.get_site_option('default_position')
        if not default_position:
            default_position = '50.84;4.36'
        return default_position
> priorité à ce qui est défini dans l'UI Et pour un argument supplémentiare, c'est aussi ce qui est fait pour l'autre paramétrage de cette page, la position par défaut : ``` def get_default_position(self): default_position = self.cfg.get('misc', {}).get('default-position', None) if not default_position: default_position = self.get_site_option('default_position') if not default_position: default_position = '50.84;4.36' return default_position ```
% _(
'System settings are currently forcing geocoding URLs, '
'this parameter won\'t have any effect.'
)
)
)
form.add(
StringWidget,
'geocoding-services-base-url',
title=_('Geocoding services base URL'),
value=misc_cfg.get('geocoding-services-base-url')
or get_publisher().get_site_option('nominatim_url'),
required=False,
hint=_('It will be suffixed by /search for geocoding and /reverse for reverse-geocoding.'),
)
if has_system_settings:
form.widgets.append(HtmlWidget('</div>'))
form.add_submit('submit', _('Submit'))
form.add_submit('cancel', _('Cancel'))
@ -1161,7 +1189,9 @@ class SettingsDirectory(AccessControlled, Directory):
return redirect('.')
if form.is_submitted() and not form.has_errors():
cfg_submit(form, 'misc', ['default-position', 'default-zoom-level'])
cfg_submit(
form, 'misc', ['default-position', 'default-zoom-level', 'geocoding-services-base-url']
)
return redirect('.')
get_response().breadcrumb.append(('geolocation', _('Geolocation Settings')))

12
wcs/qommon/publisher.py
View File

@ -835,7 +835,11 @@ class QommonPublisher(Publisher):
url = self.get_site_option('reverse_geocoding_service_url')
if url:
return url
url = self.get_site_option('nominatim_url') or 'https://nominatim.entrouvert.org'
url = (
get_cfg('misc', {}).get('geocoding-services-base-url')
or self.get_site_option('nominatim_url')
tnoel marked this conversation as resolved Outdated
tnoel commented 2023-11-19 17:23:03 +01:00
Outdated
Review
Copy Link

Par rapport à ce que je dis plus haut, on mettrait plutôt le get_site_option en premier ; s'il existe il ne peut pas y avoir de surcharge par l'interface des settings.

Par rapport à ce que je dis plus haut, on mettrait plutôt le get_site_option en premier ; s'il existe il ne peut pas y avoir de surcharge par l'interface des settings.
or 'https://nominatim.entrouvert.org'
)
url += '/reverse'
reverse_zoom_level = self.get_site_option('nominatim_reverse_zoom_level') or 18
url += '?zoom=%s' % reverse_zoom_level
@ -848,7 +852,11 @@ class QommonPublisher(Publisher):
url = self.get_site_option('geocoding_service_url')
if url:
return url
url = self.get_site_option('nominatim_url') or 'https://nominatim.entrouvert.org'
url = (
get_cfg('misc', {}).get('geocoding-services-base-url')
or self.get_site_option('nominatim_url')
tnoel marked this conversation as resolved Outdated
tnoel commented 2023-11-19 17:23:09 +01:00
Outdated
Review
Copy Link

Idem.

Idem.
or 'https://nominatim.entrouvert.org'
)
url += '/search'
key = self.get_site_option('nominatim_key')
if key: