Implement 'send_email_alerts'

* cascading from team to project; user is base-level-default
* implemented at form-level
* implemented when emails are actually sent

alerts/tasks.py

@@ -3,14 +3,48 @@ from snappea.decorators import shared_task
 from django.template.defaultfilters import truncatechars
 
 from projects.models import ProjectMembership
+from teams.models import TeamMembership
 from bugsink.app_settings import get_settings
 
 from bugsink.utils import send_rendered_email
 
 
 def _get_users_for_email_alert(issue):
-    # more like memberships as currently implemented :-D
-    return ProjectMembership.objects.filter(project=issue.project, send_email_alerts=True).select_related("user")
+    # _perhaps_ it's possible to make some super-smart 3-way join that does the below, but I'd say that "just doing it
+    # with a (constant) few separate queries and some work in Python" is absolutely fine. (especially for something in
+    # an async task)
+
+    pms = list(
+        ProjectMembership.objects.filter(project=issue.project).exclude(send_email_alerts=False).select_related("user"))
+    user_ids = [pm.user_id for pm in pms]
+    tms = {tm.user_id: tm for tm in TeamMembership.objects.filter(team=issue.project.team, user_id__in=user_ids)}
+    for pm in pms:
+        if pm.send_email_alerts is True:
+            yield pm.user
+        # elif pm.send_email_alerts is False:   # we do this with the .exclude in the above
+        #     continue
+
+        else:  # (pm.send_email_alerts is None)
+
+            if pm.user_id in tms:
+
+                if tms[pm.user_id].send_email_alerts is True:
+                    yield pm.user
+                elif tms[pm.user_id].send_email_alerts is False:
+                    continue
+                else:   # tm exists, but is set to None
+                    if pm.user.send_email_alerts is True:
+                        yield pm.user
+                    elif pm.user.send_email_alerts is False:
+                        continue
+
+            else:  # no team-level definition
+                if pm.user.send_email_alerts is True:
+                    yield pm.user
+                elif pm.user.send_email_alerts is False:
+                    continue
+
+                # there is no None at this level
 
 
 @shared_task
