update logic, remove app setting, update tests (#874)

CHANGELOG.rst

@@ -102,6 +102,7 @@ Removed
     - ``PROJECTROLES_HIDE_APP_LINKS`` setting (#1143)
     - ``CORE_API_*`` Django settings (#1278)
     - Project starring timeline event creation (#1294)
+    - ``user_email_additional`` app setting (#874)
 
 
 v0.13.4 (2024-02-16)

adminalerts/tests/test_views.py

@@ -9,6 +9,7 @@
 
 # Projectroles dependency
 from projectroles.app_settings import AppSettingAPI
+from projectroles.tests.test_models import SODARUserAdditionalEmailMixin
 
 from adminalerts.models import AdminAlert
 from adminalerts.tests.test_models import AdminAlertMixin
@@ -26,9 +27,13 @@
 ALERT_DESC_UPDATED = 'Updated description'
 ALERT_DESC_MARKDOWN = '## Description'
 EMAIL_DESC_LEGEND = 'Additional details'
+ADD_EMAIL = '[email protected]'
+ADD_EMAIL2 = '[email protected]'
 
 
-class AdminalertsViewTestBase(AdminAlertMixin, TestCase):
+class AdminalertsViewTestBase(
+    AdminAlertMixin, SODARUserAdditionalEmailMixin, TestCase
+):
     """Base class for adminalerts view testing"""
 
     def _make_alert(self):
@@ -199,16 +204,10 @@ def test_post_multiple_users(self):
         self.assertIn(EMAIL_DESC_LEGEND, mail.outbox[0].body)
         self.assertIn(ALERT_DESC, mail.outbox[0].body)
 
-    def test_post_alt_email_regular_user(self):
-        """Test POST with alt emails on regular user"""
-        alt_email = '[email protected]'
-        alt_email2 = '[email protected]'
-        app_settings.set(
-            'projectroles',
-            'user_email_additional',
-            ';'.join([alt_email, alt_email2]),
-            user=self.user_regular,
-        )
+    def test_post_add_email_regular_user(self):
+        """Test POST with additional emails on regular user"""
+        self.make_email(self.user_regular, ADD_EMAIL)
+        self.make_email(self.user_regular, ADD_EMAIL2)
         self.assertEqual(len(mail.outbox), 0)
         data = self._get_post_data()
         with self.login(self.superuser):
@@ -220,28 +219,41 @@ def test_post_alt_email_regular_user(self):
             [
                 settings.EMAIL_SENDER,
                 self.user_regular.email,
-                alt_email,
-                alt_email2,
+                ADD_EMAIL,
+                ADD_EMAIL2,
             ],
         )
 
-    def test_post_alt_email_superuser(self):
-        """Test POST with alt emails on superuser"""
-        alt_email = '[email protected]'
-        alt_email2 = '[email protected]'
-        app_settings.set(
-            'projectroles',
-            'user_email_additional',
-            ';'.join([alt_email, alt_email2]),
-            user=self.superuser,
+    def test_post_add_email_regular_user_unverified(self):
+        """Test POST with additional and unverified emails on regular user"""
+        self.make_email(self.user_regular, ADD_EMAIL)
+        self.make_email(self.user_regular, ADD_EMAIL2, verified=False)
+        self.assertEqual(len(mail.outbox), 0)
+        data = self._get_post_data()
+        with self.login(self.superuser):
+            response = self.client.post(self.url, data)
+        self.assertEqual(response.status_code, 302)
+        self.assertEqual(len(mail.outbox), 1)
+        self.assertEqual(
+            mail.outbox[0].recipients(),
+            [
+                settings.EMAIL_SENDER,
+                self.user_regular.email,
+                ADD_EMAIL,
+            ],
         )
+
+    def test_post_add_email_superuser(self):
+        """Test POST with additional emails on superuser"""
+        self.make_email(self.superuser, ADD_EMAIL)
+        self.make_email(self.superuser, ADD_EMAIL2)
         self.assertEqual(len(mail.outbox), 0)
         data = self._get_post_data()
         with self.login(self.superuser):
             response = self.client.post(self.url, data)
         self.assertEqual(response.status_code, 302)
         self.assertEqual(len(mail.outbox), 1)
-        # Superuser alt emails should not be included
+        # Superuser additional emails should not be included
         self.assertEqual(
             mail.outbox[0].recipients(),
             [settings.EMAIL_SENDER, self.user_regular.email],

docs/source/app_userprofile.rst

@@ -86,9 +86,6 @@ Receive Email for Admin Alerts
 Display Project UUID Copying Link
     If set true, display a link in the project title bar for copying the project
     UUID into the clipboard.
-Additional Email
-    In addition to the default user email, also send email notifications to
-    these addresses.
 Receive Email for Project Updates
     Receive email notifications for project or category creation, updating,
     moving and archiving.

docs/source/major_changes.rst

@@ -30,6 +30,7 @@ Release Highlights
 - Add checkusers management command
 - Add CC and BCC field support in sending generic emails
 - Rewrite sodarcache REST API views
+- Rewrite user additional email storing
 - Rename AppSettingAPI "app_name" arguments to "plugin_name"
 - Plugin API return data updates and deprecations
 - Rename timeline app models
@@ -235,6 +236,14 @@ Note that a dict using the ``category`` variable as a key will still be
 provided for your app's search template. Hence, modifying the template should
 not be required after updating the method.
 
+User Additional Email Changes
+-----------------------------
+
+The ``user_email_additional`` app setting has been removed. Instead, additional
+user email addresses can be accessed via the ``SODARUserAdditionalEmail`` model
+if needed. Using ``email.get_user_addr()`` to retrieve all user email addresses
+is strongly recommended.
+
 Remote Sync User Update Changes
 -------------------------------
 

projectroles/app_settings.py

@@ -98,18 +98,6 @@
         'user_modifiable': True,
         'global': True,
     },
-    'user_email_additional': {
-        'scope': APP_SETTING_SCOPE_USER,
-        'type': 'STRING',
-        'default': '',
-        'placeholder': '[email protected];[email protected]',
-        'label': 'Additional email',
-        'description': 'Also send user emails to these addresses. Separate '
-        'multiple emails with semicolon.',
-        'user_modifiable': True,
-        'global': True,
-        'project_types': [PROJECT_TYPE_PROJECT],
-    },
     'project_star': {
         'scope': APP_SETTING_SCOPE_PROJECT_USER,
         'type': 'BOOLEAN',

projectroles/email.py

@@ -10,6 +10,7 @@
 from django.utils.timezone import localtime
 
 from projectroles.app_settings import AppSettingAPI
+from projectroles.models import SODARUserAdditionalEmail
 from projectroles.utils import build_invite_url, get_display_name
 
 
@@ -380,31 +381,20 @@ def get_role_change_body(
 
 def get_user_addr(user):
     """
-    Return all the email addresses for a user as a list. Emails set with
-    user_email_additional are included. If a user has no main email set but
-    additional emails exist, the latter are returned.
+    Return all the email addresses for a user as a list. Verified emails set as
+    SODARUserAdditionalEmail objects are included. If a user has no main email
+    set but additional emails exist, the latter are returned.
 
     :param user: User object
     :return: list
     """
-
-    def _validate(user, email):
-        if re.match(EMAIL_RE, email):
-            return True
-        logger.error(
-            'Invalid email for user {}: {}'.format(user.username, email)
-        )
-
     ret = []
-    if user.email and _validate(user, user.email):
+    if user.email:
         ret.append(user.email)
-    add_email = app_settings.get(
-        'projectroles', 'user_email_additional', user=user
-    )
-    if add_email:
-        for e in add_email.strip().split(';'):
-            if _validate(user, e):
-                ret.append(e)
+    for e in SODARUserAdditionalEmail.objects.filter(
+        user=user, verified=True
+    ).order_by('email'):
+        ret.append(e.email)
     return ret
 
 

projectroles/migrations/0030_populate_sodaruseradditionalemail.py

@@ -0,0 +1,48 @@
+# Generated by Django 4.2.11 on 2024-04-25 13:14
+
+import random
+import string
+
+from django.db import migrations
+
+
+def populate_model(apps, schema_editor):
+    """
+    Move existing additional email app settings into the
+    SODARUserAdditionalEmail model as verified emais. Delete the settings
+    objects.
+    """
+    AppSetting = apps.get_model('projectroles', 'AppSetting')
+    SODARUserAdditionalEmail = apps.get_model(
+        'projectroles', 'SODARUserAdditionalEmail'
+    )
+    for a in AppSetting.objects.filter(
+        app_plugin=None, name='user_email_additional'
+    ):
+        for v in a.value.split(';'):
+            secret = ''.join(
+                random.SystemRandom().choice(
+                    string.ascii_lowercase + string.digits
+                )
+                for _ in range(32)
+            )
+            try:
+                SODARUserAdditionalEmail.objects.create(
+                    user=a.user, email=v, verified=True, secret=secret
+                )
+            except Exception:
+                pass
+        a.delete()
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ("projectroles", "0029_sodaruseradditionalemail"),
+    ]
+
+    operations = [
+        migrations.RunPython(
+            populate_model, reverse_code=migrations.RunPython.noop
+        )
+    ]

projectroles/tests/test_app_settings.py

@@ -518,20 +518,20 @@ def test_set_target_global_remote(self):
 
     def test_set_user_global(self):
         """Test setting global user setting on source site"""
-        n = 'user_email_additional'
-        v = '[email protected]'
+        n = 'notify_email_project'
+        v = '0'
         self.assertEqual(AppSetting.objects.filter(name=n, value=v).count(), 0)
-        app_settings.set('projectroles', n, v, user=self.user)
+        app_settings.set('projectroles', n, False, user=self.user)
         self.assertEqual(AppSetting.objects.filter(name=n, value=v).count(), 1)
 
     @override_settings(PROJECTROLES_SITE_MODE=SITE_MODE_TARGET)
     def test_set_user_global_target(self):
         """Test setting global user setting on target site"""
-        n = 'user_email_additional'
-        v = '[email protected]'
+        n = 'notify_email_project'
+        v = '0'
         self.assertEqual(AppSetting.objects.filter(name=n, value=v).count(), 0)
         with self.assertRaises(ValueError):
-            app_settings.set('projectroles', n, v, user=self.user)
+            app_settings.set('projectroles', n, False, user=self.user)
         self.assertEqual(AppSetting.objects.filter(name=n, value=v).count(), 0)
 
     def test_validate_boolean(self):