[REF] base_geolocalize, base_setup: ref + make it configurable in settings

Migrate in v12 and Refactor a part of the code
Allow to configure it in setting

10 files changed, 141 insertions, 72 deletions

diff --git a/addons/base_geolocalize/README.md b/addons/base_geolocalize/README.md
index 7909b1a4285..90d54f5c726 100644
--- a/addons/base_geolocalize/README.md
+++ b/addons/base_geolocalize/README.md
@@ -5,18 +5,9 @@ Contacts geolocation API to convert partner addresses into GPS coordinates.
 
 Configure
 ---------
-You can add a system parameter to change the default provider of the geolocation API service.
-
-* `base_geolocalize.provider = <service>`
+You can configure in General Settings the default provider of the geolocation API service.
 
 A method `_call_<service>` should be implemented in object `base.geocoder` that accepts an address string as parameter and return (latitude, longitude) tuple for this to work.
-If no parameter is set, Openstreetmap will be used by default.
-
-An optional method `_geo_query_address_<service>` which takes address fields as parameters can be defined to encode the query string for the provider.
-
-Google Places
--------------
-You can use Google Maps API if you have a valid apikey. In that case you should add the following system parameters:
+If no default provider is set, the first one will be used by default.
 
-* `base_geolocalize.provider = google`
-* `google.api_key_geocode = <your_api_key>`
+An optional method `_geo_query_address_<service>` which takes address fields as parameters can be defined to encode the query string for the provider.
\ No newline at end of file
diff --git a/addons/base_geolocalize/__manifest__.py b/addons/base_geolocalize/__manifest__.py
index d2c4ab38ad1..a5ed52e1df3 100644
--- a/addons/base_geolocalize/__manifest__.py
+++ b/addons/base_geolocalize/__manifest__.py
@@ -3,14 +3,17 @@
 {
     'name': 'Partners Geolocation',
     'version': '2.1',
-    'category': 'Sales',
+    'category': 'Extra Tools',
     'description': """
 Partners Geolocation
 ========================
     """,
-    'depends': ['base'],
+    'depends': ['base_setup'],
     'data': [
+        'security/ir.model.access.csv',
         'views/res_partner_views.xml',
+        'views/res_config_settings_views.xml',
+        'data/data.xml',
     ],
     'installable': True,
 }
diff --git a/addons/base_geolocalize/data/data.xml b/addons/base_geolocalize/data/data.xml
new file mode 100644
index 00000000000..dd2dec16505
--- /dev/null
+++ b/addons/base_geolocalize/data/data.xml
@@ -0,0 +1,12 @@
+<?xml version="1.0" encoding="utf-8"?>
+<odoo>
+    <record id="geoprovider_open_street" model="base.geo_provider">
+        <field name="tech_name">openstreetmap</field>
+        <field name="name">Open Street Map</field>
+    </record>
+
+    <record id="geoprovider_google_map" model="base.geo_provider">
+        <field name="tech_name">googlemap</field>
+        <field name="name">Google Place Map</field>
+    </record>
+</odoo>
diff --git a/addons/base_geolocalize/models/__init__.py b/addons/base_geolocalize/models/__init__.py
index bd7b8ee21e5..47cc19bfc62 100644
--- a/addons/base_geolocalize/models/__init__.py
+++ b/addons/base_geolocalize/models/__init__.py
@@ -1,4 +1,5 @@
 # -*- coding: utf-8 -*-
 # Part of Odoo. See LICENSE file for full copyright and licensing details.
 from . import base_geocoder
+from . import res_config_settings
 from . import res_partner
diff --git a/addons/base_geolocalize/models/base_geocoder.py b/addons/base_geolocalize/models/base_geocoder.py
index 87751cd400d..ee54fcf6dbd 100644
--- a/addons/base_geolocalize/models/base_geocoder.py
+++ b/addons/base_geolocalize/models/base_geocoder.py
@@ -1,22 +1,39 @@
 # -*- coding: utf-8 -*-
 # Part of Odoo. See LICENSE file for full copyright and licensing details.
-import json
-import urllib2
+import requests
 import logging
 
-from odoo import api, models, tools, _
+from odoo import api, fields, models, tools, _
 from odoo.exceptions import UserError
 
 
 _logger = logging.getLogger(__name__)
 
 
