[FIX] base_location_geonames_import: Handle duplicated city names

If there are several cities with the same name in different states, previous code doesn't
handle correctly this situation.

We amend this storing in the cached dictionary both city name and state.

Includes a test for checking this specific condition, got from real data in US.

Fixes OCA#749

base_location_geonames_import/tests/test_base_location_geonames_import.py

@@ -1,4 +1,4 @@
-# Copyright 2016 Pedro M. Baeza <pedro.baeza@tecnativa.com>
+# Copyright 2016-2019 Pedro M. Baeza <pedro.baeza@tecnativa.com>
 # License AGPL-3.0 or later (https://www.gnu.org/licenses/agpl.html).
 
 from odoo.tests import common
@@ -109,3 +109,36 @@ def test_download_error(self):
         with a wrong country code"""
         with self.assertRaises(UserError):
             self.wrong_wizard.run_import()
+
+    def test_import_duplicated_city_name(self):
+        country = self.env.ref('base.us')
+        self.wizard.country_id = country.id
+        parsed_csv = [
+            ['US', '95602', 'Auburn', ' California', 'CA', 'Placer', '61',
+             '38.9829', '-121.0944', '4'],
+            ['US', '95603', 'Auburn', ' California', 'CA', 'Placer', '61',
+             '38.9115', '-121.08', '4'],
+            ['US', '30011', 'Auburn', ' Georgia', 'GA', 'Barrow', '13',
+             '34.0191', '-83.8261', '4'],
+        ]
+        self.wizard._process_csv(parsed_csv)
+        cities = self.env['res.city'].search([('name', '=', 'Auburn')])
+        self.assertEqual(len(cities), 2)
+        mapping = [
+            ['California', '95602'],
+            ['California', '95603'],
+            ['Georgia', '30011'],
+        ]
+        for state_name, zip_code in mapping:
+            zip_entry = self.env['res.city.zip'].search([
+                ('city_id.country_id', '=', country.id),
+                ('name', '=', zip_code),
+            ])
+            state = self.env['res.country.state'].search([
+                ('country_id', '=', country.id),
+                ('name', '=', state_name),
+            ])
+            self.assertEqual(
+                zip_entry.city_id.state_id, state,
+                "Incorrect state for %s %s" % (state_name, zip_code),
+            )

base_location_geonames_import/wizard/geonames_import.py

@@ -1,10 +1,10 @@
 # Copyright 2014-2016 Akretion (Alexis de Lattre
 #                     <alexis.delattre@akretion.com>)
 # Copyright 2014 Lorenzo Battistini <lorenzo.battistini@agilebg.com>
-# Copyright 2016 Pedro M. Baeza <pedro.baeza@tecnativa.com>
 # Copyright 2017 Eficent Business and IT Consulting Services, S.L.
 #                <contact@eficent.com>
 # Copyright 2018 Aitor Bouzas <aitor.bouzas@adaptivecity.com>
+# Copyright 2016-2019 Tecnativa - Pedro M. Baeza
 # License AGPL-3.0 or later (https://www.gnu.org/licenses/agpl.html).
 
 from odoo import _, api, fields, models
@@ -168,26 +168,28 @@ def _create_cities(self, parsed_csv,
         for i, row in enumerate(parsed_csv):
             if max_import and i == max_import:
                 break
+            state_id = state_dict[row[self.code_row_index or 4]]
             city = self.select_city(
                 row, self.country_id) if search_cities else False
             if not city:
-                state_id = state_dict[
-                    row[self.code_row_index or 4]]
                 city_vals = self.prepare_city(
                     row, self.country_id, state_id)
                 if city_vals not in city_vals_list:
                     city_vals_list.append(city_vals)
             else:
-                city_dict[city.name] = city.id
-
+                city_dict[(city.name, state_id)] = city.id
         created_cities = self.env['res.city'].create(city_vals_list)
         for i, vals in enumerate(city_vals_list):
-            city_dict[vals['name']] = created_cities[i].id
+            city_dict[(vals['name'], vals['state_id'])] = created_cities[i].id
         return city_dict
 
     @api.multi
     def run_import(self):
         self.ensure_one()
+        parsed_csv = self.get_and_parse_csv()
+        return self._process_csv(parsed_csv)
+
+    def _process_csv(self, parsed_csv):
         state_model = self.env['res.country.state']
         zip_model = self.env['res.city.zip']
         res_city_model = self.env['res.city']
@@ -203,7 +205,6 @@ def run_import(self):
             [('country_id', '=', self.country_id.id)])
         search_states = True and len(current_states) > 0 or False
 
-        parsed_csv = self.get_and_parse_csv()
         max_import = self.env.context.get('max_import', 0)
         logger.info('Starting to create the cities and/or city zip entries')
 
@@ -218,12 +219,15 @@ def run_import(self):
             if max_import and i == max_import:
                 break
             # Don't search if there aren't any records
-            zip = False
+            zip_code = False
             if search_zips:
-                zip = self.select_zip(row, self.country_id)
-            if not zip:
-                city_id = city_dict[
-                    self.transform_city_name(row[2], self.country_id)]
+                zip_code = self.select_zip(row, self.country_id)
+            if not zip_code:
+                state_id = state_dict[row[self.code_row_index or 4]]
+                city_id = city_dict[(
+                    self.transform_city_name(row[2], self.country_id),
+                    state_id,
+                )]
                 zip_vals = self.prepare_zip(row, city_id)
                 if zip_vals not in zip_vals_list:
                     zip_vals_list.append(zip_vals)
@@ -243,7 +247,7 @@ def run_import(self):
             # we can delete the old ones
             created_cities = res_city_model.search(
                 [('country_id', '=', self.country_id.id),
-                 ('id', 'in', [value for key, value in city_dict.items()])]
+                 ('id', 'in', list(city_dict.values()))]
             )
             current_cities -= created_cities
             current_cities.unlink()