Merge pull request #139 from grupoesoc/better_partner_firstname

[8.0] partner_firstname: Recursive @onchange misbehavior

partner_firstname/models.py

@@ -78,8 +78,8 @@ class ResPartner(models.Model):
         trimmed whitespace.
         """
         # Company name goes to the lastname
-        if self.is_company or self.name is False:
-            parts = [self.name, False]
+        if self.is_company or not self.name:
+            parts = [self.name or False, False]
 
         # Guess name splitting
         else:
@@ -96,11 +96,28 @@ class ResPartner(models.Model):
         if not (self.firstname or self.lastname):
             raise exceptions.EmptyNamesError(self)
 
+    @api.one
+    @api.onchange("firstname", "lastname")
+    def _onchange_subnames(self):
+        """Avoid recursion when the user changes one of these fields.
+
+        This forces to skip the :attr:`~.name` inversion when the user is
+        setting it in a not-inverted way.
+        """
+        # Modify self's context without creating a new Environment.
+        # See https://github.com/odoo/odoo/issues/7472#issuecomment-119503916.
+        self.env.context = self.with_context(skip_onchange=True).env.context
+
     @api.one
     @api.onchange("name")
     def _onchange_name(self):
         """Ensure :attr:`~.name` is inverted in the UI."""
-        self._inverse_name_after_cleaning_whitespace()
+        if self.env.context.get("skip_onchange"):
+            # Do not skip next onchange
+            self.env.context = (
+                self.with_context(skip_onchange=False).env.context)
+        else:
+            self._inverse_name_after_cleaning_whitespace()
 
     @api.model
     def _install_partner_firstname(self):

partner_firstname/tests/__init__.py

@@ -28,4 +28,4 @@
 #
 ##############################################################################
 
-from . import test_name, test_empty
+from . import test_empty, test_name, test_onchange

partner_firstname/tests/base.py

@@ -29,7 +29,15 @@ from openerp.tests.common import TransactionCase
 from .. import exceptions as ex
 
 
-class BaseCase(TransactionCase):
+class MailInstalled():
+    def mail_installed(self):
+        """Check if ``mail`` module is installed.``"""
+        return (self.env["ir.module.module"]
+                .search([("name", "=", "mail")])
+                .state == "installed")
+
+
+class BaseCase(TransactionCase, MailInstalled):
     def setUp(self):
         super(BaseCase, self).setUp()
         self.check_fields = True
@@ -76,3 +84,13 @@ class BaseCase(TransactionCase):
         self.check_fields = False
         with self.assertRaises(ex.EmptyNamesError):
             self.original.firstname = self.original.lastname = False
+
+
+class OnChangeCase(TransactionCase):
+    is_company = False
+
+    def new_partner(self):
+        """Create an empty partner. Ensure it is (or not) a company."""
+        new = self.env["res.partner"].new()
+        new.is_company = self.is_company
+        return new

partner_firstname/tests/test_empty.py

@@ -16,7 +16,13 @@
 # You should have received a copy of the GNU Affero General Public License
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
+"""Test situations where names are empty.
+
+To have more accurate results, remove the ``mail`` module before testing.
+"""
+
 from openerp.tests.common import TransactionCase
+from .base import MailInstalled
 from .. import exceptions as ex
 
 
@@ -47,7 +53,16 @@ class PersonCase(CompanyCase):
     context = {"default_is_company": False}
 
 
-class UserCase(CompanyCase):
+class UserCase(CompanyCase, MailInstalled):
     """Test ``res.users``."""
     model = "res.users"
     context = {"default_login": "user@example.com"}
+
+    def tearDown(self):
+        # Cannot create users if ``mail`` is installed
+        if self.mail_installed():
+            # Skip tests
+            super(CompanyCase, self).tearDown()
+        else:
+            # Run tests
+            super(UserCase, self).tearDown()

partner_firstname/tests/test_name.py

@@ -25,6 +25,11 @@
 # along with this program; if not, write to the Free Software
 # Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA  02111-1307, USA.
 
+"""Test naming logic.
+
+To have more accurate results, remove the ``mail`` module before testing.
+"""
+
 from .base import BaseCase
 
 
@@ -54,6 +59,7 @@ class PartnerCompanyCase(BaseCase):
         self.original.is_company = True
 
     def test_copy(self):
+        """Copy the partner and compare the result."""
         super(PartnerCompanyCase, self).test_copy()
         self.expect(self.name, False, self.name)
 
@@ -66,6 +72,19 @@ class PartnerCompanyCase(BaseCase):
 
 class UserCase(PartnerContactCase):
     def create_original(self):
-        self.original = self.env["res.users"].create({
-            "name": u"%s %s" % (self.lastname, self.firstname),
-            "login": "firstnametest@example.com"})
+        name = u"%s %s" % (self.lastname, self.firstname)
+
+        # Cannot create users if ``mail`` is installed
+        if self.mail_installed():
+            self.original = self.env.ref("base.user_demo")
+            self.original.name = name
+        else:
+            self.original = self.env["res.users"].create({
+                "name": name,
+                "login": "firstnametest@example.com"})
+
+    def test_copy(self):
+        """Copy the partner and compare the result."""
+        # Skip if ``mail`` is installed
+        if not self.mail_installed():
+            super(UserCase, self).test_copy()

partner_firstname/tests/test_onchange.py

@@ -0,0 +1,102 @@
+# -*- coding: utf-8 -*-
+"""These tests try to mimic the behavior of the UI form.
+
+The form operates in onchange mode, with its limitations.
+"""
+
+from .base import OnChangeCase
+
+
+class PartnerCompanyCase(OnChangeCase):
+    is_company = True
+
+    def test_create_from_form(self):
+        """A user creates a company from the form."""
+        name = u"Sôme company"
+        with self.env.do_in_onchange():
+            # User presses ``new``
+            partner = self.new_partner()
+
+            # User sets a name, which triggers onchanges
+            partner.name = name
+            partner._onchange_name()
+
+            self.assertEqual(partner.name, name)
+            self.assertEqual(partner.firstname, False)
+            self.assertEqual(partner.lastname, name)
+
+    def test_empty_name_and_subnames(self):
+        """If the user empties ``name``, subnames must be ``False``.
+
+        Otherwise, the ``required`` attr will not work as expected.
+        """
+        with self.env.do_in_onchange():
+            # User presses ``new``
+            partner = self.new_partner()
+
+            # User sets a name, which triggers onchanges
+            partner.name = u"Foó"
+            partner._onchange_name()
+
+            # User unsets name, which triggers onchanges
+            partner.name = u""
+            partner._onchange_name()
+
+            self.assertEqual(partner.firstname, False)
+            self.assertEqual(partner.lastname, False)
+
+
+class PartnerContactCase(OnChangeCase):
+    def test_create_from_form_only_firstname(self):
+        """A user creates a contact with only the firstname from the form."""
+        firstname = u"Fïrst"
+        with self.env.do_in_onchange():
+            # User presses ``new``
+            partner = self.new_partner()
+
+            # Changes firstname, which triggers onchanges
+            partner.firstname = firstname
+            partner._onchange_subnames()
+            partner._onchange_name()
+
+            self.assertEqual(partner.lastname, False)
+            self.assertEqual(partner.firstname, firstname)
+            self.assertEqual(partner.name, firstname)
+
+    def test_create_from_form_only_lastname(self):
+        """A user creates a contact with only the lastname from the form."""
+        lastname = u"Läst"
+        with self.env.do_in_onchange():
+            # User presses ``new``
+            partner = self.new_partner()
+
+            # Changes lastname, which triggers onchanges
+            partner.lastname = lastname
+            partner._onchange_subnames()
+            partner._onchange_name()
+
+            self.assertEqual(partner.firstname, False)
+            self.assertEqual(partner.lastname, lastname)
+            self.assertEqual(partner.name, lastname)
+
+    def test_create_from_form_all(self):
+        """A user creates a contact with all names from the form."""
+        firstname = u"Fïrst"
+        lastname = u"Läst"
+        with self.env.do_in_onchange():
+            # User presses ``new``
+            partner = self.new_partner()
+
+            # Changes firstname, which triggers onchanges
+            partner.firstname = firstname
+            partner._onchange_subnames()
+            partner._onchange_name()
+
+            # Changes lastname, which triggers onchanges
+            partner.lastname = lastname
+            partner._onchange_subnames()
+            partner._onchange_name()
+
+            self.assertEqual(partner.lastname, lastname)
+            self.assertEqual(partner.firstname, firstname)
+            self.assertEqual(partner.name, u" ".join((lastname, firstname)))