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

Formatting being lost #539

Closed
chrisspen opened this issue Sep 27, 2022 · 5 comments
Closed

Formatting being lost #539

chrisspen opened this issue Sep 27, 2022 · 5 comments

Comments

@chrisspen
Copy link

I recently upgraded to django-phonenumber-field[phonenumbers]==7.0.0 and now all my phone numbers, previously formatted like (800) 123-4567, are being reformatted as +18001234567.

Do you know why this is?

Is there a new flag or formatting template I should set to maintain my preferred format?

@francoisfreitag
Copy link
Collaborator

Can you precise where the format is being lost? Is that in your console, in the HTML generated by your template, in your HTML input values?

@chrisspen
Copy link
Author

I have a unittest that tests entering an international number, like +18001234567, into a PhoneNumberField via a standard Django ModelAdmin, saving, and then confirms it's re-rendered identically as +18001234567. The test then enters a US number, like 800-123-4567, into the same field, saves, and confirms it's also re-rendered identically as 800-123-4567.

In essence, the default behavior of the field supported both formats in the same field, and this test passed. After upgrading from version 6.3.0 to 7.0.0, this no longer seems to be supported. I can get one of the checks to pass by setting PHONENUMBER_DEFAULT_FORMAT (which I'd previously not defined) to either NATIONAL or INTERNATIONAL, but I can't find a way to fix both.

I imagine the only way this was possible is if PHONENUMBER_DB_FORMAT was blank and/or not being enforced, so the raw yet validated phone number was being stored to the db.

Is it no longer possible to store US and international phone numbers in the same field anymore? If so, that's your design call, but I'll need to fork the project to maintain the previous functionality for my clients, as a ton of people store both formats in the same field.

@francoisfreitag
Copy link
Collaborator

francoisfreitag commented Sep 28, 2022

I’m confused.

Both 800-123-4567 and +18001234567 are invalid phone numbers, I’m not sure if that’s what you have been using in your test case.

>>> import phonenumbers
>>> x = phonenumbers.parse("+18001234567")
>>> phonenumbers.is_valid_number(x)
False

The library treats invalid phone numbers as just an str (storing and retrieving the raw_input property of the PhoneNumber object). The default widget does provide some formatting for valid numbers, but it does not change invalid numbers.
The following test passes:

    def test_invalid_number(self):
        invalid_numbers = ["+18001234567", "800-123-4567"]
        widget = RegionalPhoneNumberWidget()
        for invalid_number in invalid_numbers:
            number = PhoneNumber.from_string(invalid_number, region="US")
            self.assertHTMLEqual(
                widget.render("number", number),
                f'<input name="number" type="tel" value="{invalid_number}" />',
            )

Without customization, the admin should be using that widget, thus your tests should pass, unless other customizations have been applied?

For valid phone numbers, the library by default will:

  • Display national numbers in the national format. Here for a valid US number, the input +12015550123 will be displayed as (201) 555-0123 (national format)
  • Display international numbers with the prefix, following the default format, e.g. with E164: +33123456789

Here’s the widget implementing the behavior described above:

class RegionalPhoneNumberWidget(TextInput):
"""
A :class:`~django.forms.Widget` that prefers displaying numbers in the
national format, and falls back to :setting:`PHONENUMBER_DEFAULT_FORMAT`
for international numbers.
"""
input_type = "tel"
def __init__(self, region=None, attrs=None):
"""
:keyword str region: 2-letter country code as defined in ISO 3166-1.
When not supplied, defaults to :setting:`PHONENUMBER_DEFAULT_REGION`
:keyword dict attrs: See :attr:`django.forms.Widget.attrs`
"""
if region is None:
region = getattr(settings, "PHONENUMBER_DEFAULT_REGION", None)
self.region = region
super().__init__(attrs)
def value_from_datadict(self, data, files, name):
phone_number_str = super().value_from_datadict(data, files, name)
if phone_number_str is None:
phone_number_str = ""
return to_python(phone_number_str, region=self.region)
def format_value(self, value):
if isinstance(value, PhoneNumber):
if value.is_valid():
region_codes = region_codes_for_country_code(value.country_code)
if self.region in region_codes:
return value.as_national
return super().format_value(value)

@francoisfreitag
Copy link
Collaborator

Finally got some time to look at your report in a project. Indeed, the RegionalPhoneNumberWidget is not being installed, because the admin provides widgets.AdminTextInputWidget as a widget, which the FormField uses. That’s a change from 7.0.0, in 6.3.0 the formatting was (incorrectly) done by the form field.

That’s why the phone numbers are displayed in E164 and not in the configured region. I’ll try to think of a way to use the RegionalPhoneNumberWidget in the admin by default. In the meantime, the following workaround should restore the previous behavior:

@admin.register(MyModel)
class MyModelAdmin(admin.ModelAdmin):
    formfield_overrides = {
        PhoneNumberField: {"widget": widgets.RegionalPhoneNumberWidget}
    }

@francoisfreitag
Copy link
Collaborator

francoisfreitag commented Oct 11, 2022

I’ve given this matter some thoughts. I believe we should stick to the AdminTextInputWidget, even though it’s not as nice. The admin can also be used to browse the content of the database, so exposing the stored value “raw” makes sense.
Fighting against the admin is probably not worth it, especially since it’s easy to configure otherwise.

What do you think of documenting the above trick (formfield_overrides)?

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

No branches or pull requests

2 participants