From 926aad5f728cef16f370021374c71240a1469eba Mon Sep 17 00:00:00 2001
From: Valentin Deniaud <vdeniaud@entrouvert.com>
Date: Tue, 23 Feb 2021 17:24:40 +0100
Subject: [PATCH] manager: show missing role recap in csv import (#50166)

---
 src/authentic2/csv_import.py                          |  7 +++++++
 .../authentic2/manager/user_import_report.html        |  3 +++
 tests/test_user_manager.py                            | 11 +++++++++++
 3 files changed, 21 insertions(+)

diff --git a/src/authentic2/csv_import.py b/src/authentic2/csv_import.py
index da6a5826b..e2f812bf9 100644
--- a/src/authentic2/csv_import.py
+++ b/src/authentic2/csv_import.py
@@ -359,6 +359,7 @@ class UserCsvImporter(object):
     updated = 0
     created = 0
     rows_with_errors = 0
+    _missing_roles = None
 
     def add_error(self, line_error):
         if not hasattr(line_error, 'line'):
@@ -368,6 +369,7 @@ class UserCsvImporter(object):
     def run(self, fd_or_str, encoding, ou=None, simulate=False):
         self.ou = ou or get_default_ou()
         self.errors = []
+        self._missing_roles = set()
         self.csv_importer = CsvImporter()
         self.max_user_id = User.objects.aggregate(max=models.Max('id'))['max'] or -1
 
@@ -567,6 +569,10 @@ class UserCsvImporter(object):
     def allow_duplicate_key(self):
         return ROLE_NAME in self.headers_by_name or ROLE_SLUG in self.headers_by_name
 
+    @property
+    def missing_roles(self):
+        return sorted(self._missing_roles or [])
+
     def check_unique_constraints(self, row, unique_map, user=None):
         ou_users = User.objects.filter(ou=self.ou)
         # ignore new users
@@ -740,6 +746,7 @@ class UserCsvImporter(object):
             elif cell.header.name == ROLE_SLUG:
                 role = Role.objects.get(slug=cell.value, ou=self.ou)
         except Role.DoesNotExist:
+            self._missing_roles.add(cell.value)
             cell.errors.append(
                 Error('role-not-found',
                       _('Role "%s" does not exist') % cell.value))
diff --git a/src/authentic2/manager/templates/authentic2/manager/user_import_report.html b/src/authentic2/manager/templates/authentic2/manager/user_import_report.html
index 04caf180c..984fc704b 100644
--- a/src/authentic2/manager/templates/authentic2/manager/user_import_report.html
+++ b/src/authentic2/manager/templates/authentic2/manager/user_import_report.html
@@ -82,6 +82,9 @@
         <li>{% blocktrans count created=importer.created %}{{ created }} user created{% plural %}{{ created }} users created{% endblocktrans %}</li>
         <li>{% blocktrans count updated=importer.updated %}{{ updated }} user updated{% plural %}{{ updated }} users updated{% endblocktrans %}</li>
         <li>{% blocktrans count error_rows=importer.rows_with_errors %}{{ error_rows }} row has error{% plural %}{{ error_rows }} rows have errors{% endblocktrans %}</li>
+        {% if importer.missing_roles %}
+        <li>{% trans "The following roles were missing:" %} {{ importer.missing_roles|join:", " }}</li>
+        {% endif %}
         <li>{% blocktrans with duration=report.duration %}import took {{ duration }}{% endblocktrans %}</li>
       </ul>
       <h3>{% trans "Details" %}</h3>
diff --git a/tests/test_user_manager.py b/tests/test_user_manager.py
index eef3bbc80..6438f74c4 100644
--- a/tests/test_user_manager.py
+++ b/tests/test_user_manager.py
@@ -681,6 +681,17 @@ Elliott,3'''
     assert 'matches too many user' in response.pyquery('tr.row-errors').text()
 
 
+def test_user_import_missing_roles_recap(transactional_db, app, admin):
+    content = '''first_name key,last_name,_role_name
+Elliott,Doe,test1
+Jane,Doe,test1
+John,Doe,test2'''
+    login(app, admin, '/manage/users/')
+    response = import_csv(content, app)
+
+    assert 'The following roles were missing: test1, test2' in response.text
+
+
 def test_manager_create_user_next(superuser_or_admin, app, ou1):
     login(app, superuser_or_admin, '/manage/')