Extension information is lost during serialization

[jessebrennan]

Firstly, thanks for an awesome library!

It looks to me like the extension information is lost when using Django's built in serialization. We only use this for loading our test database, but it was surprising when the extension's didn't survive reloading the data.

After a little bit of investigation, I think fixing this should be pretty straight forwards, we would just need to implement value_to_string() for the field with one of the lossless serialization formats and then make sure the the to_python() function can handle it properly.

https://docs.djangoproject.com/en/4.2/howto/custom-model-fields/#converting-field-data-for-serialization

If the contribution is welcome, I might be interested in adding this fix. Thanks again!

[jessebrennan]

This also causes extension info to be deleted after running clean()!

[francoisfreitag]

Hi, thanks for raising the issue. I believe the serialization (python manage.py dumpdata) and the handling of phone number extensions works correctly, when both PHONENUMBER_DB_FORMAT and PHONENUMBER_DEFAULT_FORMAT are set to a format that handles extensions (RFC3966 or INTERNATIONAL).

When either setting is set to E164 (the default), extensions are dropped, because E164 only addresses public numbers (i.e. without extensions).

The doc on PHONENUMBER_DB_FORMAT is quite terse, and does not make it clear that you’ll need both settings.

I don’t see tests verifying phone number extension handling either, it might be a great addition. For the record, I’ve had success with the string "+33 5 55 00 00 00 ext. 66" in my experiments.

I’ll see about addressing these issues in the upcoming weeks. Please feel free to offer pull requests, that will speed up the resolution. :) Also, can you confirm that the library behaves as you would expect when both settings are set to an extension-compatible format?

[jessebrennan]

I found the documentation mostly satisfactory here, I have both variables set to the following

PHONENUMBER_DB_FORMAT = "INTERNATIONAL"
PHONENUMBER_DEFAULT_REGION = "US"

Now in a Django shell I have a User model with a PhoneNumberField.

[ins] In [8]: user.phone_number
Out[8]: PhoneNumber(country_code=1, national_number=4158675309, extension=None, italian_leading_zero=None, number_of_leading_zeros=None, country_code_source=1, preferred_domestic_carrier_code=None)
[ins] In [9]: user.phone_number.extension = '321'

[ins] In [10]: user.phone_number
Out[10]: PhoneNumber(country_code=1, national_number=4158675309, extension='321', italian_leading_zero=None, number_of_leading_zeros=None, country_code_source=1, preferred_domestic_carrier_code=None)
[ins] In [11]: user.full_clean()

[ins] In [12]: user.phone_number
Out[12]: PhoneNumber(country_code=1, national_number=4158675309, extension=None, italian_leading_zero=None, number_of_leading_zeros=None, country_code_source=1, preferred_domestic_carrier_code=None)

Let me know if you cannot reproduce that, it's possible I'm doing something wrong still. I can tray and create a more contained reproduction as well.

Similarly, when I run python manage.py dumpdata the phonenumber field is shown like:

        "phone_number": "+14158675309",

Notice that this is not in international format, despite the settings, and that the phone number does not contain the extension. Is it possible I'm not doing the setting correctly?

[francoisfreitag]

You need to define PHONENUMBER_DEFAULT_FORMAT="INTERNATIONAL" (which is different from PHONENUMBER_DEFAULT_REGION).

>>> from phone.models import Person
>>> user = Person.objects.get()
>>> user.cell
PhoneNumber(country_code=33, national_number=555001489, extension='66', italian_leading_zero=None, number_of_leading_zeros=None, country_code_source=1, preferred_domestic_carrier_code=None)
>>> user.full_clean()
>>> user.cell
PhoneNumber(country_code=33, national_number=555001489, extension='66', italian_leading_zero=None, number_of_leading_zeros=None, country_code_source=1, preferred_domestic_carrier_code=None)

Let me know if that fixes the issue.

To confirm the settings values, you can rely on django python manage.py diffsettings | grep PHONENUMBER.

[jessebrennan]

Ah, I see! I didn't realize this was a different variable! Thanks for your patience. This did indeed fix the problem. It looks like your PR should cover this.

My testing seems to indicate that there is no issue, but I wanted to confirm; is there any risk in changing these variables with data already stored in the database? Initially I expected there to be a migration or something, but there wasn't. It seems that the field is parsed regardless of the format. It might be worth making that explicit in the docs as well.

Also feel free to close this issue, my problem is solved!

[francoisfreitag]

Thanks for your feedback, glad the issue is solved. :grin:

is there any risk in changing these variables with data already stored in the database?

I don’t think so, as you’ve noticed, the field is parsed regardless of the format. The cases where you would be in trouble:

1. Moving from an extension-compatible format to "E164" or NATIONAL
2. changing from PHONENUMBER_DB_FORMAT="NATIONAL" and changing PHONENUMBER_DEFAULT_REGION

It seems that the field is parsed regardless of the format. It might be worth making that explicit in the docs as well.

That’s correct. I’m tempted to leave it undocumented, as I don’t want to commit to this behavior. It’s the best way to obtain a PhoneNumber for now, but maybe there will be optimized helpers in the future that do not validate the phone number, or a good reason to change that behavior. I would rather avoid giving some guarantee for implementation details if that can be avoided.

[jessebrennan]

It makes sense to not want to commit to particular behavior. Unfortunately the current behavior is already being relied upon. Anyone who changes these variable with a database running is dependent upon the current behavior of flexible parsing. Changes to the parsing behavior will probably need to be documented (or else result in some nasty surprises).

Regardless, this information:

1. Moving from an extension-compatible format to "E164" or NATIONAL
2. changing from PHONENUMBER_DB_FORMAT="NATIONAL" and changing PHONENUMBER_DEFAULT_REGION

was very useful to me, and would probably be useful to other users who find that they need to change the db format. In my case it was to support extensions. I saw you were considering changing the default format to support extensions which would avoid cases like mine. I think that's a good idea!

[francoisfreitag]

Makes sense, I added a warning to the PHONENUMBER_DEFAULT_FORMAT setting in the documentation for these cases. (currently in PR #583)