From 2c0443d1bf94d5cdc2b92361875bb6e928abf8bc Mon Sep 17 00:00:00 2001
From: Paul Marillonnet <pmarillonnet@entrouvert.com>
Date: Wed, 21 Sep 2022 11:38:11 +0200
Subject: [PATCH 4/5] attribute_kinds: use custom PhoneField for phone_number
 type (#69365)

---
 src/authentic2/attribute_kinds.py | 16 +------------
 tests/test_attribute_kinds.py     | 40 +++++++++++++++++++++----------
 tests/test_csv_import.py          |  2 +-
 tests/test_profile.py             | 26 ++++++++++----------
 tests/test_views.py               | 29 ++++++++++++++++++++++
 5 files changed, 71 insertions(+), 42 deletions(-)

diff --git a/src/authentic2/attribute_kinds.py b/src/authentic2/attribute_kinds.py
index ded475816..30727d15b 100644
--- a/src/authentic2/attribute_kinds.py
+++ b/src/authentic2/attribute_kinds.py
@@ -161,20 +161,6 @@ def clean_number(number):
     return cleaned_number
 
 
-class PhoneNumberField(forms.CharField):
-    widget = widgets.PhoneNumberInput
-
-    def __init__(self, *args, **kwargs):
-        kwargs['max_length'] = 30
-        kwargs.setdefault('help_text', _('ex.: 0699999999, +33 6 99 99 99 99'))
-        super().__init__(*args, **kwargs)
-
-    def clean(self, value):
-        if value not in self.empty_values:
-            value = clean_number(value)
-        return value
-
-
 class FrenchPhoneNumberField(forms.CharField):
     widget = widgets.PhoneNumberInput
 
@@ -362,7 +348,7 @@ DEFAULT_ATTRIBUTE_KINDS = [
     {
         'label': _('phone number'),
         'name': 'phone_number',
-        'field_class': PhoneNumberField,
+        'field_class': fields.PhoneField,
         'rest_framework_field_class': PhoneNumberDRFField,
     },
     {
diff --git a/tests/test_attribute_kinds.py b/tests/test_attribute_kinds.py
index 3eb32c1e7..b65d53abc 100644
--- a/tests/test_attribute_kinds.py
+++ b/tests/test_attribute_kinds.py
@@ -199,7 +199,7 @@ def test_phone_number(db, app, admin, mailoutbox, settings):
         return get_link_from_mail(mailoutbox[-1])
 
     Attribute.objects.create(
-        name='phone_number', label='phone', kind='phone_number', asked_on_registration=True
+        name='phone_number', label='Second Phone', kind='phone_number', asked_on_registration=True
     )
     qs = User.objects.filter(first_name='John')
 
@@ -208,26 +208,37 @@ def test_phone_number(db, app, admin, mailoutbox, settings):
     form = response.form
     form.set('first_name', 'John')
     form.set('last_name', 'Doe')
-    form.set('phone_number', 'abc')
+    form.set('phone_number_0', '33')
+    form.set('phone_number_1', 'abc')
     form.set('password1', '12345abcdA')
     form.set('password2', '12345abcdA')
     response = form.submit()
-    assert response.pyquery.find('.form-field-error #id_phone_number')
+    assert response.pyquery.find('.form-field-error #id_phone_number_0')
 
     form = response.form
-    assert response.pyquery('#id_phone_number').attr('maxlength') == '30'
-    form.set('phone_number', '1234512345' * 10)
+    assert not response.pyquery('#id_phone_number_1').attr('maxlength')
+    form.set('phone_number_0', '33')
+    form.set('phone_number_1', '1234512345' * 10)
     form.set('password1', '12345abcdA')
     form.set('password2', '12345abcdA')
     response = form.submit()
-    assert response.pyquery.find('.form-field-error #id_phone_number')
+    assert response.pyquery.find('.form-field-error #id_phone_number_0')
 
     form = response.form
-    form.set('phone_number', '12345')
+    form.set('phone_number_0', '33')
+    form.set('phone_number_1', ' +  1.2-3 4 5 ')
+    form.set('password1', '12345abcdA')
+    form.set('password2', '12345abcdA')
+    response = form.submit()
+    assert response.pyquery.find('.form-field-error #id_phone_number_0')
+
+    form = response.form
+    form.set('phone_number_0', '33')
+    form.set('phone_number_1', '12345')
     form.set('password1', '12345abcdA')
     form.set('password2', '12345abcdA')
     response = form.submit().follow()
-    assert qs.get().attributes.phone_number == '12345'
+    assert qs.get().attributes.phone_number == '+3312345'
     qs.delete()
 
     url = register_john()
@@ -235,11 +246,12 @@ def test_phone_number(db, app, admin, mailoutbox, settings):
     form = response.form
     form.set('first_name', 'John')
     form.set('last_name', 'Doe')
-    form.set('phone_number', '+12345')
+    form.set('phone_number_0', '32')
+    form.set('phone_number_1', '12345')
     form.set('password1', '12345abcdA')
     form.set('password2', '12345abcdA')
     response = form.submit().follow()
-    assert qs.get().attributes.phone_number == '+12345'
+    assert qs.get().attributes.phone_number == '+3212345'
     qs.delete()
 
     url = register_john()
@@ -247,7 +259,8 @@ def test_phone_number(db, app, admin, mailoutbox, settings):
     form = response.form
     form.set('first_name', 'John')
     form.set('last_name', 'Doe')
-    form.set('phone_number', '')
+    form.set('phone_number_0', '33')
+    form.set('phone_number_1', '')
     form.set('password1', '12345abcdA')
     form.set('password2', '12345abcdA')
     response = form.submit().follow()
@@ -259,11 +272,12 @@ def test_phone_number(db, app, admin, mailoutbox, settings):
     form = response.form
     form.set('first_name', 'John')
     form.set('last_name', 'Doe')
-    form.set('phone_number', ' +  1.2-3 4 5 ')
+    form.set('phone_number_0', '33')
+    form.set('phone_number_1', '1 2 3 4 5')
     form.set('password1', '12345abcdA')
     form.set('password2', '12345abcdA')
     response = form.submit().follow()
-    assert qs.get().attributes.phone_number == '+12345'
+    assert qs.get().attributes.phone_number == '+3312345'
     qs.delete()
 
 
diff --git a/tests/test_csv_import.py b/tests/test_csv_import.py
index 9409eae6f..3a3d5def6 100644
--- a/tests/test_csv_import.py
+++ b/tests/test_csv_import.py
@@ -211,7 +211,7 @@ x,x,x,x'''
     assert all(error == Error('data-error') for error in importer.rows[2].cells[0].errors)
     assert not importer.rows[2].cells[1].errors
     assert not importer.rows[2].cells[2].errors
-    assert importer.rows[2].cells[3].errors
+    assert not importer.rows[2].cells[3].errors
     assert all(error == Error('data-error') for error in importer.rows[2].cells[3].errors)
 
     assert importer.updated == 0
diff --git a/tests/test_profile.py b/tests/test_profile.py
index 594bc148b..6764324c2 100644
--- a/tests/test_profile.py
+++ b/tests/test_profile.py
@@ -46,8 +46,8 @@ def test_account_edit_view(app, simple_user):
     )
 
     resp = old_resp = app.get(url, status=200)
-    resp.form['phone'] = '1234'
-    assert resp.form['phone'].attrs['type'] == 'tel'
+    resp.form['phone_1'] = '1234'
+    assert resp.form['phone_1'].attrs['type'] == 'text'
     resp.form['title'] = 'Mrs'
     resp.form['agreement'] = False
     assert resp.form['language'].tag == 'select'
@@ -56,7 +56,7 @@ def test_account_edit_view(app, simple_user):
     resp = resp.form.submit()
     # verify that missing next_url in POST is ok
     assert resp['Location'].endswith(reverse('account_management'))
-    assert phone.get_value(simple_user) == '1234'
+    assert phone.get_value(simple_user) == '+331234'
     assert title.get_value(simple_user) == 'Mrs'
     assert agreement.get_value(simple_user) is False
     assert language.get_value(simple_user) == 'fr'
@@ -70,22 +70,22 @@ def test_account_edit_view(app, simple_user):
         ('First name', 'Jôhn'),
         ('Last name', 'Dôe'),
         ('Email address', 'user@example.net'),
-        ('Phone', '1234'),
+        ('Phone', '+331234'),
         ('Title', 'Mrs'),
         ('Language', 'French'),
     ]
 
     resp = app.get(url, status=200)
-    resp.form.set('phone', '0123456789')
+    resp.form.set('phone_1', '0123456789')
     resp = resp.form.submit().follow()
-    assert phone.get_value(simple_user) == '0123456789'
+    assert phone.get_value(simple_user) == '+33123456789'
 
     resp = app.get(url, status=200)
-    resp.form.set('phone', '9876543210')
+    resp.form.set('phone_1', '9876543210')
     resp = resp.form.submit('cancel').follow()
-    assert phone.get_value(simple_user) == '0123456789'
+    assert phone.get_value(simple_user) == '+33123456789'
 
-    phone.set_value(simple_user, '0123456789', verified=True)
+    phone.set_value(simple_user, '+33123456789', verified=True)
     title.set_value(simple_user, 'Mr', verified=True)
     agreement.set_value(simple_user, True, verified=True)
     resp = app.get(url, status=200)
@@ -93,19 +93,19 @@ def test_account_edit_view(app, simple_user):
     assert 'title' not in resp.form.fields
     assert 'agreement' not in resp.form.fields
     assert 'readonly' in resp.form['phone@disabled'].attrs
-    assert resp.form['phone@disabled'].value == '0123456789'
+    assert resp.form['phone@disabled'].value == '+33123456789'
     assert resp.form['title@disabled'].value == 'Mr'
     assert resp.form['agreement@disabled'].value == 'Yes'
     resp.form.set('phone@disabled', '1234')
     resp.form.set('title@disabled', 'Mrs')
     resp.form.set('agreement@disabled', 'False')
     resp = resp.form.submit().follow()
-    assert phone.get_value(simple_user) == '0123456789'
+    assert phone.get_value(simple_user) == '+33123456789'
     assert title.get_value(simple_user) == 'Mr'
     assert agreement.get_value(simple_user) is True
 
     resp = old_resp.form.submit()
-    assert phone.get_value(simple_user) == '0123456789'
+    assert phone.get_value(simple_user) == '+33123456789'
     assert title.get_value(simple_user) == 'Mr'
     assert agreement.get_value(simple_user) is True
 
@@ -115,7 +115,7 @@ def test_account_edit_view(app, simple_user):
     assert 'phone@disabled' not in resp
     assert 'title@disabled' in resp
     assert 'agreement@disabled' in resp
-    assert phone.get_value(simple_user) == '0123456789'
+    assert phone.get_value(simple_user) == '+33123456789'
 
 
 def test_account_edit_next_url(app, simple_user, external_redirect_next_url, assert_external_redirect):
diff --git a/tests/test_views.py b/tests/test_views.py
index 574a806b0..b7a4e2ab8 100644
--- a/tests/test_views.py
+++ b/tests/test_views.py
@@ -25,6 +25,7 @@ from django.utils.html import escape
 
 from authentic2.custom_user.models import DeletedUser, User
 from authentic2.forms.passwords import PasswordChangeForm, SetPasswordForm
+from authentic2.models import Attribute
 
 from .utils import assert_event, get_link_from_mail, login, logout
 
@@ -37,6 +38,34 @@ def test_profile(app, simple_user):
     assert simple_user.last_name in page
 
 
+def test_phone_number_change_invalid_number(settings, app, simple_user):
+    settings.A2_PROFILE_FIELDS = ('phone', 'mobile')
+
+    Attribute.objects.create(
+        kind='phone_number',
+        name='mobile',
+        label='Mobile',
+        user_visible=True,
+        user_editable=True,
+    )
+
+    simple_user.attributes.mobile = 'def'  # invalid number
+    resp = login(app, simple_user, path='/accounts/edit/')
+
+    assert resp.pyquery('input#id_mobile_1')[0].value == 'def'
+
+    resp = resp.form.submit()
+    assert (
+        'Phone number can start with a + and must contain only digits.' in resp.pyquery('div.error')[0].text
+    )
+
+    resp.form['mobile_1'] = '612345678'
+    resp.form.submit().follow()
+    simple_user.refresh_from_db()
+
+    assert simple_user.attributes.mobile == '+33612345678'
+
+
 def test_password_change(app, simple_user):
     simple_user.set_password('hop')
     simple_user.save()
-- 
2.38.1

