From 56097dae42b020dbfc3f1aeb74b1c8c454bfc8b3 Mon Sep 17 00:00:00 2001
From: Paul Marillonnet <pmarillonnet@entrouvert.com>
Date: Wed, 21 Sep 2022 15:09:39 +0200
Subject: [PATCH 5/5] csv_import adapt user csv logic to new phone_number kind
 (#69365)

---
 src/authentic2/csv_import.py | 22 +++++++++++++++++++---
 tests/test_csv_import.py     | 30 ++++++++++++++++--------------
 tests/test_user_manager.py   | 13 ++++++-------
 3 files changed, 41 insertions(+), 24 deletions(-)

diff --git a/src/authentic2/csv_import.py b/src/authentic2/csv_import.py
index e3a375236..dd52db738 100644
--- a/src/authentic2/csv_import.py
+++ b/src/authentic2/csv_import.py
@@ -19,6 +19,7 @@ import csv
 import io
 
 import attr
+import phonenumbers
 from chardet.universaldetector import UniversalDetector
 from django import forms
 from django.contrib.auth.hashers import identify_hasher
@@ -27,6 +28,7 @@ from django.core.validators import RegexValidator
 from django.db import IntegrityError, models
 from django.db.transaction import atomic
 from django.utils.encoding import force_bytes, force_str
+from django.utils.timezone import now
 from django.utils.translation import gettext as _
 
 from authentic2 import app_settings
@@ -35,7 +37,7 @@ from authentic2.a2_rbac.utils import get_default_ou
 from authentic2.custom_user.models import User
 from authentic2.forms.profile import BaseUserForm, modelform_factory
 from authentic2.models import Attribute, AttributeValue, PasswordReset, UserExternalId
-from authentic2.utils.misc import send_password_reset_mail
+from authentic2.utils.misc import parse_phone_number, send_password_reset_mail
 
 
 # http://www.attrs.org/en/stable/changelog.html :
@@ -470,7 +472,7 @@ class UserCsvImporter:
                     header.key = True
             elif header.name not in SPECIAL_COLUMNS:
                 try:
-                    if header.name in ['email', 'first_name', 'last_name', 'username']:
+                    if header.name in ['email', 'phone', 'first_name', 'last_name', 'username']:
                         User._meta.get_field(header.name)
                         header.field = True
                         if header.name == 'email':
@@ -543,14 +545,26 @@ class UserCsvImporter:
 
     def parse_row(self, form_class, row, line):
         data = {}
-
+        errors = {}
         for header in self.headers:
             try:
                 data[header.name] = row[header.column - 1]
             except IndexError:
                 pass
 
+        if 'phone' in data:
+            pn = parse_phone_number(data['phone'])
+            if pn:
+                # fill multi value field
+                data['phone_0'] = str(pn.country_code)
+                data['phone_1'] = phonenumbers.format_number(pn, phonenumbers.PhoneNumberFormat.NATIONAL)
+                data.pop('phone')
+            else:
+                # E.164 compliant parsing failed, leave the number untouched, add an error
+                errors.update({'phone': [_('Enter a valid phone number.')]})
+
         form = form_class(data=data)
+        form.errors.update(errors)
         form.is_valid()
 
         def get_form_errors(form, name):
@@ -716,6 +730,8 @@ class UserCsvImporter:
                     setattr(user, cell.header.name, cell.value)
                     if cell.header.name == 'email' and cell.header.verified:
                         user.set_email_verified(True)
+                    if cell.header.name == 'phone' and cell.header.verified:
+                        user.phone_verified_on = now()
                     cell.action = 'updated'
                     continue
             cell.action = 'nothing'
diff --git a/tests/test_csv_import.py b/tests/test_csv_import.py
index 91c761968..035b4b609 100644
--- a/tests/test_csv_import.py
+++ b/tests/test_csv_import.py
@@ -201,7 +201,7 @@ x,x,x,x'''
         CsvHeader(1, 'email', field=True, key=True, verified=True),
         CsvHeader(2, 'first_name', field=True),
         CsvHeader(3, 'last_name', field=True),
-        CsvHeader(4, 'phone', attribute=True),
+        CsvHeader(4, 'phone', field=True),
     ]
     assert importer.has_errors
     assert len(importer.rows) == 3
@@ -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 not importer.rows[2].cells[3].errors
+    assert 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
@@ -225,7 +225,8 @@ x,x,x,x'''
     assert thomas.attributes.first_name == 'Thomas'
     assert thomas.last_name == 'Noël'
     assert thomas.attributes.last_name == 'Noël'
-    assert thomas.attributes.phone == '1234'
+    # phonenumbers' e.164 representation from a settings.DEFAULT_COUNTRY_CODE dial:
+    assert thomas.attributes.phone == '+331234'
     assert thomas.password
 
     fpeters = User.objects.get(email='fpeters@entrouvert.com')
@@ -235,7 +236,8 @@ x,x,x,x'''
     assert fpeters.attributes.first_name == 'Frédéric'
     assert fpeters.last_name == 'Péters'
     assert fpeters.attributes.last_name == 'Péters'
-    assert fpeters.attributes.phone == '5678'
+    # phonenumbers' e.164 representation from a settings.DEFAULT_COUNTRY_CODE dial:
+    assert fpeters.attributes.phone == '+335678'
 
 
 def test_simulate(profile, user_csv_importer_factory):
@@ -251,7 +253,7 @@ x,x,x,x'''
         CsvHeader(1, 'email', field=True, key=True, verified=True),
         CsvHeader(2, 'first_name', field=True),
         CsvHeader(3, 'last_name', field=True),
-        CsvHeader(4, 'phone', attribute=True),
+        CsvHeader(4, 'phone', field=True),
     ]
     assert importer.has_errors
     assert len(importer.rows) == 3
