From 89d687d4625a00df90766ca48476dcacb245f250 Mon Sep 17 00:00:00 2001
From: "Pedro M. Baeza" <pedro.baeza@gmail.com>
Date: Sat, 31 Dec 2016 00:56:06 +0100
Subject: [PATCH] [FIX+IMP] base_location_geonames_import

* Tests
* New menu location
* Wizard options fixed

[IMP] base_location_geonames_import

* Speed improvement applying cache for not doing duplicated searches
  over states.
* Tests change to SavepointCase, which only passes over setUp one time.
---
 .../test_base_location_geonames_import.py     | 31 ++++++++++----
 .../wizard/geonames_import.py                 | 42 ++++++++++---------
 .../wizard/geonames_import_view.xml           | 10 +++--
 3 files changed, 52 insertions(+), 31 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 3015f5c15..303e6ad67 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
@@ -5,12 +5,13 @@
 from odoo.tests import common
 
 
-class TestBaseLocationGeonamesImport(common.TransactionCase):
-    def setUp(self):
-        super(TestBaseLocationGeonamesImport, self).setUp()
-        self.country = self.env.ref('base.mc')
-        self.wizard = self.env['better.zip.geonames.import'].create({
-            'country_id': self.country.id,
+class TestBaseLocationGeonamesImport(common.SavepointCase):
+    @classmethod
+    def setUpClass(cls):
+        super(TestBaseLocationGeonamesImport, cls).setUpClass()
+        cls.country = cls.env.ref('base.mc')
+        cls.wizard = cls.env['better.zip.geonames.import'].create({
+            'country_id': cls.country.id,
         })
 
     def test_import_country(self):
@@ -21,7 +22,7 @@ class TestBaseLocationGeonamesImport(common.TransactionCase):
             ('country_id', '=', self.country.id)
         ])
         self.assertTrue(state_count)
-        # Look if the are imported zips
+        # Look if there are imported zips
         zip_count = self.env['res.better.zip'].search_count([
             ('country_id', '=', self.country.id)
         ])
@@ -44,3 +45,19 @@ class TestBaseLocationGeonamesImport(common.TransactionCase):
         })
         self.wizard.run_import()
         self.assertFalse(zip_entry.exists())
+
+    def test_import_title(self):
+        self.wizard.letter_case = 'title'
+        self.wizard.with_context(max_import=1).run_import()
+        zip = self.env['res.better.zip'].search(
+            [('country_id', '=', self.country.id)], limit=1
+        )
+        self.assertEqual(zip.city, zip.city.title())
+
+    def test_import_upper(self):
+        self.wizard.letter_case = 'upper'
+        self.wizard.with_context(max_import=1).run_import()
+        zip = self.env['res.better.zip'].search(
+            [('country_id', '=', self.country.id)], limit=1
+        )
+        self.assertEqual(zip.city, zip.city.upper())
diff --git a/base_location_geonames_import/wizard/geonames_import.py b/base_location_geonames_import/wizard/geonames_import.py
index 6eb2a922b..c3c3e1266 100644
--- a/base_location_geonames_import/wizard/geonames_import.py
+++ b/base_location_geonames_import/wizard/geonames_import.py
@@ -4,7 +4,7 @@
 # © 2016 Pedro M. Baeza <pedro.baeza@serviciosbaeza.com>
 # License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl.html).
 
-from odoo import models, fields, api, _
+from odoo import _, api, fields, models, tools
 from odoo.exceptions import UserError
 import requests
 import tempfile
@@ -70,10 +70,10 @@ class BetterZipGeonamesImport(models.TransientModel):
                     "correspond to the selected country (%s).")
                 % (row[0], country.code))
         logger.debug('ZIP = %s - City = %s' % (row[1], row[2]))
-        if self.letter_case.title:
+        if self.letter_case == 'title':
             row[2] = row[2].title()
             row[3] = row[3].title()
-        elif self.letter_case.upper:
+        elif self.letter_case == 'upper':
             row[2] = row[2].upper()
             row[3] = row[3].upper()
         if row[1] and row[2]:
@@ -86,28 +86,30 @@ class BetterZipGeonamesImport(models.TransientModel):
                 vals = self._prepare_better_zip(row, country)
                 if vals:
                     return zip_model.create(vals)
-        else:
+        else:  # pragma: no cover
             return False
 
+    @tools.ormcache('country_id', 'code')
+    def _get_state(self, country_id, code, name):
+        state = self.env['res.country.state'].search(
+            [('country_id', '=', country_id),
+             ('code', '=', code)], limit=1,
+        )
+        if state:  # pragma: no cover
+            return state
+        else:
+            return self.env['res.country.state'].create({
+                'name': name,
+                'code': code,
+                'country_id': country_id,
+            })
+
     @api.model
     def select_or_create_state(
             self, row, country, code_row_index=4, name_row_index=3):
-        states = self.env['res.country.state'].search([
-            ('country_id', '=', country.id),
-            ('code', '=', row[code_row_index]),
-            ])
-        if len(states) > 1:
-            raise UserError(
-                _("Too many states with code %s for country %s")
-                % (row[code_row_index], country.code))
-        if len(states) == 1:
-            return states[0]
-        else:
-            return self.env['res.country.state'].create({
-                'name': row[name_row_index],
-                'code': row[code_row_index],
-                'country_id': country.id
-                })
+        return self._get_state(
+            country.id, row[code_row_index], row[name_row_index],
+        )
 
     @api.multi
     def run_import(self):
diff --git a/base_location_geonames_import/wizard/geonames_import_view.xml b/base_location_geonames_import/wizard/geonames_import_view.xml
index da6c21140..8aab10d1b 100644
--- a/base_location_geonames_import/wizard/geonames_import_view.xml
+++ b/base_location_geonames_import/wizard/geonames_import_view.xml
@@ -31,9 +31,11 @@
         <field name="target">new</field>
     </record>
 
-    <menuitem id="better_zip_geonames_import_menu"
-        action="better_zip_geonames_import_action"
-        parent="sales_team.menu_localisation"
-        sequence="50"/>
+    <menuitem
+            id="better_zip_geonames_import_menu"
+            action="better_zip_geonames_import_action"
+            parent="base_location.locations_root_menu"
+            sequence="50"
+    />
 
 </odoo>