Skip to content

Commit

Permalink
fix: ensure users are passed to tagging email util
Browse files Browse the repository at this point in the history
  • Loading branch information
DawoudSheraz authored and Ali-D-Akbar committed Feb 24, 2025
1 parent e0f63ad commit 2a95216
Show file tree
Hide file tree
Showing 4 changed files with 14 additions and 9 deletions.
8 changes: 6 additions & 2 deletions course_discovery/apps/tagging/emails.py
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,12 @@ def send_email_for_course_vertical_assignment(course, to_users):
"""
Sends an email to specified users requesting action to assign vertical and sub-vertical
for a given course, but only to those who have email notifications enabled.
Arguments:
course(Object): course model instance
to_users(List): list of user objects
"""
email_enabled_users = [user for user in to_users if is_email_notification_enabled(user)]
email_enabled_users = [user.email for user in to_users if is_email_notification_enabled(user)]
if not email_enabled_users:
logger.exception(
f"Failed to send vertical assignment email for course '{course.title}' (UUID: {course.uuid})"
Expand Down Expand Up @@ -70,5 +74,5 @@ def send_email_for_course_vertical_assignment(course, to_users):
except Exception as e: # pylint: disable=broad-except
logger.exception(
f"Failed to send vertical assignment email for course '{course.title}' (UUID: {course.uuid}) to "
f"recipients {', '.join(list(map(lambda user: user.email, email_enabled_users)))}. Error: {str(e)}"
f"recipients {', '.join(email_enabled_users)}. Error: {str(e)}"
)
9 changes: 5 additions & 4 deletions course_discovery/apps/tagging/signals.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
from django.conf import settings
from django.contrib.auth.models import Group
from django.contrib.auth import get_user_model
from django.db.models.signals import post_save
from django.dispatch import receiver

Expand All @@ -16,10 +16,11 @@ def notify_vertical_assignment(instance, created, **kwargs):
if instance.draft or not created:
return

User = get_user_model()
management_groups = getattr(settings, "VERTICALS_MANAGEMENT_GROUPS", [])

groups = Group.objects.filter(name__in=management_groups).exclude(user__isnull=True)
recipients = set(groups.values_list('user__email', flat=True))

recipients = list(
User.objects.prefetch_related('groups').filter(groups__name__in=management_groups).distinct()
)
if recipients:
send_email_for_course_vertical_assignment(instance, recipients)
2 changes: 1 addition & 1 deletion course_discovery/apps/tagging/tests/test_emails.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ def test_email_sent_to_recipients(self):
self.assertEqual(len(mail.outbox), 1)

email = mail.outbox[0]
self.assertEqual(email.to, [self.user1, self.user2])
self.assertEqual(email.to, [self.user1.email, self.user2.email])
expected_subject = f"Action Required: Assign Vertical and Sub-vertical for Course '{self.course.title}'"
self.assertEqual(email.subject, expected_subject)

Expand Down
4 changes: 2 additions & 2 deletions course_discovery/apps/tagging/tests/test_signals.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,11 +34,11 @@ def test_notify_vertical_assignment_email_sent(self, mock_send_email):
course_run.status = CourseRunStatus.Reviewed
course_run.save()

expected_recipients = {"[email protected]", "[email protected]"}
expected_recipients = [user2, user1]
mock_send_email.assert_called_once()
called_args = mock_send_email.call_args[0]
self.assertEqual(called_args[0].uuid, course.uuid)
self.assertEqual(called_args[1], expected_recipients)
self.assertListEqual(called_args[1], expected_recipients)

@mock.patch("course_discovery.apps.tagging.signals.send_email_for_course_vertical_assignment")
def test_notify_vertical_assignment_email_when_course_is_draft(self, mock_send_email):
Expand Down

0 comments on commit 2a95216

Please sign in to comment.