Add fix for national + international numbers combined

[fraimondo]

Another attempt at #331

[francoisfreitag]

I’m failing to see what this brings over using the region keyword argument for the field?

diff --git a/tests/models.py b/tests/models.py
index 4edb9b5..358df87 100644
--- a/tests/models.py
+++ b/tests/models.py
@@ -30,7 +30,7 @@ class TestModel(models.Model):
     """Basic Field Test"""
 
     name = models.CharField(max_length=255, blank=True, default="")
-    phone = PhoneNumberField()
+    phone = PhoneNumberField(region="AR")
 
 
 class TestModelPhoneB(models.Model):
diff --git a/tests/tests.py b/tests/tests.py
index 74f0b22..679eb3e 100644
--- a/tests/tests.py
+++ b/tests/tests.py
@@ -520,9 +520,6 @@ class RegionPhoneNumberModelFieldTest(TestCase):
         self.assertEqual(phonenumbers.parse(ALGERIAN_PHONE_NUMBER), m.phone_number)
         self.assertEqual(ALGERIAN_PHONE_NUMBER, m.phone_number)
 
-    @override_settings(
-        PHONENUMBER_DEFAULT_REGION="AR", PHONENUMBER_DEFAULT_FORMAT="NATIONAL"
-    )
     def test_international_phone_is_valid(self):
         class PhoneNumberForm(forms.ModelForm):
             class Meta:

[fraimondo]

The use case is a to allow both national + international numbers. In particular:

1- It removes the international prefix of the numbers from the PHONENUMBER_DEFAULT_REGION. So every phone number from that region is displayed/parsed without the country code (+XX).

2-It allows to parse/display phone numbers from other regions in the international format, as set by PHONENUMBER_GLOBAL_FORMAT.

If I set the region in the field, it only allows for one specific region to be used in the field.

[francoisfreitag]

Can you update the test to show the limitation. By setting the "AR" region on the model field, I am able to send an E164 phone number and a national phone number. The national phone number is displayed in the national format and the E164 number is displayed in the E164 format.

diff --git a/tests/models.py b/tests/models.py
index 4edb9b5..358df87 100644
--- a/tests/models.py
+++ b/tests/models.py
@@ -30,7 +30,7 @@ class TestModel(models.Model):
     """Basic Field Test"""
 
     name = models.CharField(max_length=255, blank=True, default="")
-    phone = PhoneNumberField()
+    phone = PhoneNumberField(region="AR")
 
 
 class TestModelPhoneB(models.Model):
diff --git a/tests/tests.py b/tests/tests.py
index 74f0b22..7b3af31 100644
--- a/tests/tests.py
+++ b/tests/tests.py
@@ -520,17 +520,26 @@ class RegionPhoneNumberModelFieldTest(TestCase):
         self.assertEqual(phonenumbers.parse(ALGERIAN_PHONE_NUMBER), m.phone_number)
         self.assertEqual(ALGERIAN_PHONE_NUMBER, m.phone_number)
 
-    @override_settings(
-        PHONENUMBER_DEFAULT_REGION="AR", PHONENUMBER_DEFAULT_FORMAT="NATIONAL"
-    )
     def test_international_phone_is_valid(self):
         class PhoneNumberForm(forms.ModelForm):
             class Meta:
                 model = models.TestModel
-                fields = "__all__"
+                fields = ["phone"]
 
         form = PhoneNumberForm({"phone": "+32468547825"})  # belgian phone
         self.assertTrue(form.is_valid())
+        self.assertEqual(
+            form.as_p(),
+            '<p><label for="id_phone">Phone:</label> '
+            '<input type="tel" name="phone" value="+32468547825" maxlength="128" required id="id_phone">'
+            "</p>",
+        )
 
         form = PhoneNumberForm({"phone": "01145482368"})  # argentinian phone
         self.assertTrue(form.is_valid())
+        self.assertEqual(
+            form.as_p(),
+            '<p><label for="id_phone">Phone:</label> '
+            '<input type="tel" name="phone" value="01145482368" maxlength="128" required id="id_phone">'
+            "</p>",
+        )

[fraimondo]

This is the test that fails if you don't override the settings.

