From 799b54896a54492ac4ce55f419d71ccd9fd4a32f Mon Sep 17 00:00:00 2001 From: "Pedro M. Baeza" Date: Mon, 29 Jul 2019 12:25:50 +0200 Subject: [PATCH] [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 #749 --- base_location_geonames_import/__manifest__.py | 2 +- .../test_base_location_geonames_import.py | 35 ++++++++++++++++++- .../wizard/geonames_import.py | 30 +++++++++------- 3 files changed, 52 insertions(+), 15 deletions(-) diff --git a/base_location_geonames_import/__manifest__.py b/base_location_geonames_import/__manifest__.py index 441a6982f..a8fd04766 100644 --- a/base_location_geonames_import/__manifest__.py +++ b/base_location_geonames_import/__manifest__.py @@ -8,7 +8,7 @@ { 'name': 'Base Location Geonames Import', - 'version': '12.0.1.0.0', + 'version': '12.0.1.0.1', 'category': 'Partner Management', 'license': 'AGPL-3', 'summary': 'Import zip entries from Geonames', diff --git a/base_location_geonames_import/tests/test_base_location_geonames_import.py b/base_location_geonames_import/tests/test_base_location_geonames_import.py index d441b2e95..5cf951a1d 100644 --- a/base_location_geonames_import/tests/test_base_location_geonames_import.py +++ b/base_location_geonames_import/tests/test_base_location_geonames_import.py @@ -1,4 +1,4 @@ -# Copyright 2016 Pedro M. Baeza +# Copyright 2016-2019 Pedro M. Baeza # License AGPL-3.0 or later (https://www.gnu.org/licenses/agpl.html). from odoo.tests import common @@ -109,3 +109,36 @@ class TestBaseLocationGeonamesImport(common.SavepointCase): 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), + ) diff --git a/base_location_geonames_import/wizard/geonames_import.py b/base_location_geonames_import/wizard/geonames_import.py index 03e7e5233..5328173d9 100644 --- a/base_location_geonames_import/wizard/geonames_import.py +++ b/base_location_geonames_import/wizard/geonames_import.py @@ -1,10 +1,10 @@ # Copyright 2014-2016 Akretion (Alexis de Lattre # ) # Copyright 2014 Lorenzo Battistini -# Copyright 2016 Pedro M. Baeza # Copyright 2017 Eficent Business and IT Consulting Services, S.L. # # Copyright 2018 Aitor Bouzas +# 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 @@ class CityZipGeonamesImport(models.TransientModel): 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 @@ class CityZipGeonamesImport(models.TransientModel): [('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 @@ class CityZipGeonamesImport(models.TransientModel): 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 @@ class CityZipGeonamesImport(models.TransientModel): # 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()