+class GeoProvider(models.Model):
+    _name = "base.geo_provider"
+    _description = "Geo Provider"
+
+    tech_name = fields.Char()
+    name = fields.Char()
+
+
 class GeoCoder(models.AbstractModel):
     """
     Abstract class used to call Geolocalization API and convert addresses
     into GPS coordinates.
     """
     _name = "base.geocoder"
+    _description = "Geo Coder"
+
+    @api.model
+    def _get_provider(self):
+        prov_id = self.env['ir.config_parameter'].sudo().get_param('base_geolocalize.geo_provider')
+        if prov_id:
+            provider = self.env['base.geo_provider'].browse(int(prov_id))
+        if not prov_id or not provider.exists():
+            provider = self.env['base.geo_provider'].search([], limit=1)
+        return provider
 
     @api.model
     def geo_query_address(self, street=None, zip=None, city=None, state=None, country=None):
@@ -29,18 +46,13 @@ class GeoCoder(models.AbstractModel):
         :param country: country
         :return: formatted string
         """
-        provider = self.env['ir.config_parameter'].get_param('base_geolocalize.provider', 'openstreetmap')
+        provider = self._get_provider().tech_name
         if hasattr(self, '_geo_query_address_' + provider):
             # Makes the transformation defined for provider
-            service = getattr(self, '_geo_query_address_' + provider)
-            return service(street, zip, city, state, country)
+            return getattr(self, '_geo_query_address_' + provider)(street, zip, city, state, country)
         else:
             # By default, join the non-empty parameters
-            return tools.ustr(', '.join(filter(None, [
-                street,
-                ("%s %s" % (zip or '', city or '')).strip(),
-                state,
-                country])))
+            return self._geo_query_address_default(street=street, zip=zip, city=city, state=state, country=country)
 
     @api.model
     def geo_find(self, addr):
@@ -49,8 +61,7 @@ class GeoCoder(models.AbstractModel):
         :param addr: Address string passed to API
         :return: (latitude, longitude) or None if not found
         """
-        provider = self.env['ir.config_parameter'].get_param(
-            'base_geolocalize.provider', 'openstreetmap')
+        provider = self._get_provider().tech_name
         try:
             service = getattr(self, '_call_' + provider)
             result = service(addr)
@@ -60,7 +71,7 @@ class GeoCoder(models.AbstractModel):
             ) % provider)
         except UserError:
             raise
-        except:
+        except Exception:
             _logger.debug('Geolocalize call failed', exc_info=True)
             result = None
         return result
@@ -74,34 +85,31 @@ class GeoCoder(models.AbstractModel):
         if not addr:
             _logger.info('invalid address given')
             return None
-        url = 'https://nominatim.openstreetmap.org/search?format=json&q='
-        url += urllib2.quote(addr.encode('utf8'))
+        url = 'https://nominatim.openstreetmap.org/search'
         try:
+            result = requests.get(url, {'format': 'json', 'q': addr}).json()
             _logger.info('openstreetmap nominatim service called')
-            result = json.load(urllib2.urlopen(url))
         except Exception as e:
-            self._raise_internet_access_error(e)
+            self._raise_query_error(e)
         geo = result[0]
         return float(geo['lat']), float(geo['lon'])
 
     @api.model
-    def _call_google(self, addr):
+    def _call_googlemap(self, addr):
         """ Use google maps API. It won't work without a valid API key.
         :return: (latitude, longitude) or None if not found
         """
-        apikey = self.env['ir.config_parameter'].sudo().get_param(
-            'google.api_key_geocode')
+        apikey = self.env['ir.config_parameter'].sudo().get_param('base_geolocalize.google_map_api_key')
         if not apikey:
             raise UserError(_(
                 "API key for GeoCoding (Places) required.\n"
-                "Save this key in System Parameters with key: google.api_key_geocode, value: <your api key> Visit https://developers.google.com/maps/documentation/geocoding/get-api-key for more information."
+                "Visit https://developers.google.com/maps/documentation/geocoding/get-api-key for more information."
             ))
-        url = 'https://maps.googleapis.com/maps/api/geocode/json?key=%s&sensor=false&address=' % apikey
-        url += urllib2.quote(addr.encode('utf8'))
+        url = "https://maps.googleapis.com/maps/api/geocode/json"
         try:
