From d10867a6701489904bc0462e4a56f51b0873ee98 Mon Sep 17 00:00:00 2001
From: Mikko Nieminen <mikko.nieminen@bih-charite.de>
Date: Tue, 3 Sep 2024 11:00:17 +0200
Subject: [PATCH] fix remote sync user additional email update (#1476)

---
 CHANGELOG.rst                                   |  1 +
 docs/source/major_changes.rst                   |  1 +
 projectroles/remote_projects.py                 |  8 +++++++-
 .../projectroles/remoteproject_sync.html        |  4 ++--
 projectroles/tests/test_remote_project_api.py   | 17 +++++++++++++++++
 5 files changed, 28 insertions(+), 3 deletions(-)

diff --git a/CHANGELOG.rst b/CHANGELOG.rst
index f3a4294d..e3fdb165 100644
--- a/CHANGELOG.rst
+++ b/CHANGELOG.rst
@@ -44,6 +44,7 @@ Fixed
 
 - **Projectroles**
     - ``BatchUpdateRolesMixin`` removal breaking tests in other repos (#1464)
+    - Remote sync crash on updating user with additional email (#1476)
 - **Timeline**
     - Deprecated link dict ``blank`` field assumed as mandatory (#1462)
 
diff --git a/docs/source/major_changes.rst b/docs/source/major_changes.rst
index 91dace5e..22f56ed2 100644
--- a/docs/source/major_changes.rst
+++ b/docs/source/major_changes.rst
@@ -18,6 +18,7 @@ Release Highlights
 
 - Update app setting rendering in remote sync UI
 - Fix project sidebar and dropdown app plugin link order
+- Fix remote sync crash on updating user with additional email
 - General bug fixes and minor updates
 
 
diff --git a/projectroles/remote_projects.py b/projectroles/remote_projects.py
index 3bf2e7d3..0dae1549 100644
--- a/projectroles/remote_projects.py
+++ b/projectroles/remote_projects.py
@@ -32,6 +32,7 @@
     AppSetting,
 )
 from projectroles.plugins import get_backend_api
+from projectroles.utils import build_secret
 
 
 app_settings = AppSettingAPI()
@@ -530,8 +531,13 @@ def _sync_user(self, uuid, user_data):
                 )
             )
         for e in add_emails:
+            if SODARUserAdditionalEmail.objects.filter(
+                user=user, email=e
+            ).exists():
+                continue
+            # TODO: Remove redundant secret once #1477 is implemented
             email_obj = SODARUserAdditionalEmail.objects.create(
-                user=user, email=e, verified=True
+                user=user, email=e, verified=True, secret=build_secret(16)
             )
             logger.info(
                 'Created user {} additional email "{}"'.format(
diff --git a/projectroles/templates/projectroles/remoteproject_sync.html b/projectroles/templates/projectroles/remoteproject_sync.html
index c1fcce50..ad8e2388 100644
--- a/projectroles/templates/projectroles/remoteproject_sync.html
+++ b/projectroles/templates/projectroles/remoteproject_sync.html
@@ -257,9 +257,9 @@ <h4>
                     {% elif a.type == 'JSON' %}
                       <td class="text-muted font-italic">Empty</td>
                     {% elif a.type == 'BOOLEAN' %}
-                      <td class="text-monospace">{% if a.value == '1' %}True{% else %}False{% endif %}</td>
+                      <td><code>{% if a.value == '1' %}True{% else %}False{% endif %}</code></td>
                     {% else %}
-                      <td class="text-monospace">{{ a.value | truncatechars:255 }}</td>
+                      <td><code>{{ a.value | truncatechars:255 }}</code></td>
                     {% endif %}
                     <td>{{ a.status | title }}</td>
                   </tr>
diff --git a/projectroles/tests/test_remote_project_api.py b/projectroles/tests/test_remote_project_api.py
index de047f9b..bf85126f 100644
--- a/projectroles/tests/test_remote_project_api.py
+++ b/projectroles/tests/test_remote_project_api.py
@@ -2398,6 +2398,23 @@ def test_update_user_add_email(self):
         self.assertEqual(email.email, ADD_EMAIL)
         self.assertEqual(email.verified, True)
 
+    def test_update_user_add_email_exists(self):
+        """Test sync with existing additional email on user"""
+        target_user = User.objects.get(sodar_uuid=SOURCE_USER_UUID)
+        SODARUserAdditionalEmail.objects.create(
+            user=target_user,
+            email=ADD_EMAIL,
+            verified=True,
+            secret=build_secret(16),
+        )
+        self.assertEqual(SODARUserAdditionalEmail.objects.count(), 1)
+        remote_data = self.default_data
+        remote_data['users'][SOURCE_USER_UUID]['additional_emails'] = [
+            ADD_EMAIL
+        ]
+        self.remote_api.sync_remote_data(self.source_site, remote_data)
+        self.assertEqual(SODARUserAdditionalEmail.objects.count(), 1)
+
     def test_update_user_add_email_delete(self):
         """Test sync with deleting existing additional email on user"""
         self.make_email(self.user_target, ADD_EMAIL)