Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove non-geo phone numbers from the SplitPhoneNumberWidget #614

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

francoisfreitag
Copy link
Collaborator

The COUNTRY_CODE_TO_REGION_CODE dict contains values that are not specific to a region code: the COUNTRY_CODES_FOR_NON_GEO_REGIONS. These country codes are international, and are not a good fit for the SplitPhoneNumberWidget, which offers to select a region, then fill in the national number.

Fixes #605

Copy link
Owner

@stefanfoulis stefanfoulis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤔will this cause problems for people who have built things around the old behaviour?

@francoisfreitag
Copy link
Collaborator Author

Finally came back to this PR.

Will this cause problems for people who have built things around the old behaviour?

They couldn’t. Using the field with a world (non-geo) phone number results in an exception because we cannot generate an example number for that region (phonenumbers.example_number("001") returns None).

    def test_nongeo(self):
        class PhoneNumberForm(forms.Form):
            number = SplitPhoneNumberField()

        nongeo_country_code = list(phonenumbers.COUNTRY_CODES_FOR_NON_GEO_REGIONS)[0]
        nongeo_region_code = phonenumbers.COUNTRY_CODE_TO_REGION_CODE[
            nongeo_country_code
        ][0]
        form = PhoneNumberForm(
            {
                "number_0": nongeo_region_code,
                "number_1": "800-002-1244",
            }
        )
        self.assertIs(form.is_valid(), False)

Outputs:

======================================================================
ERROR: test_nongeo_deprecation_warning (tests.test_formfields.SplitPhoneNumberFormFieldTest.test_nongeo_deprecation_warning)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/freitafr/dev/django-phonenumber-field/tests/test_formfields.py", line 712, in test_nongeo_deprecation_warning
    self.assertIs(form.is_valid(), False)
                  ~~~~~~~~~~~~~^^
  File "/home/freitafr/dev/django-phonenumber-field/.venv/lib/python3.13/site-packages/django/forms/forms.py", line 206, in is_valid
    return self.is_bound and not self.errors
                                 ^^^^^^^^^^^
  File "/home/freitafr/dev/django-phonenumber-field/.venv/lib/python3.13/site-packages/django/forms/forms.py", line 201, in errors
    self.full_clean()
    ~~~~~~~~~~~~~~~^^
  File "/home/freitafr/dev/django-phonenumber-field/.venv/lib/python3.13/site-packages/django/forms/forms.py", line 337, in full_clean
    self._clean_fields()
    ~~~~~~~~~~~~~~~~~~^^
  File "/home/freitafr/dev/django-phonenumber-field/.venv/lib/python3.13/site-packages/django/forms/forms.py", line 345, in _clean_fields
    self.cleaned_data[name] = field._clean_bound_field(bf)
                              ~~~~~~~~~~~~~~~~~~~~~~~~^^^^
  File "/home/freitafr/dev/django-phonenumber-field/.venv/lib/python3.13/site-packages/django/forms/fields.py", line 270, in _clean_bound_field
    return self.clean(value)
           ~~~~~~~~~~^^^^^^^
  File "/home/freitafr/dev/django-phonenumber-field/phonenumber_field/formfields.py", line 191, in clean
    example_number = to_python(number).as_national
                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
AttributeError: 'NoneType' object has no attribute 'as_national'

----------------------------------------------------------------------

Besides, according to the list of phone numbers country code on Wikipedia, the non-geo codes are:

>>> from phonenumbers import COUNTRY_CODES_FOR_NON_GEO_REGIONS
>>> COUNTRY_CODES_FOR_NON_GEO_REGIONS
{800, 870, 808, 878, 881, 882, 883, 979, 888}

These numbers are unlikely to legitimately appear in a standard app.

The COUNTRY_CODE_TO_REGION_CODE dict contains values that are not
specific to a region code: the COUNTRY_CODES_FOR_NON_GEO_REGIONS. These
country codes are international, and are not a good fit for the
SplitPhoneNumberWidget, which offers to select a region, then fill in
the national number.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Handling of special (non-geo) phone numbers
2 participants