stefanfoulis / django-phonenumber-field

A django model and form field for normalised phone numbers using python-phonenumbers
MIT License
1.48k stars 318 forks source link

Move validation from widgets to the form fields #603

Closed francoisfreitag closed 3 months ago

francoisfreitag commented 6 months ago

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 commented 5 months ago

@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 commented 5 months ago

Hi @stefanfoulis,

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