From ac1cf3b5e8333ca503f4ef9819d89d34150357dd 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 --- .../test_base_location_geonames_import.py | 35 ++++++++++++++++++- .../wizard/geonames_import.py | 30 +++++++++------- 2 files changed, 51 insertions(+), 14 deletions(-) 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 d441b2e953d..5cf951a1df3 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 @@ 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), + ) diff --git a/base_location_geonames_import/wizard/geonames_import.py b/base_location_geonames_import/wizard/geonames_import.py index 03e7e523376..5328173d901 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 @@ 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()