Skip to content

Commit

Permalink
add admin alert email sending (wip) (#415)
Browse files Browse the repository at this point in the history
  • Loading branch information
mikkonie committed Apr 23, 2024
1 parent 52837dc commit ba5ce08
Show file tree
Hide file tree
Showing 6 changed files with 193 additions and 20 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG.rst
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ Added
- **General**
- Python v3.11 support (#1157)
- Flake8 rule in ``Makefile`` (#1387)
- **Adminalerts**
- Admin alert email sending (#415)
- **Filesfolders**
- Optional pagination for REST API list views (#1313)
- **Projectroles**
Expand All @@ -29,6 +31,7 @@ Added
- ``SODARPageNumberPagination`` pagination class (#1313)
- Optional pagination for REST API list views (#1313)
- Email notification opt-out settings (#1417)
- CC and BCC field support in sending generic emails (#415)
- **Timeline**
- ``sodar_uuid`` field in ``TimelineEventObjectRef`` model (#1415)
- REST API views (#1350)
Expand Down
17 changes: 17 additions & 0 deletions adminalerts/forms.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,16 +9,28 @@
from adminalerts.models import AdminAlert


# Local constants
EMAIL_HELP_CREATE = 'Send alert as email to all users on this site'
EMAIL_HELP_UPDATE = 'Send updated alert as email to all users on this site'


class AdminAlertForm(SODARModelForm):
"""Form for AdminAlert creation/updating"""

send_email = forms.BooleanField(
initial=True,
label='Send alert as email',
required=False,
)

class Meta:
model = AdminAlert
fields = [
'message',
'date_expire',
'active',
'require_auth',
'send_email',
'description',
]

Expand All @@ -40,13 +52,18 @@ def __init__(self, current_user=None, *args, **kwargs):

# Creation
if not self.instance.pk:
# TODO: Why don't we use self.initial here?
self.fields['date_expire'].initial = (
timezone.now() + timezone.timedelta(days=1)
)
self.fields['send_email'].help_text = EMAIL_HELP_CREATE
# Updating
else: # self.instance.pk
# Set description value as raw markdown
self.initial['description'] = self.instance.description.raw
self.fields['send_email'].help_text = EMAIL_HELP_UPDATE
# Sending email for update should be false by default
self.initial['send_email'] = False

def clean(self):
"""Custom form validation and cleanup"""
Expand Down
59 changes: 47 additions & 12 deletions adminalerts/tests/test_views.py
Original file line number Diff line number Diff line change
@@ -1,17 +1,29 @@
"""Tests for UI views in the adminalerts app"""

from django.conf import settings
from django.core import mail
from django.urls import reverse
from django.utils import timezone

from test_plus.test import TestCase

from adminalerts.models import AdminAlert
from adminalerts.tests.test_models import AdminAlertMixin
from adminalerts.views import EMAIL_SUBJECT


class AdminalertsViewTestBase(AdminAlertMixin, TestCase):
"""Base class for adminalerts view testing"""

def _make_alert(self):
return self.make_alert(
message='alert',
user=self.superuser,
description='description',
active=True,
require_auth=True,
)

def setUp(self):
# Create users
self.superuser = self.make_user('superuser')
Expand All @@ -21,14 +33,6 @@ def setUp(self):
self.regular_user = self.make_user('regular_user')
# No user
self.anonymous = None
# Create alert
self.alert = self.make_alert(
message='alert',
user=self.superuser,
description='description',
active=True,
require_auth=True,
)
self.expiry_str = (
timezone.now() + timezone.timedelta(days=1)
).strftime('%Y-%m-%d')
Expand All @@ -37,6 +41,10 @@ def setUp(self):
class TestAdminAlertListView(AdminalertsViewTestBase):
"""Tests for AdminAlertListView"""

def setUp(self):
super().setUp()
self.alert = self._make_alert()

def test_get(self):
"""Test AdminAlertListView GET"""
with self.login(self.superuser):
Expand All @@ -49,6 +57,10 @@ def test_get(self):
class TestAdminAlertDetailView(AdminalertsViewTestBase):
"""Tests for AdminAlertDetailView"""

def setUp(self):
super().setUp()
self.alert = self._make_alert()

def test_get(self):
"""Test AdminAlertDetailView GET"""
with self.login(self.superuser):
Expand Down Expand Up @@ -77,7 +89,8 @@ def test_get(self):

def test_post(self):
"""Test POST"""
self.assertEqual(AdminAlert.objects.all().count(), 1)
self.assertEqual(AdminAlert.objects.all().count(), 0)
self.assertEqual(len(mail.outbox), 0)
post_data = {
'message': 'new alert',
'description': 'description',
Expand All @@ -89,11 +102,26 @@ def test_post(self):
response = self.client.post(self.url, post_data)
self.assertEqual(response.status_code, 302)
self.assertEqual(response.url, reverse('adminalerts:list'))
self.assertEqual(AdminAlert.objects.all().count(), 2)
self.assertEqual(AdminAlert.objects.all().count(), 1)
self.assertEqual(len(mail.outbox), 1)
self.assertIn(
EMAIL_SUBJECT.format(state='New', message='new alert'),
mail.outbox[0].subject,
)
self.assertEqual(
mail.outbox[0].recipients(),
[settings.EMAIL_SENDER, self.regular_user.email],
)
# TODO: Assert message and description

# TODO: Test with no description
# TODO: Test with email set false
# TODO: Test with email set true and alt emails on user
# TODO: Test with email notifications disabled

def test_post_expired(self):
"""Test POST with old expiry date (should fail)"""
self.assertEqual(AdminAlert.objects.all().count(), 1)
self.assertEqual(AdminAlert.objects.all().count(), 0)
expiry_fail = (timezone.now() + timezone.timedelta(days=-1)).strftime(
'%Y-%m-%d'
)
Expand All @@ -107,14 +135,15 @@ def test_post_expired(self):
with self.login(self.superuser):
response = self.client.post(self.url, post_data)
self.assertEqual(response.status_code, 200)
self.assertEqual(AdminAlert.objects.all().count(), 1)
self.assertEqual(AdminAlert.objects.all().count(), 0)


class TestAdminAlertUpdateView(AdminalertsViewTestBase):
"""Tests for AdminAlertUpdateView"""

def setUp(self):
super().setUp()
self.alert = self._make_alert()
self.url = reverse(
'adminalerts:update',
kwargs={'adminalert': self.alert.sodar_uuid},
Expand All @@ -129,6 +158,7 @@ def test_get(self):
def test_post(self):
"""Test POST"""
self.assertEqual(AdminAlert.objects.all().count(), 1)
self.assertEqual(len(mail.outbox), 0)
post_data = {
'message': 'updated alert',
'description': 'updated description',
Expand All @@ -144,6 +174,10 @@ def test_post(self):
self.assertEqual(self.alert.message, 'updated alert')
self.assertEqual(self.alert.description.raw, 'updated description')
self.assertEqual(self.alert.active, False)
self.assertEqual(len(mail.outbox), 0)

# TODO: Test with email set true
# TODO: Test with email set true and email notifications disabled

def test_post_user(self):
"""Test POST by different user"""
Expand All @@ -169,6 +203,7 @@ class TestAdminAlertDeleteView(AdminalertsViewTestBase):

def setUp(self):
super().setUp()
self.alert = self._make_alert()
self.url = reverse(
'adminalerts:delete',
kwargs={'adminalert': self.alert.sodar_uuid},
Expand Down
98 changes: 96 additions & 2 deletions adminalerts/views.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
"""UI views for the adminalerts app"""

import logging

from django.conf import settings
from django.contrib import messages
from django.contrib.auth import get_user_model
from django.shortcuts import redirect
from django.urls import reverse
from django.views.generic import (
Expand All @@ -14,6 +17,8 @@
from django.views.generic.edit import ModelFormMixin

# Projectroles dependency
from projectroles.app_settings import AppSettingAPI
from projectroles.email import get_email_user, send_generic_mail
from projectroles.views import (
LoggedInPermissionMixin,
HTTPRefererMixin,
Expand All @@ -25,8 +30,27 @@
from adminalerts.models import AdminAlert


app_settings = AppSettingAPI()
logger = logging.getLogger(__name__)
User = get_user_model()


# Local constants
DEFAULT_PAGINATION = 15

EMAIL_SUBJECT = '{state} admin alert: {message}'
EMAIL_BODY = r'''
An admin alert has been {action}d by {issuer}:
{message}
'''.lstrip()
EMAIL_BODY_DESCRIPTION = r'''
Additional details:
----------------------------------------
{description}
----------------------------------------
'''


# Listing/details views --------------------------------------------------------

Expand Down Expand Up @@ -63,10 +87,80 @@ class AdminAlertDetailView(


class AdminAlertModifyMixin(ModelFormMixin):
"""Common modification methods for AdminAlert create/update views"""

@classmethod
def _get_email_recipients(cls, alert):
"""
Return list of email addresses for alert email recipients, excluding the
alert issuer.
"""
ret = []
users = User.objects.exclude(sodar_uuid=alert.user.sodar_uuid).order_by(
'email'
)
for u in users:
# TODO: Add opt-out app settings check here
if not u.email:
logger.warning('No email set for user: {}'.format(u.username))
continue
if u.email not in ret:
ret.append(u.email)
alt_emails = app_settings.get(
'projectroles', 'user_email_additional', user=u
)
if not alt_emails:
continue
for e in alt_emails.split(';'):
if e not in ret:
ret.append(e)
return ret

def _send_email(self, alert, action):
"""
Send email alerts to all users except for the alert issuer.
:param alert: AdminAlert object
:param action: "create" or "update" (string)
"""
subject = EMAIL_SUBJECT.format(
state='New' if action == 'create' else 'Updated',
message=alert.message,
)
body = EMAIL_BODY.format(
action=action,
issuer=get_email_user(alert.user),
message=alert.message,
)
if alert.description:
body += EMAIL_BODY_DESCRIPTION.format(
description=alert.description.raw
)
recipients = self._get_email_recipients(alert)
# NOTE: Recipients go under bcc
return send_generic_mail(
subject,
body,
[settings.EMAIL_SENDER],
self.request,
reply_to=None,
bcc=recipients,
)

def form_valid(self, form):
form.save()
form_action = 'update' if self.object else 'create'
messages.success(self.request, 'Alert {}d.'.format(form_action))
self.object = form.save()
email_count = 0
email_msg_suffix = ''
if settings.PROJECTROLES_SEND_EMAIL and self.object.active:
email_count = self._send_email(form.instance, form_action)
if email_count > 0:
email_msg_suffix = ', {} email{} sent'.format(
email_count, 's' if email_count != 1 else ''
)
messages.success(
self.request, 'Alert {}d{}.'.format(form_action, email_msg_suffix)
)
return redirect(reverse('adminalerts:list'))


Expand Down
2 changes: 2 additions & 0 deletions docs/source/major_changes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,13 @@ Release Highlights
- Add REST API versioning independent from repo/site versions
- Add timeline REST API
- Add optional pagination for REST API list views
- Add admin alert email sending to all users
- Add user opt-out settings for email notifications
- Add target site user UUID updating in remote sync
- Add remote sync of existing target local users
- Add remote sync of USER scope app settings
- Add checkusers management command
- Add CC and BCC field support in sending generic emails
- Rewrite sodarcache REST API views
- Rename AppSettingAPI "app_name" arguments to "plugin_name"
- Plugin API return data updates and deprecations
Expand Down
Loading

0 comments on commit ba5ce08

Please sign in to comment.