-            result = json.load(urllib2.urlopen(url))
+            result = requests.get(url, params={'sensor': 'false', 'address': addr, 'key': apikey}).json()
         except Exception as e:
-            self._raise_internet_access_error(e)
+            self._raise_query_error(e)
 
         try:
             if result['status'] != 'OK':
@@ -115,22 +123,25 @@ class GeoCoder(models.AbstractModel):
             return None
 
     @api.model
-    def _geo_query_address_google(self, street=None, zip=None, city=None,
-                                   state=None, country=None):
-        # This may be useful if using GMaps API.
-        # put country qualifier in front, otherwise GMap gives wrong
-        # results, e.g. 'Congo, Democratic Republic of the' =>
-        # 'Democratic Republic of the Congo'
-        if country and ',' in country and (
-                country.endswith(' of') or country.endswith(' of the')):
-            country = '{1} {0}'.format(*country.split(',', 1))
-        return tools.ustr(', '.join(filter(None, [
+    def _geo_query_address_default(self, street=None, zip=None, city=None, state=None, country=None):
+        address_list = [
             street,
             ("%s %s" % (zip or '', city or '')).strip(),
             state,
-            country])))
+            country
+        ]
+        address_list = [item for item in address_list if item]
+        return tools.ustr(', '.join(address_list))
+
+    @api.model
+    def _geo_query_address_googlemap(self, street=None, zip=None, city=None, state=None, country=None):
+        # put country qualifier in front, otherwise GMap gives wrong# results
+        #  e.g. 'Congo, Democratic Republic of the' =>  'Democratic Republic of the Congo'
+        if country and ',' in country and (
+                country.endswith(' of') or country.endswith(' of the')):
+            country = '{1} {0}'.format(*country.split(',', 1))
+        return self._geo_query_address_default(street=street, zip=zip, city=city, state=state, country=country)
+
+    def _raise_query_error(self, error):
+        raise UserError(_('Error with geolocation server:') + ' %s' % error)
 
-    def _raise_internet_access_error(self, error):
-        raise UserError(_(
-            'Cannot contact geolocation servers. Please make sure that your Internet connection is up and running (%s).'
-        ) % error)
diff --git a/addons/base_geolocalize/models/res_config_settings.py b/addons/base_geolocalize/models/res_config_settings.py
new file mode 100644
index 00000000000..4cd6cdb87da
--- /dev/null
+++ b/addons/base_geolocalize/models/res_config_settings.py
@@ -0,0 +1,21 @@
+# -*- coding: utf-8 -*-
+# Part of Odoo. See LICENSE file for full copyright and licensing details.
+
+from odoo import fields, models
+
+
+class ResConfigSettings(models.TransientModel):
+    _inherit = 'res.config.settings'
+
+    geoloc_provider_id = fields.Many2one(
+        'base.geo_provider',
+        string='API',
+        config_parameter='base_geolocalize.geo_provider',
+        default=lambda x: x.env['base.geocoder']._get_provider()
+    )
+    geoloc_provider_techname = fields.Char(related='geoloc_provider_id.tech_name', readonly=1)
+    geoloc_provider_googlemap_key = fields.Char(
+        string='Google Map API Key',
+        config_parameter='base_geolocalize.google_map_api_key',
+        help="Visit https://developers.google.com/maps/documentation/geocoding/get-api-key for more information."
+    )
diff --git a/addons/base_geolocalize/models/res_partner.py b/addons/base_geolocalize/models/res_partner.py
index bc1185d1b19..6c7cbad5aac 100644
--- a/addons/base_geolocalize/models/res_partner.py
+++ b/addons/base_geolocalize/models/res_partner.py
@@ -8,24 +8,25 @@ class ResPartner(models.Model):
     partner_longitude = fields.Float(string='Geo Longitude', digits=(16, 5))
     date_localization = fields.Date(string='Geolocation Date')
 
+    @api.model
+    def _geo_localize(self, street='', zip='', city='', state='', country=''):
+        geo_obj = self.env['base.geocoder']
+        search = geo_obj.geo_query_address(street=street, zip=zip, city=city, state=state, country=country)
+        result = geo_obj.geo_find(search)
+        if result is None:
+            search = geo_obj.geo_query_address(city=city, state=state, country=country)
+            result = geo_obj.geo_find(search)
+        return result
+
     @api.multi
     def geo_localize(self):
         # We need country names in English below
