Improve phone handling

docs/localization.rst

@@ -35,3 +35,8 @@ Currency conversion
 *******************
 
 Saleor can use currency exchange rate data to show price estimations in the visitor's local currency. Please consult :ref:`openexchangerates` for how to set this up for `Open Exchange Rates <https://openexchangerates.org/>`_.
+
+Phone numbers format
+********************
+
+Saleor uses `Google's libphonenumber library <https://github.com/googlei18n/libphonenumber>`_ to ensure provided numbers are correct. You need to choose prefix and type the number separately. No matter what country has been chosen, you may enter phone number belonging to any other country format.

saleor/static/scss/components/_forms.scss

@@ -18,6 +18,23 @@
 .form-group {
   text-align: left;
   position: relative;
+  .phone-prefix-input {
+    display: grid;
+    grid-template-columns: 6rem calc(100% - 6rem);
+    input {
+      border-left-color: transparent;
+      border: {
+        bottom-left-radius: 0;
+        top-left-radius: 0;
+      }
+    }
+    select {
+      border: {
+        bottom-right-radius: 0;
+        top-right-radius: 0;
+      }
+    }
+  }
 }
 
 .input-btn {
@@ -149,3 +166,11 @@ input#id_quantity {
   right: 0.5rem;
   cursor: pointer;
 }
+
+// ATM there is no better solution to set
+// border-radius on select inputs - 12.2017
+select.form-control {
+  appearance: none;
+  border-radius: 2px;
+  background: url('../images/arrow_select.svg') 100% 50% no-repeat / 24px 14px;
+}

saleor/userprofile/forms.py

@@ -3,26 +3,30 @@
 
 from django.conf import settings
 from django.contrib.auth import forms as django_forms, update_session_auth_hash
+from phonenumbers.phonenumberutil import country_code_for_region
 
 from .i18n import AddressMetaForm, get_address_form_class
 
 
 def get_address_form(data, country_code, initial=None, instance=None, **kwargs):
     country_form = AddressMetaForm(data, initial=initial)
     preview = False
-
     if country_form.is_valid():
         country_code = country_form.cleaned_data['country']
         preview = country_form.cleaned_data['preview']
 
+    if initial is None and country_code:
+        initial = {}
+    if country_code:
+        initial['phone'] = '+{}'.format(country_code_for_region(country_code))
+
     address_form_class = get_address_form_class(country_code)
 
     if not preview and instance is not None:
         address_form_class = get_address_form_class(
             instance.country.code)
         address_form = address_form_class(
-            data, instance=instance,
-            **kwargs)
+            data, instance=instance, **kwargs)
     else:
         initial_address = (
             initial if not preview

saleor/userprofile/i18n.py

@@ -5,10 +5,14 @@
 import i18naddress
 from django import forms
 from django.forms.forms import BoundField
-from django.utils.translation import pgettext, pgettext_lazy
 from django_countries.data import COUNTRIES
+from django.utils.translation import pgettext_lazy
+from phonenumber_field.formfields import PhoneNumberField
 
 from .models import Address
+from .widgets import PhonePrefixWidget
+from .validators import validate_possible_number
+
 
 COUNTRY_FORMS = {}
 UNKNOWN_COUNTRIES = set()
@@ -37,6 +41,20 @@
     'zip': pgettext_lazy('Address field', 'ZIP code')}
 
 
+class PossiblePhoneNumberFormField(PhoneNumberField):
+    """
+    Modify PhoneNumberField form field to allow using phone numbers from
+    countries other than selected one.
+    To achieve this both default_validator attribute and to_python method needs
+    to be overwritten.
+    """
+
+    default_validators = [validate_possible_number]
+
+    def to_python(self, value):
+        return value
+
+
 class AddressMetaForm(forms.ModelForm):
     # This field is never visible in UI
     preview = forms.BooleanField(initial=False, required=False)
@@ -74,6 +92,9 @@ class Meta:
         model = Address
         exclude = []
 
+    phone = PossiblePhoneNumberFormField(
+        widget=PhonePrefixWidget, required=False)
+
     def __init__(self, *args, **kwargs):
         autocomplete_type = kwargs.pop('autocomplete_type', None)
         super(AddressForm, self).__init__(*args, **kwargs)

saleor/userprofile/migrations/0015_auto_20171213_0734.py

@@ -0,0 +1,21 @@
+# -*- coding: utf-8 -*-
+# Generated by Django 1.11.5 on 2017-12-13 13:34
+from __future__ import unicode_literals
+
+from django.db import migrations
+import saleor.userprofile.models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('userprofile', '0014_auto_20171129_1004'),
+    ]
+
+    operations = [
+        migrations.AlterField(
+            model_name='address',
+            name='phone',
+            field=saleor.userprofile.models.PossiblePhoneNumberField(blank=True, default='', max_length=128, verbose_name='phone number'),
+        ),
+    ]

saleor/userprofile/models.py

@@ -10,6 +10,15 @@
 from django_countries.fields import Country, CountryField
 from phonenumber_field.modelfields import PhoneNumberField
 
+from .validators import validate_possible_number
+
+
+class PossiblePhoneNumberField(PhoneNumberField):
+    """
+    Less strict rule for phone numbers written to database.
+    """
+    default_validators = [validate_possible_number]
+
 
 class AddressManager(models.Manager):
 
@@ -72,7 +81,7 @@ class Address(models.Model):
     country_area = models.CharField(
         pgettext_lazy('Address field', 'state or province'),
         max_length=128, blank=True)