The idea is to display national numbers on the national format and the rest using the international.
Without these changes, it is impossible as when the form is created from the instance, it is filled with the international phone number.

    # @override_settings(
    #     PHONENUMBER_DEFAULT_REGION="AR", PHONENUMBER_DEFAULT_FORMAT="NATIONAL"
    # )
    def test_international_phone_is_valid(self):
        class PhoneNumberForm(forms.ModelForm):
            class Meta:
                model = models.TestModel
                fields = ["phone"]

        form = PhoneNumberForm({"phone": "+32468547825"})  # belgian phone
        self.assertTrue(form.is_valid())
        self.assertEqual(
            form.as_p(),
            '<p><label for="id_phone">Phone:</label> '
            '<input type="tel" name="phone" value="+32468547825" maxlength="128" required id="id_phone">'
            "</p>",
        )

        form = PhoneNumberForm({"phone": "01145482368"})  # argentinian phone
        self.assertTrue(form.is_valid())
        self.assertEqual(
            form.as_p(),
            '<p><label for="id_phone">Phone:</label> '
            '<input type="tel" name="phone" value="01145482368" maxlength="128" required id="id_phone">'
            "</p>",
        )

        obj = models.TestModel.objects.create(phone='+32468547825')
        form = PhoneNumberForm(instance=obj)
        self.assertEqual(
            form.as_p(),
            '<p><label for="id_phone">Phone:</label> '
            '<input type="tel" name="phone" value="+32468547825" maxlength="128" required id="id_phone">'
            "</p>",
        )

        obj = models.TestModel.objects.create(phone='01145482368')
        form = PhoneNumberForm(instance=obj)
        self.assertEqual(
            form.as_p(),
            '<p><label for="id_phone">Phone:</label> '
            '<input type="tel" name="phone" value="01145482368" maxlength="128" required id="id_phone">'
            "</p>",
        )

[fraimondo]

So the point is what "NATIONAL" means:

1. The number without the country code
2. The number you need to dial from the region of the field or the default region.

So far, before this PR, the behaviour is like in option 1. The method __str__ from phone number_field.phonenumber.PhoneNumber will format every number without the country code. So when a form is rendered from an instance which has a number in E614, it will leave out the country code. Once the form is processed (from request.POST), the country code will be lost and parsed as a national number.

With this PR, if the number is not from the default region, it will be rendered as E614 (or according to PHONENUMBER_GLOBAL_FORMAT). When the form is processed, the country code will still be there.

Problem is that this PR does not account for the form field region. That is, if the form field region is different from the default region, the number is rendered in the default region and then parsed according to the field region. I am trying to solve this issue, but I can't figure out in the way it is implemented.

The field region is stored in modelfields.PhoneNumberField._region. However, it needs to be accessed from the PhoneNumber object created somewhere from attr_class, which I don't seem to find anywhere in Django. Guess I'm hitting the dark side here.

[francoisfreitag]

Thanks for the explanation. I don’t think a setting is the way to go, but agree on the goal. If a region has been specified, the phone numbers should be formatted in the national format when possible. That’s what is the most helpful to national users, and it makes sense to use an international number for international users. 👍

Would the following patch work for you:

diff --git a/phonenumber_field/formfields.py b/phonenumber_field/formfields.py
index 5f880ee..23f4594 100644
--- a/phonenumber_field/formfields.py
+++ b/phonenumber_field/formfields.py
@@ -5,7 +5,7 @@ from django.forms.fields import CharField
 from django.utils.text import format_lazy
 from django.utils.translation import gettext_lazy as _
 
-from phonenumber_field.phonenumber import to_python, validate_region
+from phonenumber_field.phonenumber import PhoneNumber, to_python, validate_region
 from phonenumber_field.validators import validate_international_phonenumber
 
 
@@ -46,3 +46,21 @@ class PhoneNumberField(CharField):
             raise ValidationError(self.error_messages["invalid"])
 
         return phone_number
+
+    def prepare_value(self, value):
+        if self.region:
+            phone_number = (
+                value
+                if isinstance(value, PhoneNumber)
+                else to_python(value, region=self.region)
+            )
+            try:
+                phone_region_codes = phonenumbers.data._COUNTRY_CODE_TO_REGION_CODE[
+                    phone_number.country_code
+                ]
+            except KeyError:
+                pass
+            else:
+                if self.region in phone_region_codes:
+                    value = phone_number.as_national
+        return value
diff --git a/tests/models.py b/tests/models.py
index 4edb9b5..358df87 100644
--- a/tests/models.py
+++ b/tests/models.py
@@ -30,7 +30,7 @@ class TestModel(models.Model):
     """Basic Field Test"""
 
     name = models.CharField(max_length=255, blank=True, default="")