@@ -277,7 +279,7 @@ tnoel@entrouvert.com,Thomas,Noël,1234'''
     importer = user_csv_importer_factory(content)
 
     user = User.objects.create(ou=get_default_ou())
-    user.attributes.phone = '1234'
+    user.attributes.phone = '+331234'
 
     assert importer.run()
 
@@ -298,7 +300,7 @@ tnoel@entrouvert.com,Thomas,Noël,1234'''
     importer = user_csv_importer_factory(content)
 
     user = User.objects.create()
-    user.attributes.phone = '1234'
+    user.attributes.phone = '+331234'
 
     assert importer.run()
 
@@ -318,7 +320,7 @@ tnoel@entrouvert.com,Thomas,Noël,1234'''
     importer = user_csv_importer_factory(content)
 
     user = User.objects.create()
-    user.attributes.phone = '1234'
+    user.attributes.phone = '+331234'
 
     assert importer.run()
 
@@ -356,7 +358,7 @@ tnoel@entrouvert.com,Thomas,Noël,1234'''
     importer = user_csv_importer_factory(content)
 
     user = User.objects.create(ou=get_default_ou())
-    user.attributes.phone = '1234'
+    user.attributes.phone = '+331234'
 
     user = User.objects.create(email='tnoel@entrouvert.com', ou=get_default_ou())
 
@@ -378,7 +380,7 @@ tnoel@entrouvert.com,Thomas,Noël,1234'''
     importer = user_csv_importer_factory(content)
 
     user = User.objects.create()
-    user.attributes.phone = '1234'
+    user.attributes.phone = '+331234'
 
     User.objects.create(email='tnoel@entrouvert.com', ou=get_default_ou())
 
@@ -400,7 +402,7 @@ tnoel@entrouvert.com,Thomas,Noël,1234'''
     importer = user_csv_importer_factory(content)
 
     user = User.objects.create()
-    user.attributes.phone = '1234'
+    user.attributes.phone = '+331234'
 
     thomas = User.objects.create(email='tnoel@entrouvert.com', ou=get_default_ou())
 