-        geo_obj = self.env['base.geocoder']
         for partner in self.with_context(lang='en_US'):
-            result = geo_obj.geo_find(geo_obj.geo_query_address(
-                street=partner.street,
-                zip=partner.zip,
-                city=partner.city,
-                state=partner.state_id.name,
-                country=partner.country_id.name
-            ))
-            if result is None:
-                result = geo_obj.geo_find(geo_obj.geo_query_address(
-                    city=partner.city,
-                    state=partner.state_id.name,
-                    country=partner.country_id.name
-                ))
+            result = self._geo_localize(partner.street,
+                                        partner.zip,
+                                        partner.city,
+                                        partner.state_id.name,
+                                        partner.country_id.name)
 
             if result:
                 partner.write({
diff --git a/addons/base_geolocalize/security/ir.model.access.csv b/addons/base_geolocalize/security/ir.model.access.csv
new file mode 100644
index 00000000000..b6ad51578ce
--- /dev/null
+++ b/addons/base_geolocalize/security/ir.model.access.csv
@@ -0,0 +1,2 @@
+id,name,model_id:id,group_id:id,perm_read,perm_write,perm_create,perm_unlink

+access_base_geo_provider,access_base_geo_provider,model_base_geo_provider,base.group_user,1,0,0,0

diff --git a/addons/base_geolocalize/tests/test_geolocalize.py b/addons/base_geolocalize/tests/test_geolocalize.py
index 3bc7cb54705..8052e49f3b4 100644
--- a/addons/base_geolocalize/tests/test_geolocalize.py
+++ b/addons/base_geolocalize/tests/test_geolocalize.py
@@ -3,7 +3,10 @@
 from odoo.tests import TransactionCase
 from odoo.exceptions import UserError
 
+import odoo.tests
 
+
+@odoo.tests.tagged('external', '-standard')
 class TestGeoLocalize(TransactionCase):
 
     def test_default_openstreetmap(self):
@@ -14,12 +17,17 @@ class TestGeoLocalize(TransactionCase):
         self.assertTrue(test_partner.partner_latitude)
         self.assertTrue(test_partner.date_localization)
 
-    def test_google_without_api_key(self):
+        # we don't check here that the localization is at right place
+        # but just that result is realistic float coordonates
+        self.assertTrue(float(test_partner.partner_longitude) != 0.0)
+        self.assertTrue(float(test_partner.partner_latitude) != 0.0)
+
+    def test_googlemap_without_api_key(self):
         """ Without providing API key to google maps,
         the service doesn't work."""
         test_partner = self.env.ref('base.res_partner_address_4')
-        self.env['ir.config_parameter'].set_param('base_geolocalize.provider',
-                                                  'google')
+        google_map = self.env.ref('base_geolocalize.geoprovider_google_map').id
+        self.env['ir.config_parameter'].set_param('base_geolocalize.geo_provider', google_map)
         with self.assertRaises(UserError):
             test_partner.geo_localize()
         self.assertFalse(test_partner.partner_longitude)
diff --git a/addons/base_geolocalize/views/res_config_settings_views.xml b/addons/base_geolocalize/views/res_config_settings_views.xml
new file mode 100644
index 00000000000..68eb238e4bf
--- /dev/null
+++ b/addons/base_geolocalize/views/res_config_settings_views.xml
@@ -0,0 +1,19 @@
+<?xml version="1.0" encoding="utf-8"?>
+<odoo>
+    <record id="res_config_settings_view_form" model="ir.ui.view">
+        <field name="name">res.config.settings.view.form.inherit.web.geoloclaize</field>
+        <field name="model">res.config.settings</field>
+        <field name="inherit_id" ref="base_setup.res_config_settings_view_form"/>
+        <field name="arch" type="xml">
+            <xpath expr="//div[@name='base_geolocalize_warning']" position="replace">
+                <div class="content-group" attrs="{'invisible': [('module_base_geolocalize', '=', False)]}">
+                    API: <field name="geoloc_provider_id"/>
+                    <field name="geoloc_provider_techname" invisible="1"/>
+                    <div attrs="{'invisible': [('geoloc_provider_techname', '!=', 'googlemap')]}">
+                        Key: <field name='geoloc_provider_googlemap_key'/>
+                    </div>
+                </div>
+            </xpath>
+        </field>
+    </record>
+</odoo>
\ No newline at end of file