Skip to content

Commit

Permalink
add code to allow cc-ing fellow institutional admins
Browse files Browse the repository at this point in the history
  • Loading branch information
John Tordoff committed Dec 9, 2024
1 parent cd45df2 commit 0a90637
Show file tree
Hide file tree
Showing 6 changed files with 133 additions and 16 deletions.
15 changes: 12 additions & 3 deletions api/users/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -696,6 +696,11 @@ class UserMessageSerializer(JSONAPISerializer):
related_view_kwargs={'institution_id': '<institution._id>'},
help_text='The institution associated with this message.',
)
cc = ser.BooleanField(
required=False,
default=False,
help_text='If true, CCs all institutional admins.'
)

def get_absolute_url(self, obj: UserMessage) -> str:
return api_v2_url(
Expand Down Expand Up @@ -756,13 +761,17 @@ def create(self, validated_data: Dict[str, Any]) -> UserMessage:

if not recipient.is_affiliated_with_institution(institution):
raise Conflict(
{'user': 'Can not send to recipient that is not affiliated with the provided institution.'},
{'user': 'Cannot send to a recipient that is not affiliated with the provided institution.'},
)

return UserMessage.objects.create(
user_message = UserMessage.objects.create(
sender=sender,
recipient=recipient,
institution=institution,
message_type=MessageTypes.INSTITUTIONAL_REQUEST,
message_type=validated_data['message_type'],
message_text=validated_data['message_text'],
institutional_admins_CCed=validated_data['cc'],
)
user_message.save()
return user_message

73 changes: 71 additions & 2 deletions api_tests/users/views/test_user_message_institutional_access.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@
AuthUserFactory,
InstitutionFactory
)
from website.mails import USER_MESSAGE_INSTITUTIONAL_ACCESS_REQUEST


@pytest.mark.django_db
class TestUserMessageInstitutionalAccess:
Expand Down Expand Up @@ -37,6 +39,13 @@ def institutional_admin(self, institution):
institution.get_group('institutional_admins').user_set.add(admin_user)
return admin_user

@pytest.fixture()
def institutional_admin_cc(self, institution):
""" Just existe to be CCed"""
admin_user = AuthUserFactory()
institution.get_group('institutional_admins').user_set.add(admin_user)
return admin_user

@pytest.fixture()
def url_with_affiliation(self, user_with_affiliation):
return f'/{API_BASE}users/{user_with_affiliation._id}/messages/'
Expand Down Expand Up @@ -151,5 +160,65 @@ def test_admin_cannot_message_user_outside_institution(
"""
res = app.post_json_api(url_without_affiliation, payload, auth=institutional_admin.auth, expect_errors=True)
assert res.status_code == 409
assert 'Can not send to recipient that is not affiliated with the provided institution.'\
in res.json['errors'][0]['detail']['user']
assert ('Cannot send to a recipient that is not affiliated with the provided institution.'
in res.json['errors'][0]['detail']['user'])

@mock.patch('osf.models.user_message.send_mail')
def test_cc_institutional_admins(self, mock_send_mail, app, institutional_admin, institutional_admin_cc,
institution, url_with_affiliation, user_with_affiliation,
payload):
"""
Ensure CC option works as expected, sending messages to all institutional admins except the sender.
"""
mock_send_mail.return_value = mock.MagicMock()

# Enable CC in the payload
payload['data']['attributes']['cc'] = True

# Perform the API request
res = app.post_json_api(
url_with_affiliation,
payload,
auth=institutional_admin.auth,
)
assert res.status_code == 201

# Retrieve institutional admins excluding the sender
admin_users_other = institution.get_group('institutional_admins').user_set.all()
admin_usernames = list(admin_users_other.values_list('username', flat=True))

# Retrieve UserMessages created for the institution, validate only one.
UserMessage.objects.get()

mock_send_mail.assert_called_once_with(
to_addr=user_with_affiliation.username,
cc_addr=admin_usernames,
message_text='Requesting user access for collaboration',
mail=USER_MESSAGE_INSTITUTIONAL_ACCESS_REQUEST,
user=user_with_affiliation,
sender=institutional_admin,
recipient=user_with_affiliation,
institution=institution,
)

@mock.patch('osf.models.user_message.send_mail')
def test_cc_field_defaults_to_false(self, mock_send_mail, app, institutional_admin, url_with_affiliation, user_with_affiliation, institution, payload):
"""
Ensure the `cc` field defaults to `false` when not provided in the payload.
"""
res = app.post_json_api(url_with_affiliation, payload, auth=institutional_admin.auth)
assert res.status_code == 201

user_message = UserMessage.objects.get(sender=institutional_admin)
assert user_message.message_text == payload['data']['attributes']['message_text']
mock_send_mail.assert_called_once_with(
to_addr=user_with_affiliation.username,
cc_addr=None,
message_text='Requesting user access for collaboration',
mail=USER_MESSAGE_INSTITUTIONAL_ACCESS_REQUEST,
user=user_with_affiliation,
sender=institutional_admin,
recipient=user_with_affiliation,
institution=institution,
)

9 changes: 7 additions & 2 deletions framework/email/tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,8 @@ def send_email(
)


def _send_with_smtp(from_addr, to_addr, subject, message, ttls=True, login=True, username=None, password=None):
def _send_with_smtp(from_addr, to_addr, subject, message, ttls=True, login=True, username=None, password=None,
cc_addr=None):
username = username or settings.MAIL_USERNAME
password = password or settings.MAIL_PASSWORD

Expand All @@ -83,6 +84,10 @@ def _send_with_smtp(from_addr, to_addr, subject, message, ttls=True, login=True,
msg['Subject'] = subject
msg['From'] = from_addr
msg['To'] = to_addr
if cc_addr: # Add CC header if CC addresses exist
msg['Cc'] = ', '.join(cc_addr)

recipients = [to_addr] + (cc_addr or [])

s = smtplib.SMTP(settings.MAIL_SERVER)
s.ehlo()
Expand All @@ -93,7 +98,7 @@ def _send_with_smtp(from_addr, to_addr, subject, message, ttls=True, login=True,
s.login(username, password)
s.sendmail(
from_addr=from_addr,
to_addrs=[to_addr],
to_addrs=recipients,
msg=msg.as_string(),
)
s.quit()
Expand Down
3 changes: 2 additions & 1 deletion osf/migrations/0025_usermessage.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
# Generated by Django 4.2.13 on 2024-12-06 21:31
# Generated by Django 4.2.13 on 2024-12-09 14:50

from django.conf import settings
from django.db import migrations, models
Expand All @@ -23,6 +23,7 @@ class Migration(migrations.Migration):
('_id', models.CharField(db_index=True, default=osf.models.base.generate_object_id, max_length=24, unique=True)),
('message_text', models.TextField(help_text='The content of the message. The custom text of a formatted email.')),
('message_type', models.CharField(choices=[('institutional_request', 'INSTITUTIONAL_REQUEST')], help_text='The type of message being sent, as defined in MessageTypes.', max_length=50)),
('institutional_admins_CCed', models.BooleanField(default=False, help_text='The boolean value that indicates whether other institutional admins were CCed')),
('institution', models.ForeignKey(help_text='The institution associated with this message.', on_delete=django.db.models.deletion.CASCADE, to='osf.institution')),
('recipient', models.ForeignKey(help_text='The recipient of this message.', on_delete=django.db.models.deletion.CASCADE, related_name='received_user_messages', to=settings.AUTH_USER_MODEL)),
('sender', models.ForeignKey(help_text='The user who sent this message.', on_delete=django.db.models.deletion.CASCADE, related_name='sent_user_messages', to=settings.AUTH_USER_MODEL)),
Expand Down
26 changes: 24 additions & 2 deletions osf/models/user_message.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@
from django.db import models
from django.db.models.signals import post_save
from django.dispatch import receiver

from . import OSFUser
from .base import BaseModel, ObjectIDMixin
from website.mails import send_mail, USER_MESSAGE_INSTITUTIONAL_ACCESS_REQUEST

Expand Down Expand Up @@ -70,14 +72,34 @@ class UserMessage(BaseModel, ObjectIDMixin):
on_delete=models.CASCADE,
help_text='The institution associated with this message.'
)
institutional_admins_CCed = models.BooleanField(
default=False,
help_text = 'The boolean value that indicates whether other institutional admins were CCed',
)

def send_institution_request(self) -> None:
def send_institution_request(self):
"""
Sends an institutional access request email to the recipient of the message.
If cc is True, send copies to all institutional admins (excluding the sender).
"""
if self.institutional_admins_CCed:
cc_addrs = list(
self.institution.get_group(
'institutional_admins'
).user_set.values_list(
'username',
flat=True
)
)
self._send_institution_request(cc_addrs=cc_addrs)
else:
self._send_institution_request()

def _send_institution_request(self, cc_addrs=None):
send_mail(
to_addr=self.recipient.username,
mail=MessageTypes.get_template(MessageTypes.INSTITUTIONAL_REQUEST),
cc_addr=cc_addrs,
mail=MessageTypes.get_template(self.message_type),
user=self.recipient,
**{
'sender': self.sender,
Expand Down
23 changes: 17 additions & 6 deletions website/mails/mails.py
Original file line number Diff line number Diff line change
Expand Up @@ -76,17 +76,27 @@ def render_message(tpl_name, **context):


def send_mail(
to_addr, mail, from_addr=None, mailer=None, celery=True,
username=None, password=None, callback=None, attachment_name=None,
attachment_content=None, **context):
"""Send an email from the OSF.
Example: ::
to_addr,
mail,
from_addr=None,
cc_addr=None, # Add CC recipients
mailer=None,
celery=True,
username=None,
password=None,
callback=None,
attachment_name=None,
attachment_content=None,
**context):
"""
Send an email from the OSF.
Example:
from website import mails
mails.send_email('[email protected]', mails.TEST, name="Foo")
:param str to_addr: The recipient's email address
:param str cc_addr: The CC recipient's email address (or list of addresses)
:param Mail mail: The mail object
:param str mimetype: Either 'plain' or 'html'
:param function callback: celery task to execute after send_mail completes
Expand All @@ -112,6 +122,7 @@ def send_mail(
to_addr=to_addr,
subject=subject,
message=message,
cc_addr=cc_addr, # Include CC in kwargs
ttls=ttls,
login=login,
username=username,
Expand Down

0 comments on commit 0a90637

Please sign in to comment.