diff --git a/course_discovery/apps/tagging/emails.py b/course_discovery/apps/tagging/emails.py index 0b473b8a4d..005d947c83 100644 --- a/course_discovery/apps/tagging/emails.py +++ b/course_discovery/apps/tagging/emails.py @@ -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})" @@ -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)}" ) diff --git a/course_discovery/apps/tagging/signals.py b/course_discovery/apps/tagging/signals.py index 1c7705f47a..381d2d4450 100644 --- a/course_discovery/apps/tagging/signals.py +++ b/course_discovery/apps/tagging/signals.py @@ -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 @@ -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) diff --git a/course_discovery/apps/tagging/tests/test_emails.py b/course_discovery/apps/tagging/tests/test_emails.py index ed8186e1f3..56e2c54a82 100644 --- a/course_discovery/apps/tagging/tests/test_emails.py +++ b/course_discovery/apps/tagging/tests/test_emails.py @@ -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) diff --git a/course_discovery/apps/tagging/tests/test_signals.py b/course_discovery/apps/tagging/tests/test_signals.py index e7b4103136..21072aba5c 100644 --- a/course_discovery/apps/tagging/tests/test_signals.py +++ b/course_discovery/apps/tagging/tests/test_signals.py @@ -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 = {"user1@example.com", "user2@example.com"} + 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):