Skip to content

Commit

Permalink
fix email module crash with empty ADMINS setting (#1287)
Browse files Browse the repository at this point in the history
  • Loading branch information
mikkonie committed Sep 20, 2023
1 parent 74e96ea commit 71b1a38
Show file tree
Hide file tree
Showing 5 changed files with 28 additions and 10 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.rst
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ Fixed
- **Projectroles**
- User account update signals not triggered on login (#1274)
- Project list rendering failure with finder role (#1276)
- Crash in ``email`` module with empty ``ADMINS`` setting (#1287)
- **Timeline**
- Ajax view permission test issues (#1267)

Expand Down
5 changes: 1 addition & 4 deletions config/settings/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -140,10 +140,7 @@
# Provide ADMINS as: Name:email,Name:email
ADMINS = [
x.split(':')
for x in env.list(
'ADMINS',
default=['Admin:[email protected]', 'Admin2:[email protected]'],
)
for x in env.list('ADMINS', default=['Admin User:[email protected]'])
]

# See: https://docs.djangoproject.com/en/3.2/ref/settings/#managers
Expand Down
5 changes: 5 additions & 0 deletions config/settings/test.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,11 @@
# Note: This key only used for development and testing.
SECRET_KEY = env('DJANGO_SECRET_KEY', default='CHANGEME!!!')

# MANAGER CONFIGURATION
# ------------------------------------------------------------------------------
ADMINS = [('Admin User', '[email protected]')]
MANAGERS = ADMINS

# Mail settings
# ------------------------------------------------------------------------------
EMAIL_HOST = 'localhost'
Expand Down
14 changes: 8 additions & 6 deletions projectroles/email.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@
EMAIL_SENDER = settings.EMAIL_SENDER
DEBUG = settings.DEBUG
SITE_TITLE = settings.SITE_INSTANCE_TITLE
ADMIN_RECIPIENT = settings.ADMINS[0]

# Local constants
EMAIL_RE = re.compile(r'(^[a-zA-Z0-9_.+-]+@[a-zA-Z0-9-]+\.[a-zA-Z0-9-.]+$)')
Expand Down Expand Up @@ -285,11 +284,14 @@ def get_email_footer():
custom_footer = getattr(settings, 'PROJECTROLES_EMAIL_FOOTER', None)
if custom_footer:
return '\n' + custom_footer
return MESSAGE_FOOTER.format(
site_title=SITE_TITLE,
admin_name=ADMIN_RECIPIENT[0],
admin_email=ADMIN_RECIPIENT[1],
)
admin_recipient = settings.ADMINS[0] if settings.ADMINS else None
if admin_recipient:
return MESSAGE_FOOTER.format(
site_title=SITE_TITLE,
admin_name=admin_recipient[0],
admin_email=admin_recipient[1],
)
return ''


def get_invite_subject(project):
Expand Down
13 changes: 13 additions & 0 deletions projectroles/tests/test_email.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
"""Tests for email sending in the projectroles Django app"""

from django.conf import settings
from django.core import mail
from django.test import override_settings
from django.urls import reverse
Expand All @@ -14,6 +15,7 @@
send_project_create_mail,
get_email_user,
get_user_addr,
get_email_footer,
)
from projectroles.tests.test_models import (
ProjectMixin,
Expand Down Expand Up @@ -326,6 +328,17 @@ def test_generic_mail_multiple(self):
self.assertEqual(email_sent, 2)
self.assertEqual(len(mail.outbox), 2)

def test_get_email_footer(self):
"""Test get_email_footer() with default admin"""
footer = get_email_footer()
self.assertIn(settings.ADMINS[0][0], footer)
self.assertIn(settings.ADMINS[0][1], footer)

@override_settings(ADMINS=[])
def test_get_email_footer_no_admin(self):
"""Test get_email_footer() with empty admin list"""
self.assertEqual(get_email_footer(), '')

@override_settings(PROJECTROLES_EMAIL_HEADER=CUSTOM_HEADER)
def test_custom_header(self):
"""Test send_generic_mail() with custom header"""
Expand Down

0 comments on commit 71b1a38

Please sign in to comment.