-    phone = PhoneNumberField()
+    phone = PhoneNumberField(region="AR")
 
 
 class TestModelPhoneB(models.Model):
diff --git a/tests/tests.py b/tests/tests.py
index 74f0b22..a138692 100644
--- a/tests/tests.py
+++ b/tests/tests.py
@@ -520,17 +520,56 @@ class RegionPhoneNumberModelFieldTest(TestCase):
         self.assertEqual(phonenumbers.parse(ALGERIAN_PHONE_NUMBER), m.phone_number)
         self.assertEqual(ALGERIAN_PHONE_NUMBER, m.phone_number)
 
-    @override_settings(
-        PHONENUMBER_DEFAULT_REGION="AR", PHONENUMBER_DEFAULT_FORMAT="NATIONAL"
-    )
     def test_international_phone_is_valid(self):
         class PhoneNumberForm(forms.ModelForm):
             class Meta:
                 model = models.TestModel
-                fields = "__all__"
+                fields = ["phone"]
 
         form = PhoneNumberForm({"phone": "+32468547825"})  # belgian phone
         self.assertTrue(form.is_valid())
+        self.assertEqual(
+            form.as_p(),
+            '<p><label for="id_phone">Phone:</label> '
+            '<input type="tel" name="phone" value="+32468547825" maxlength="128" required id="id_phone">'
+            "</p>",
+        )
 
         form = PhoneNumberForm({"phone": "01145482368"})  # argentinian phone
         self.assertTrue(form.is_valid())
+        self.assertEqual(
+            form.as_p(),
+            '<p><label for="id_phone">Phone:</label> '
+            '<input type="tel" name="phone" value="011 4548-2368" maxlength="128" required id="id_phone">'
+            "</p>",
+        )
+
+        obj = models.TestModel.objects.create(phone="+32468547825")
+        form = PhoneNumberForm(instance=obj)
+        self.assertEqual(
+            form.as_p(),
+            '<p><label for="id_phone">Phone:</label> '
+            '<input type="tel" name="phone" value="+32468547825" maxlength="128" required id="id_phone">'
+            "</p>",
+        )
+
+        obj = models.TestModel.objects.create(phone="01145482368")
+        form = PhoneNumberForm(instance=obj)
+        self.assertEqual(
+            form.as_p(),
+            '<p><label for="id_phone">Phone:</label> '
+            '<input type="tel" name="phone" value="011 4548-2368" maxlength="128" required id="id_phone">'
+            "</p>",
+        )
+
+        form = PhoneNumberForm({"phone": "invalid"})
+        self.assertFalse(form.is_valid())
+        self.assertEqual(
+            form.as_p(),
+            '<ul class="errorlist">'
+            "<li>Enter a valid phone number (e.g. 011 2345-6789) or a number with an international call prefix.</li>"
+            "</ul>\n"
+            '<p><label for="id_phone">Phone:</label> '
+            '<input type="tel" name="phone" value="invalid" maxlength="128" required id="id_phone">'
+            "</p>",
+        )

[fraimondo]

Just pushed.

This solution works, as long as there is no default region or format in settings.

I also took the liberty to fix tox.ini to work with djmain (I'm using Django 4.0a1).

[fraimondo]

I added a new test case. The method prepare_value needed to check not only the region but the empty values. In case on a field with blank=True and a region set, the value might be None.

[francoisfreitag]

I pushed a few cosmetic edits to the PR. If they look good to you, I’ll merge the PR.

Thanks for suggesting that improvement and your work on it! 🎉

[fraimondo]

Looks good for me.

Happy to contribute. This is one of the projects that make developing Django apps easier. Happy to help keep it updated!

[francoisfreitag]

Changes:

• squashed commits
  - trust the data from phonenumbers and expect the lookup of the country code to region code to succeed Turns out, it is needed when the data is invalid. Added a test.

I’ll merge this PR tomorrow and make a release this week.