forms/fields: provide clearer validation error for PhoneField (#88045)

src/authentic2/attribute_kinds.py

@@ -148,22 +148,23 @@ def get_title_choices():
 
 
 def validate_phone_number(value):
-    default_country = settings.PHONE_COUNTRY_CODES[settings.DEFAULT_COUNTRY_CODE]['region']
+    conf = []
+    for conf_key in (
+        'region',
+        'region_desc',
+        'example_value',
+    ):
+        conf.append(settings.PHONE_COUNTRY_CODES[settings.DEFAULT_COUNTRY_CODE].get(conf_key))
     try:
         phonenumbers.parse(value)
     except phonenumbers.NumberParseException:
         try:
             phonenumbers.parse(
                 value,
-                default_country,
+                conf[0],
             )
         except phonenumbers.NumberParseException:
-            raise ValidationError(
-                _(
-                    'Phone number must be either in E.164 globally unique format or dialable from'
-                    ' {code} country code ({country}).'
-                ).format(code=settings.DEFAULT_COUNTRY_CODE, country=default_country)
-            )
+            raise ValidationError(_(f'Phone number must be dialable from {conf[1]} (e.g. {conf[2]}).'))
 
 
 french_validate_phone_number = RegexValidator(

src/authentic2/forms/fields.py

@@ -224,15 +224,30 @@ class PhoneField(MultiValueField):
             country_code = data_list[0]
             data_list[0] = '+%s' % data_list[0]
             data_list[1] = clean_number(data_list[1])
-            dial = (
-                settings.PHONE_COUNTRY_CODES.get(country_code, {}).get('region', None)
-                or settings.PHONE_COUNTRY_CODES[settings.DEFAULT_COUNTRY_CODE]['region']
-            )
+
+            conf = []
+            for conf_key in (
+                'region',
+                'region_desc',
+                'example_value',
+            ):
+                conf.append(
+                    settings.PHONE_COUNTRY_CODES.get(country_code, {}).get(conf_key, None)
+                    or settings.PHONE_COUNTRY_CODES[settings.DEFAULT_COUNTRY_CODE][conf_key]
+                )
+            if all(conf[1:]):
+                validation_error_message = _(
+                    f'Invalid phone number. Phone number from {conf[1]} must respect local format '
+                    f'(e.g. {conf[2]}).'
+                )
+            else:
+                # missing human-friendly config elements, can't provide a clearer validation error message:
+                validation_error_message = _('Invalid phone number.')
             try:
-                pn = phonenumbers.parse(''.join(data_list), dial)
+                pn = phonenumbers.parse(''.join(data_list), conf[0])
             except phonenumbers.NumberParseException:
-                raise ValidationError(_('Invalid phone number.'))
+                raise ValidationError(validation_error_message)
             if not phonenumbers.is_valid_number(pn):
-                raise ValidationError(_('Invalid phone number.'))
+                raise ValidationError(validation_error_message)
             return phonenumbers.format_number(pn, phonenumbers.PhoneNumberFormat.E164)
         return ''

src/authentic2/settings.py

@@ -394,13 +394,41 @@ SELECT2_CSS = '/static/xstatic/select2.min.css'
 
 # Phone prefixes by country for phone number as authentication identifier
 PHONE_COUNTRY_CODES = {
-    '32': {'region': 'BE', 'region_desc': _('Belgium')},
-    '33': {'region': 'FR', 'region_desc': _('Metropolitan France')},
-    '262': {'region': 'RE', 'region_desc': _('Réunion')},
-    '508': {'region': 'PM', 'region_desc': _('Saint Pierre and Miquelon')},
-    '590': {'region': 'GP', 'region_desc': _('Guadeloupe')},
-    '594': {'region': 'GF', 'region_desc': _('French Guiana')},
-    '596': {'region': 'MQ', 'region_desc': _('Martinique')},
+    '32': {
+        'region': 'BE',
+        'region_desc': _('Belgium'),
+        'example_value': '042 11 22 33',
+    },
+    '33': {
+        'region': 'FR',
+        'region_desc': _('Metropolitan France'),
+        'example_value': '06 39 98 01 23',
+    },
+    '262': {
+        'region': 'RE',
+        'region_desc': _('Réunion'),
+        'example_value': '06 39 98 01 23',
+    },
+    '508': {
+        'region': 'PM',
+        'region_desc': _('Saint Pierre and Miquelon'),
+        'example_value': '06 39 98 01 23',
+    },
+    '590': {
+        'region': 'GP',
+        'region_desc': _('Guadeloupe'),
+        'example_value': '06 39 98 01 23',
+    },
+    '594': {
+        'region': 'GF',
+        'region_desc': _('French Guiana'),
+        'example_value': '06 39 98 01 23',
+    },
+    '596': {
+        'region': 'MQ',
+        'region_desc': _('Martinique'),
+        'example_value': '06 39 98 01 23',
+    },
 }
 
 DEFAULT_COUNTRY_CODE = '33'

tests/test_change_phone.py

@@ -224,17 +224,25 @@ def test_change_phone_wrong_input(app, nomail_user, user_ou1, phone_activated_au
     resp.form.set('phone_1', '12244666')
     resp.form.set('password', nomail_user.username)
     resp = resp.form.submit()
-    assert 'Invalid phone number.' in resp.pyquery('.error p')[0].text
+    assert (
+        'Invalid phone number. Phone number from Metropolitan France must respect local format (e.g. 06 39 98 01 23).'
+    ) == resp.pyquery('.error p')[0].text_content().strip()
+
+    resp.form.set('phone_0', '32')
+    resp.form.set('phone_1', '12244')
+    resp = resp.form.submit()
+    assert (
+        'Invalid phone number. Phone number from Belgium must respect local format (e.g. 042 11 22 33).'
+    ) == resp.pyquery('.error p')[0].text_content().strip()
 
     assert not SMSCode.objects.count()
     assert not Token.objects.count()
     resp.form.set('phone_1', 'abc')
     resp.form.set('password', nomail_user.username)
     resp = resp.form.submit()
-    assert (
-        'Phone number must be either in E.164 globally unique format or dialable from 33 country code (FR).'
-        in resp.pyquery('.error p')[0].text
-    )
+    assert ('Phone number must be dialable from Metropolitan France (e.g. 06 39 98 01 23).') == resp.pyquery(
+        '.error p'
+    )[0].text_content().strip()
     assert not SMSCode.objects.count()
     assert not Token.objects.count()
 

tests/test_registration.py

@@ -1012,10 +1012,9 @@ def test_registration_erroneous_phone_identifier(app, db, settings, phone_activa
     resp = app.get(reverse('registration_register'))
     resp.form.set('phone_1', 'thatsnotquiteit')
     resp = resp.form.submit()
-    assert (
-        'Phone number must be either in E.164 globally unique format or dialable from 33 country code (FR).'
-        in resp.pyquery('.error')[0].text_content()
-    )
+    assert ('Phone number must be dialable from Metropolitan France (e.g. 06 39 98 01 23).') == resp.pyquery(
+        '.error p'
+    )[0].text_content().strip()
 
 
 @responses.activate
@@ -1082,6 +1081,26 @@ def test_phone_registration_cancel(app, db, settings, freezer, phone_activated_a
     assert not SMSCode.objects.count()
 
 
+@responses.activate
+def test_phone_registration_wrong_input(app, db, settings, freezer, phone_activated_authn):
+    settings.SMS_URL = 'https://foo.whatever.none/'
+    responses.post('https://foo.whatever.none/', status=200)
+
+    resp = app.get(reverse('registration_register'))
+    resp.form.set('phone_1', '12244666')
+    resp = resp.form.submit()
+    assert (
+        'Invalid phone number. Phone number from Metropolitan France must respect local format (e.g. 06 39 98 01 23).'
+    ) == resp.pyquery('.error p')[0].text_content().strip()
+
+    resp.form.set('phone_0', '32')
+    resp.form.set('phone_1', '12244')
+    resp = resp.form.submit()
+    assert (
+        'Invalid phone number. Phone number from Belgium must respect local format (e.g. 042 11 22 33).'
+    ) == resp.pyquery('.error p')[0].text_content().strip()
+
+
 @responses.activate
 def test_phone_registration_improperly_configured(app, db, settings, freezer, caplog, phone_activated_authn):
     settings.SMS_URL = ''

tests/test_views.py

@@ -59,10 +59,9 @@ def test_phone_number_change_invalid_number(settings, app, simple_user):
     assert resp.pyquery('input#id_mobile_1')[0].value == 'def'
 
     resp = resp.form.submit()
-    assert (
-        'Phone number must be either in E.164 globally unique format or dialable from'
-        in resp.pyquery('.error').text()
-    )
+    assert ('Phone number must be dialable from Metropolitan France (e.g. 06 39 98 01 23).') == resp.pyquery(
+        '.error p'
+    )[0].text_content().strip()
 
     resp.form['mobile_1'] = '612345678'
     resp.form.submit().follow()