@@ -32,11 +66,11 @@ def _send_alert(issue_id, state_description, alert_article, alert_reason, **kwar
     from issues.models import Issue  # avoid circular import
 
     issue = Issue.objects.get(id=issue_id)
-    for membership in _get_users_for_email_alert(issue):
+    for user in _get_users_for_email_alert(issue):
         send_rendered_email(
             subject=truncatechars(f'"{issue.title()}" in "{issue.project.name}" ({state_description})', 100),
             base_template_name="mails/issue_alert",
-            recipient_list=[membership.user.email],
+            recipient_list=[user.email],
             context={
                 "site_title": get_settings().SITE_TITLE,
                 "base_url": get_settings().BASE_URL + "/",

alerts/tests.py

@@ -7,8 +7,9 @@ from django.template.loader import get_template
 from issues.factories import get_or_create_issue
 from projects.models import Project, ProjectMembership
 from events.factories import create_event
+from teams.models import Team, TeamMembership
 
-from .tasks import send_new_issue_alert, send_regression_alert, send_unmute_alert
+from .tasks import send_new_issue_alert, send_regression_alert, send_unmute_alert, _get_users_for_email_alert
 from .views import DEBUG_CONTEXTS
 
 User = get_user_model()
@@ -83,3 +84,51 @@ class TestAlertSending(DjangoTestCase):
 
                 self.assertTrue(
                     "{{ %s" % variable in template.template.source, "'{{ %s ' not in %s template" % (variable, type_))
+
+    def test_get_users_for_email_alert(self):
+        team = Team.objects.create(name="Test team")
+        project = Project.objects.create(name="Test project", team=team)
+        user = User.objects.create_user(username="testuser", email="test@example.org", send_email_alerts=True)
+        issue, _ = get_or_create_issue(project=project)
+
+        # no ProjectMembership, user should not be included
+        self.assertEqual(list(_get_users_for_email_alert(issue)), [])
+
+        # ProjectMembership w/ send=False, should not be included
+        pm = ProjectMembership.objects.create(project=project, user=user, send_email_alerts=False)
+        self.assertEqual(list(_get_users_for_email_alert(issue)), [])
+
+        # ProjectMembership w/ send=True, should be included
+        pm.send_email_alerts = True
+        pm.save()
+        self.assertEqual(list(_get_users_for_email_alert(issue)), [user])
+
+        # Set send=None, fall back to User (which has True)
+        pm.send_email_alerts = None
+        pm.save()
+        self.assertEqual(list(_get_users_for_email_alert(issue)), [user])
+
+        # (User has False)
+        user.send_email_alerts = False
+        user.save()
+        self.assertEqual(list(_get_users_for_email_alert(issue)), [])
+
+        # Insert TeamMembership - this provides an intermediate layer of configuration between User and
+        # ProjectMembership; we start with send=True at the tm level and expect the user to be included
+        tm = TeamMembership.objects.create(team=team, user=user, send_email_alerts=True)
+        self.assertEqual(list(_get_users_for_email_alert(issue)), [user])
+
+        # Set send=False at the tm level, user should not be included
+        tm.send_email_alerts = False
+        tm.save()
+        self.assertEqual(list(_get_users_for_email_alert(issue)), [])
+
+        # Set send=None at the tm level, back to the user level (which is False)
+        tm.send_email_alerts = None
+        tm.save()
+        self.assertEqual(list(_get_users_for_email_alert(issue)), [])
+
+        # Set send=True at the user level, user should be included
+        user.send_email_alerts = True
+        user.save()
+        self.assertEqual(list(_get_users_for_email_alert(issue)), [user])

projects/forms.py

@@ -45,14 +45,20 @@ class MyProjectMembershipForm(forms.ModelForm):
         if not edit_role:
             del self.fields['role']
 
-        # True as default ... same TODO as in teams/forms.py
         try:
             tm = TeamMembership.objects.get(team=self.instance.project.team, user=self.instance.user)
-            team_send_email_alerts = tm.send_email_alerts if tm.send_email_alerts is not None else True
-        except TeamMembership.DoesNotExist:
-            team_send_email_alerts = True
+            if tm.send_email_alerts is not None:
+                sea_defined_at = "team membership"
+                sea_default = tm.send_email_alerts
+            else:
+                sea_defined_at = "user"
+                sea_default = self.instance.user.send_email_alerts
 
-        empty_label = "Team-default (currently: %s)" % yesno(team_send_email_alerts)
+        except TeamMembership.DoesNotExist:
+            sea_defined_at = "user"
+            sea_default = self.instance.user.send_email_alerts
+
+        empty_label = 'Default (%s, as per %s settings)' % (yesno(sea_default).capitalize(), sea_defined_at)
         self.fields['send_email_alerts'].empty_label = empty_label
         self.fields['send_email_alerts'].widget.choices[0] = ("unknown", empty_label)
 

projects/models.py

@@ -112,10 +112,6 @@ class ProjectMembership(models.Model):
     project = models.ForeignKey(Project, on_delete=models.CASCADE)
     user = models.ForeignKey(settings.AUTH_USER_MODEL, on_delete=models.CASCADE)
 
-    # TODO inheriting True/False for None from either Team (which we also don't have yet) or directly from
-    # User(Profile) is something we'll do later. At that point we'll probably implement it as denormalized here, so
-    # we'll just have to shift the currently existing field into send_email_alerts_denormalized and create a 3-way
-    # field.
     send_email_alerts = models.BooleanField(default=None, null=True)
 
     role = models.IntegerField(choices=ProjectRole.choices, default=ProjectRole.MEMBER)

teams/forms.py

@@ -42,9 +42,8 @@ class MyTeamMembershipForm(forms.ModelForm):
         if not edit_role:
             del self.fields['role']
 
-        # self.instance.user[.profile].send_email_alerts  TODO implement
-        global_send_email_alerts = True
-        empty_label = "User-default (currently: %s)" % yesno(global_send_email_alerts)
+        global_send_email_alerts = self.instance.user.send_email_alerts
+        empty_label = "User-default (%s)" % yesno(global_send_email_alerts).capitalize()
         self.fields['send_email_alerts'].empty_label = empty_label
         self.fields['send_email_alerts'].widget.choices[0] = ("unknown", empty_label)
 

users/migrations/0003_user_send_email_alerts.py

@@ -0,0 +1,18 @@
+# Generated by Django 4.2.13 on 2024-06-12 14:53
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('users', '0002_emailverification'),
+    ]
+
+    operations = [
+        migrations.AddField(
+            model_name='user',
+            name='send_email_alerts',
+            field=models.BooleanField(blank=True, default=True),
+        ),
+    ]

users/models.py

@@ -10,6 +10,13 @@ class User(AbstractUser):
     # > User model is sufficient for you. This model behaves identically to the default user model, but you’ll be able
     # > to customize it in the future if the need arises
 
+    # (The above is no longer the only reason for a custom User model, since we started introducing custom fields.
+    # Regarding those fields, there is some pressure in the docs to put UserProfile fields in a separate model, but
+    # as long as the number of fields is small I think the User model makes more sense. We can always push them out
+    # later)
+
+    send_email_alerts = models.BooleanField(default=True, blank=True)
+
     class Meta:
         db_table = 'auth_user'