[IMP] partner_phone_country_validation: add tests

partner_phone_country_validation/README.rst

@@ -1,44 +1,150 @@
-===============
-partner_phone_country_validation
-===============
+================================
+Partner Phone Country Validation
+================================
 
-Check the correct format of phone according to country when partner creation/validation
+This module enhances phone number handling on partners by adding automatic
+country detection from phone numbers and enforcing country consistency.
+
+It extends the standard ``phone_validation`` module and the OCA ``base_phone``
+module to provide additional features for phone number management.
+
+**Table of contents**
+
+.. contents::
+   :local:
+
+Features
+========
+
+Automatic Phone Number Normalization
+------------------------------------
+
+When entering a phone number, the module automatically converts numbers
+starting with ``00`` to the international ``+`` format:
+
+* ``0033 6 12 34 56 78`` → ``+33 6 12 34 56 78``
+* ``0041 76 123 45 67`` → ``+41 76 123 45 67``
+
+This normalization happens before the standard formatting, ensuring
+compatibility with the ``phonenumbers`` library.
+
+Automatic Country Detection
+---------------------------
+
+When a phone number with an international prefix (``+xx``) is entered and
+the partner has no country set, the module automatically detects and sets
+the country based on the phone number prefix:
+
+* Enter ``+33 6 12 34 56 78`` → Country is set to **France**
+* Enter ``+32 4 123 45 67`` → Country is set to **Belgium**
+* Enter ``+41 76 123 45 67`` → Country is set to **Switzerland**
+
+This works for both ``phone`` and ``mobile`` fields.
+
+Country Validation Constraint
+-----------------------------
+
+The module enforces that a country must be set on the partner if a phone
+or mobile number is present. This ensures data consistency and allows
+proper phone number formatting.
+
+If you try to save a partner with a phone number but no country, a
+validation error will be raised.
+
+How It Works
+============
+
+The module overrides the ``_onchange_phone_validation`` and
+``_onchange_mobile_validation`` methods from the standard ``phone_validation``
+module. The processing order is:
+
+1. **Normalize**: Convert ``00xx`` format to ``+xx`` international format
+2. **Detect country**: If no country is set, detect it from the phone prefix
+3. **Format**: Call the standard formatting (via ``super()``) which formats
+   the number according to the partner's country
+
+When the country is changed on a partner, both phone and mobile numbers
+are automatically reformatted to match the new country's format.
+
+Dependencies
+============
+
+This module depends on:
+
+* ``base``: Odoo base module
+* ``base_phone``: OCA module providing the ``phone.validation.mixin``
+  (from `connector-telephony <https://github.com/OCA/connector-telephony>`_)
+
+The ``base_phone`` module itself depends on the standard ``phone_validation``
+module which uses the `phonenumbers <https://github.com/daviddrysdale/python-phonenumbers>`_
+Python library.
 
 Installation
 ============
 
-Use Odoo normal module installation procedure to install
-``partner_phone_country_validation``.
+1. Install the ``base_phone`` module from the OCA ``connector-telephony`` repository
+2. Install this module using the standard Odoo module installation procedure
 
-Known issues / Roadmap
-======================
+Configuration
+=============
 
-None yet.
+No configuration is required. The module works automatically once installed.
+
+Known Issues / Limitations
+==========================
+
+National Numbers Without International Prefix
+---------------------------------------------
+
+When changing a partner's country, phone numbers in **national format**
+(without international prefix) may not be reformatted if they are not
+valid for the new country.
+
+For example:
+
+* A French mobile ``06 12 34 56 78`` (without ``+33``) cannot be reformatted
+  to a Belgian format because ``06`` is not a valid Belgian mobile prefix.
+* The ``phonenumbers`` library validates numbers and will keep the original
+  format if the number is invalid for the target country.
+
+**Recommendation**: Always use international format (``+xx``) for phone numbers
+to ensure proper handling when countries change.
 
 Bug Tracker
 ===========
 