@@ -418,7 +420,7 @@ tnoel@entrouvert.com,Thomas,Noël,1234'''
     thomas.refresh_from_db()
     assert not thomas.first_name
     assert not thomas.last_name
-    assert thomas.attributes.phone == '1234'
+    assert thomas.attributes.phone == '+331234'
 
 
 def test_external_id(profile, user_csv_importer_factory):
@@ -436,7 +438,7 @@ app1,2,tnoel@entrouvert.com,Thomas,Noël,1234
         CsvHeader(3, 'email', field=True, verified=True),
         CsvHeader(4, 'first_name', field=True),
         CsvHeader(5, 'last_name', field=True),
-        CsvHeader(6, 'phone', attribute=True),
+        CsvHeader(6, 'phone', field=True),
     ]
     assert not importer.has_errors
     assert len(importer.rows) == 2
@@ -449,7 +451,7 @@ app1,2,tnoel@entrouvert.com,Thomas,Noël,1234
         assert thomas.attributes.first_name == 'Thomas'
         assert thomas.last_name == 'Noël'
         assert thomas.attributes.last_name == 'Noël'
-        assert thomas.attributes.phone == '1234'
+        assert thomas.attributes.phone == '+331234'
 
     importer = user_csv_importer_factory(content)
     assert importer.run(), importer.errors
diff --git a/tests/test_user_manager.py b/tests/test_user_manager.py
index 06b965c30..f4f66e9fd 100644
--- a/tests/test_user_manager.py
+++ b/tests/test_user_manager.py
@@ -467,7 +467,7 @@ def test_user_import(encoding, transactional_db, app, admin, ou1, admin_ou1):
             'users.csv',
             '''email key verified,first_name,last_name,phone
 tnoel@entrouvert.com,Thomas,Noël,1234
-fpeters@entrouvert.com,Frédéric,Péters,5678
+fpeters@entrouvert.com,Frédéric,Péters,+325678
 john.doe@entrouvert.com,John,Doe,9101112
 x,x,x,x'''.encode(
                 encoding
@@ -529,7 +529,7 @@ x,x,x,x'''.encode(
     assert len(response.pyquery('tr.row-cells-errors')) == 1
     assert sum(bool(response.pyquery(td).text()) for td in response.pyquery('tr.row-cells-errors td li')) == 2
     assert 'Enter a valid email address' in response.pyquery('tr.row-cells-errors td.cell-email li').text()
-    assert 'Phone number can start with' in response.pyquery('tr.row-cells-errors td.cell-phone li').text()
+    assert 'Enter a valid phone number' in response.pyquery('tr.row-cells-errors td.cell-phone li').text()
 
     assert User.objects.count() == user_count
 
@@ -548,7 +548,7 @@ x,x,x,x'''.encode(
             email='tnoel@entrouvert.com',
             first_name='Thomas',
             last_name='Noël',
-            attribute_values__content='1234',
+            attribute_values__content='+331234',
         ).count()
         == 1
     )
@@ -557,7 +557,7 @@ x,x,x,x'''.encode(
             email='fpeters@entrouvert.com',
             first_name='Frédéric',
             last_name='Péters',
-            attribute_values__content='5678',
+            attribute_values__content='+325678',
         ).count()
         == 1
     )
@@ -566,7 +566,7 @@ x,x,x,x'''.encode(
             email='john.doe@entrouvert.com',
             first_name='John',
             last_name='Doe',
-            attribute_values__content='9101112',
+            attribute_values__content='+339101112',
         ).count()
         == 1
     )
@@ -696,7 +696,6 @@ def test_user_import_attributes(transactional_db, app, admin):
     assert 'Enter a valid date.' in response.text
     assert 'birthdate must be in the past and greater or equal than 1900-01-01.' in response.text
     assert 'The value must be a valid french postcode' in response.text
-    assert 'Phone number can start with a + and must contain only digits' in response.text
 
     assert User.objects.count() == user_count + 1
     elliot = User.objects.filter(email='elliot@universalpictures.com')[0]
@@ -706,7 +705,7 @@ def test_user_import_attributes(transactional_db, app, admin):
     assert elliot.attributes.values['saintsday'].content == '2019-07-20'
     assert elliot.attributes.values['birthdate'].content == '1972-05-26'
     assert elliot.attributes.values['zip'].content == '75014'
-    assert elliot.attributes.values['phone'].content == '1234'
+    assert elliot.attributes.values['phone'].content == '+331234'
 
     csv_lines[2] = "et@universalpictures.com,ET,the Extra-Terrestrial,,,,,,42000,+888 5678"
     response = import_csv('\n'.join(csv_lines), app)
-- 
2.38.1

