Move validation from widgets to the form fields

[francoisfreitag]

Move validation from widgets to the form fields

Previously, the widgets handled part of the validation. That behavior prevents overriding validation in form fields, as widgets were casting the value into a PhoneNumber object, validating it in the process.

Following the MultiValueField implementation from Django (and MultiWidget), the widget now handles the presentation logic, but makes no attempt at validation.

The new SplitPhoneNumberField handles the logic of validating the region choice and the number, and the PhoneNumberPrefixWidget simply dispatches the region and number data to the appropriate widget.

In order to retain backward compatibility, now that the validate_international_phonenumber actually comes into play, it’s error code has been changed to invalid, so that the custom error message for invalid shows.

Migration guide

validate_international_phonenumber

Review uses of the invalid_phone_number error code. Given that the validator usually did not come into play, you shouldn’t find many uses.

PhoneNumberField with RegionalPhoneNumberWidget

Make sure custom validation occurs in the Django Form clean_FIELD() (or clean()), and no changes should be noticeable.

PhoneNumberField with PhoneNumberPrefixWidget

Use the SplitPhoneNumberField instead. Error messages will change slightly and should be more precise (whether the region is not part of the choices, or the number cannot be interpreted in the selected region).

For more examples, take a look at tests.test_formfields.SplitPhoneNumberFieldTest. In particular test_custom_attrs and test_custom_choices, to see how they were migrated to the new field.

Finally, added a validate_phonenumber validator that allows short numbers.

Closes #441
Closes #597

[francoisfreitag]

@stefanfoulis Would you like to review this PR?
If so, it can easily wait a few weeks. Otherwise, I would like to proceed with this change in an 8.0.0 release.

[amateja]

Hi @stefanfoulis,

I'm honored but recently I've been a bit stretched. I should find some time next week.

[stefanfoulis]

🚀

[stefanfoulis]

🤔 are the region codes unique within COUNTRY_CODE_TO_REGION_CODE?

quick'n'dirty test:

In [1]: from phonenumbers import COUNTRY_CODE_TO_REGION_CODE
In [2]: from collections import Counter
In [3]: counter = Counter()
In [4]: for region_codes in COUNTRY_CODE_TO_REGION_CODE.values():
   ...:     for region_code in region_codes:
   ...:         counter[region_code] += 1
   ...: 
In [5]: counter.most_common(5)
Out[5]: [('001', 9), ('US', 1), ('AG', 1), ('AI', 1), ('AS', 1)]

Looks like only 001 has duplicates 🤔

In [12]: from collections import defaultdict
    ...: region_2_country = defaultdict(list)
    ...: for country_code, region_codes in COUNTRY_CODE_TO_REGION_CODE.items():
    ...:     for region_code in region_codes:
    ...:         region_2_country[region_code].append(country_code)
    ...: 

In [13]: region_2_country["001"]
Out[13]: [800, 808, 870, 878, 881, 882, 883, 888, 979]

In[14]: from phonenumber_field.formfields import REGION_CODE_TO_COUNTRY_CODE
In[15]: REGION_CODE_TO_COUNTRY_CODE["001"]
Out[15]: 979

So we currently "pick" the last one in the list. I'm not sure if this is the most relevant country code for that region code, or if it even makes sense to only pick one 🤔 .

[stefanfoulis]

phonenumbers.COUNTRY_CODES_FOR_NON_GEO_REGIONS seems to be exactly that list. Perhaps we should include those variants in our REGION_CODE_TO_COUNTRY_CODE as well 🤔 . But might be tricky to get that to correctly work with the widget and stuff.

In [13]: phonenumbers.COUNTRY_CODES_FOR_NON_GEO_REGIONS
Out[13]: {800, 808, 870, 878, 881, 882, 883, 888, 979}

[stefanfoulis]

Anyway... I'm not sure how the "old" implementation dealt with this. So if this is not a regression I don't think it is necessarily to fix in this PR.

[francoisfreitag]

Thanks, I never noticed this entry, and that’s indeed a bug.
The entry shows up as “world” in the prefix <select>, with only +979 (as you noticed). I opened #605 to deal with it. I’ll wait until this PR lands before touching that issue though.