-    phone = PhoneNumberField(
+    phone = PossiblePhoneNumberField(
         verbose_name=pgettext_lazy('Address field', 'phone number'),
         blank=True, default='')
     objects = AddressManager()
@@ -94,7 +103,7 @@ def __repr__(self):
                 self.first_name, self.last_name, self.company_name,
                 self.street_address_1, self.street_address_2, self.city,
                 self.postal_code, self.country, self.country_area,
-                str(self.phone)))
+                self.phone))
 
 
 class UserManager(BaseUserManager):

saleor/userprofile/validators.py

@@ -0,0 +1,12 @@
+from django.core.exceptions import ValidationError
+from django.utils.translation import ugettext_lazy as _
+from phonenumber_field.phonenumber import to_python
+from phonenumbers.phonenumberutil import is_possible_number
+
+
+def validate_possible_number(value):
+    phone_number = to_python(value)
+    if phone_number and not is_possible_number(phone_number):
+        raise ValidationError(
+            _('The phone number entered is not valid.'),
+            code='invalid_phone_number')

saleor/userprofile/widgets.py

@@ -0,0 +1,24 @@
+from django.forms import Select, TextInput
+from phonenumber_field.widgets import PhoneNumberPrefixWidget
+from phonenumbers import COUNTRY_CODE_TO_REGION_CODE
+
+from .validators import validate_possible_number
+
+
+phone_prefixes = [
+    ('+{}'.format(k), '+{}'.format(k)) for
+    (k, v) in sorted(COUNTRY_CODE_TO_REGION_CODE.items())]
+
+
+class PhonePrefixWidget(PhoneNumberPrefixWidget):
+    """
+    Overwrite widget to use choices with tuple in a simple form of "+XYZ: +XYZ"
+    Workaround for an issue:
+    https://github.com/stefanfoulis/django-phonenumber-field/issues/82
+    """
+
+    template_name = 'userprofile/snippets/phone-prefix-widget.html'
+
+    def __init__(self, attrs=None):
+        widgets = (Select(attrs=attrs, choices=phone_prefixes), TextInput())
+        super(PhoneNumberPrefixWidget, self).__init__(widgets, attrs)

templates/userprofile/snippets/address-form.html

@@ -1,22 +1,22 @@
 {% load bootstrap_field from bootstrap3 %}
 <div class="i18n-address">
-    {% bootstrap_field address_form.phone %}
-    {% with address_form_lines=address_form.i18n_fields_order %}
-        {% if address_form_lines %}
-            {% for line in address_form_lines %}
-                <div class="row">
-                    {% for field in line %}
-                        <div class="col-6">
-                            {% bootstrap_field field %}
-                        </div>
-                    {% endfor %}
-                </div>
-            {% endfor %}
-        {% endif %}
-    {% endwith %}
-    {% for field in address_form.hidden_fields %}
-        {{ field }}
-    {% endfor %}
-    {% bootstrap_field address_form.country %}
-    <input type="hidden" class="preview" name="preview">
+  {% with address_form_lines=address_form.i18n_fields_order %}
+    {% if address_form_lines %}
+      {% for line in address_form_lines %}
+        <div class="row">
+          {% for field in line %}
+            <div class="col-6">
+              {% bootstrap_field field %}
+            </div>
+          {% endfor %}
+        </div>
+      {% endfor %}
+    {% endif %}
+  {% endwith %}
+  {% for field in address_form.hidden_fields %}
+    {{ field }}
+  {% endfor %}
+  {% bootstrap_field address_form.country %}
+  {% bootstrap_field address_form.phone %}
+  <input type="hidden" class="preview" name="preview">
 </div>

templates/userprofile/snippets/address-short.html

@@ -11,7 +11,7 @@
     {{ address.street_address_1 }} {% if address.street_address_2 %} {{ address.street_address_2 }} {% endif %}<br>
     {{ address.postal_code }} {{ address.city }}
     {% if address.country_area %}
-        {{ address.country_area }}, 
+        {{ address.country_area }},
     {% endif %}<br>
     {{ address.get_country_display }}<br>
     {{ address.phone }}</p>

templates/userprofile/snippets/phone-prefix-widget.html

@@ -0,0 +1,5 @@
+<div class="phone-prefix-input">
+  {% for widget in widget.subwidgets %}
+    {% include widget.template_name %}
+  {% endfor %}
+</div>

tests/test_userprofile.py

@@ -10,9 +10,11 @@
 
 import i18naddress
 import pytest
+from django.core.exceptions import ValidationError
 from django.http import QueryDict
 
 from saleor.userprofile import forms, models, i18n
+from saleor.userprofile.validators import validate_possible_number
 
 @pytest.fixture
 def billing_address(db):
@@ -30,7 +32,9 @@ def test_address_form_for_country(country):
     data = {
         'first_name': 'John',
         'last_name': 'Doe',
-        'country': country}
+        'country': country,
+        'phone': '123456789'}
+
     form = forms.get_address_form(data, country_code=country)[0]
     errors = form.errors
     rules = i18naddress.get_validation_rules({'country_code': country})
@@ -105,3 +109,18 @@ def test_country_aware_form_has_only_supported_countries():
     for country in i18n.UNKNOWN_COUNTRIES:
         assert country not in i18n.COUNTRY_FORMS
         assert country not in country_choices
+
+
+@pytest.mark.parametrize("input,exception", [
+    ('123', ValidationError),
+    ('+48123456789', None),
+    ('+12025550169', None),
+    ('+481234567890', ValidationError),
+    ('testext', ValidationError),
+])
+def test_validate_possible_number(input, exception):
+    if exception is not None:
+        with pytest.raises(exception):
+            validate_possible_number(input)
+    else:
+        validate_possible_number(input)