-Bugs are tracked on `our issues website <https://github.com/elabore-coop/partner_phone_country_validation/issues>`_. In case of
-trouble, please check there if your issue has already been
-reported. If you spotted it first, help us smashing it by providing a
-detailed and welcomed feedback.
+Bugs are tracked on `Elabore Git Issues <https://git.elabore.coop/Elabore/partner-tools/issues>`_.
+
+In case of trouble, please check there if your issue has already been reported.
+If you spotted it first, help us smash it by providing a detailed and welcomed
+feedback.
 
 Credits
 =======
 
+Authors
+-------
+
+* `Elabore <https://elabore.coop>`_
+
 Contributors
 ------------
 
-* Boris Gallet - `Email<mailto:boris.gallet@elabore.coop>` - `Github<https://github.com/b0g>`
+* Boris Gallet <boris.gallet@elabore.coop> (`GitHub <https://github.com/b0g>`_)
+* Stéphan Sainléger <stephan.sainleger@elabore.coop>
 
 Funders
 -------
 
 The development of this module has been financially supported by:
-* Elabore (https://elabore.coop)
 
+* `Elabore <https://elabore.coop>`_
 
 Maintainer
 ----------
 
-This module is maintained by Elabore.
+.. image:: https://elabore.coop/logo.png
+   :alt: Elabore
+   :target: https://elabore.coop
+
+This module is maintained by Elabore.

partner_phone_country_validation/models/res_partner.py

@@ -1,73 +1,54 @@
 from odoo import api, models, _
-from odoo.addons.partner_phone_country_validation.tools.get_country_from_phone_number import get_country_from_phone_number
+from odoo.addons.partner_phone_country_validation.tools.get_country_from_phone_number import (
+    get_country_from_phone_number,
+)
 from odoo.exceptions import ValidationError
-import logging
 
-_logger = logging.getLogger(__name__)
 
 class Partner(models.Model):
-    _name = 'res.partner'
-    _inherit = ['res.partner', 'phone.validation.mixin']
+    _name = "res.partner"
+    _inherit = ["res.partner", "phone.validation.mixin"]
 
-    @api.constrains('country_id','phone','mobile')
+    @api.constrains("country_id", "phone", "mobile")
     def _check_country_id(self):
-        if not self.country_id and (self.phone or self.mobile):
-            raise ValidationError(_('You must set a country for the phone number'))
-        
-    @api.onchange('country_id')
-    def _onchange_country(self):
-        self.format_mobile_from_country()
-        self.format_phone_from_country()
+        for partner in self:
+            if not partner.country_id and (partner.phone or partner.mobile):
+                raise ValidationError(_("You must set a country for the phone number"))
 
-    def format_number_zerozero(self, number):
-        if number.startswith("00"):
-            number = "+"+number[2:]
+    def _normalize_phone_number(self, number):
+        """Convert 00xx format to +xx international format."""
+        if number and number.startswith("00"):
+            return "+" + number[2:]
         return number
 
-    def format_phone_from_country(self):
-        if self.phone:            
-            country = None
-            if self.phone.startswith('+'):
-                country = self.env['res.country'].search(
-                    [('code', '=', get_country_from_phone_number(self.phone))], limit=1
-                )
-            if self.country_id:
-                country = self.country_id
+    def _set_country_from_phone_number(self, phone_number):
+        """Auto-detect and set country from phone number prefix if not already set."""
+        if not phone_number or self.country_id:
+            return
+        country_code = get_country_from_phone_number(phone_number)
+        if country_code:
+            country = self.env["res.country"].search(
+                [("code", "=", country_code)], limit=1
+            )
             if country:
-                self.phone = self.phone_format(self.phone, country=country)
+                self.country_id = country
 
-    def format_mobile_from_country(self):
-        if self.mobile:            
-            country = None
-            if self.mobile.startswith('+'):
-                country = self.env['res.country'].search(
-                    [('code', '=', get_country_from_phone_number(self.mobile))], limit=1
-                )
-            if self.country_id:
-                country = self.country_id
-            if country:
-                self.mobile = self.phone_format(self.mobile, country=country)
-
-    def set_country_from_phone_number(self, phone_number):
-        country = self.env['res.country'].search(
-            [('code', '=', get_country_from_phone_number(phone_number))], limit=1
-        )
-        if country and not self.country_id:
-            self.country_id = country
-
-    @api.onchange('phone')
+    @api.onchange("phone", "country_id", "company_id")
     def _onchange_phone_validation(self):
-        # If no country is found, we define the country based on the beginning of the number
+        # Normalize 00xx → +xx before standard processing
         if self.phone:
-            self.phone = self.format_number_zerozero(self.phone)
-            self.set_country_from_phone_number(self.phone)
-            self.format_phone_from_country()
-        
+            self.phone = self._normalize_phone_number(self.phone)
+            # Auto-detect country if not set
+            self._set_country_from_phone_number(self.phone)
+        # Let standard module do the formatting
+        super()._onchange_phone_validation()
 
-    @api.onchange('mobile')
+    @api.onchange("mobile", "country_id", "company_id")
     def _onchange_mobile_validation(self):
-        # If no country is found, we define the country based on the beginning of the number
+        # Normalize 00xx → +xx before standard processing
         if self.mobile:
-            self.mobile = self.format_number_zerozero(self.mobile)
-            self.set_country_from_phone_number(self.mobile)
-            self.format_mobile_from_country()
+            self.mobile = self._normalize_phone_number(self.mobile)
+            # Auto-detect country if not set
+            self._set_country_from_phone_number(self.mobile)
+        # Let standard module do the formatting
+        super()._onchange_mobile_validation()

partner_phone_country_validation/tests/__init__.py

@@ -0,0 +1,4 @@
+# Copyright 2024 Elabore (https://elabore.coop)
+# License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl.html).
+
+from . import test_partner_phone_country_validation

partner_phone_country_validation/tests/test_partner_phone_country_validation.py

@@ -0,0 +1,149 @@
+# Copyright 2024 Elabore (https://elabore.coop)
+# License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl.html).
+
+from odoo.exceptions import ValidationError
+from odoo.tests import Form, TransactionCase
+
+
+class TestPartnerPhoneCountryValidation(TransactionCase):
+    """Tests for partner phone country validation module.
+
+    Note on testing @api.onchange methods:
+    --------------------------------------
+    In Odoo, @api.onchange methods are only triggered by the web UI, not by
+    direct field assignment in Python code.
+
+    - `partner.phone = "xxx"` → triggers write() → validates constraints
+      BEFORE any onchange can run → fails if constraint not met
+
+    - `Form(partner)` → simulates web UI behavior → triggers onchange methods
+      when field values change → onchange can set country BEFORE save
+
+    We use Form() for tests that need onchange behavior (auto-detection of
+    country from phone prefix), and direct create/write for constraint tests.
+    """
+
+    @classmethod
+    def setUpClass(cls):
+        super().setUpClass()
+        cls.env = cls.env(context=dict(cls.env.context, tracking_disable=True))
+        cls.country_fr = cls.env.ref("base.fr")
+        cls.country_be = cls.env.ref("base.be")
+        # Set company country to France for default formatting
+        cls.env.company.country_id = cls.country_fr
+
+    def test_normalize_00_to_plus(self):
+        """Test conversion of 00xx format to +xx format."""
+        partner = self.env["res.partner"].create(
+            {"name": "Test Partner", "country_id": self.country_fr.id}
+        )
+        # 00 → + conversion
+        self.assertEqual(
+            partner._normalize_phone_number("0033612345678"), "+33612345678"
+        )
+        # + numbers unchanged
+        self.assertEqual(
+            partner._normalize_phone_number("+33612345678"), "+33612345678"
+        )
+        # National numbers unchanged
+        self.assertEqual(partner._normalize_phone_number("0612345678"), "0612345678")
+        # Edge cases
+        self.assertIsNone(partner._normalize_phone_number(None))
+        self.assertEqual(partner._normalize_phone_number(""), "")
+
+    def test_auto_country_detection(self):
+        """Test automatic country detection from phone prefix."""
+        partner = self.env["res.partner"].create({"name": "Test Partner"})
+        self.assertFalse(partner.country_id)
+
+        partner._set_country_from_phone_number("+33612345678")
+        self.assertEqual(partner.country_id, self.country_fr)
+
+    def test_auto_country_detection_does_not_override(self):
+        """Test that auto-detection does not override existing country."""
+        partner = self.env["res.partner"].create(
+            {"name": "Test Partner", "country_id": self.country_be.id}
+        )
+        partner._set_country_from_phone_number("+33612345678")
+        # Country should remain Belgium
+        self.assertEqual(partner.country_id, self.country_be)
+
+    def test_onchange_phone_full_flow(self):
+        """Test complete onchange flow: 00→+ normalization, country detection, formatting.
+
+        Use Form to simulate UI behavior where onchange triggers before save.
+        """
+        with Form(self.env["res.partner"]) as partner_form:
+            partner_form.name = "Test Partner"
+            partner_form.phone = "0033612345678"
+            # Onchange should have normalized and detected country
+            self.assertIn("+33", partner_form.phone)
+            self.assertEqual(partner_form.country_id, self.country_fr)
+
+    def test_onchange_mobile_full_flow(self):
+        """Test complete onchange flow for mobile field."""
+        with Form(self.env["res.partner"]) as partner_form:
+            partner_form.name = "Test Partner"
+            partner_form.mobile = "0032475123456"
+            # Onchange should have normalized and detected country
+            self.assertIn("+32", partner_form.mobile)
+            self.assertEqual(partner_form.country_id, self.country_be)
+
+    def test_onchange_with_existing_country(self):
+        """Test formatting with existing country uses that country."""
+        with Form(self.env["res.partner"]) as partner_form:
+            partner_form.name = "Test Partner"
+            partner_form.country_id = self.country_fr
+            partner_form.phone = "06 12 34 56 78"
+            # Should be formatted with French prefix
+            self.assertIn("+33", partner_form.phone)
+
+    def test_constraint_country_required_with_phone(self):
+        """Test that country is required when phone/mobile is set."""
+        with self.assertRaises(ValidationError):
+            self.env["res.partner"].create(
+                {"name": "Test Partner", "phone": "0612345678"}
+            )
+
+        with self.assertRaises(ValidationError):
+            self.env["res.partner"].create(
+                {"name": "Test Partner", "mobile": "0612345678"}
+            )
+
+    def test_constraint_passes_without_phone(self):
+        """Test that partner without phone doesn't require country."""
+        partner = self.env["res.partner"].create({"name": "Test Partner"})
+        self.assertTrue(partner.id)
+        self.assertFalse(partner.country_id)
+
+
+class TestGetCountryFromPhoneNumber(TransactionCase):
+    """Test the utility function for country detection."""
+
+    def test_valid_international_number(self):
+        """Test detection from valid international number."""
+        from odoo.addons.partner_phone_country_validation.tools.get_country_from_phone_number import (
+            get_country_from_phone_number,
+        )
+
+        self.assertEqual(get_country_from_phone_number("+33612345678"), "FR")
+
+    def test_invalid_number_returns_false(self):
+        """Test that invalid numbers return False."""
+        from odoo.addons.partner_phone_country_validation.tools.get_country_from_phone_number import (
+            get_country_from_phone_number,
+        )
+
+        self.assertFalse(get_country_from_phone_number("invalid"))
+        self.assertFalse(get_country_from_phone_number("0612345678"))
+
+    def test_national_number_without_prefix(self):
+        """Test handling of national number without international prefix."""
+        from odoo.addons.partner_phone_country_validation.tools.get_country_from_phone_number import (
+            get_country_from_phone_number,
+        )
+
+        # National numbers without + cannot be reliably detected
+        result = get_country_from_phone_number("0612345678")
+        # phonenumbers cannot determine country without prefix
+        self.assertFalse(result)

partner_phone_country_validation/tools/get_country_from_phone_number.py

@@ -7,7 +7,7 @@ try:
 
     def get_country_from_phone_number(number):
         try:
-            number = phonenumbers.parse(number)            
+            number = phonenumbers.parse(number)
             return phonenumbers.region_code_for_number(number)
         except phonenumbers.phonenumberutil.NumberParseException:
             return False
@@ -22,4 +22,4 @@ except ImportError:
                 "verified. Please install the `phonenumbers` Python module."
             )
             _phonenumbers_lib_warning = True
-        return number
+        return number