From 6972a6a562056cd9dcc1d53fb1fbdedcfe90a7fa Mon Sep 17 00:00:00 2001 From: Muhammad Adeel Tajamul <77053848+muhammadadeeltajamul@users.noreply.github.com> Date: Tue, 22 Apr 2025 12:44:04 +0500 Subject: [PATCH] feat: prevent sending bulk email to disabled users (#36549) --- lms/djangoapps/bulk_email/tasks.py | 20 +++++++++++++++++-- lms/djangoapps/bulk_email/tests/test_tasks.py | 12 +++++++++++ 2 files changed, 30 insertions(+), 2 deletions(-) diff --git a/lms/djangoapps/bulk_email/tasks.py b/lms/djangoapps/bulk_email/tasks.py index 98e79347b9..4f18b224fe 100644 --- a/lms/djangoapps/bulk_email/tasks.py +++ b/lms/djangoapps/bulk_email/tasks.py @@ -181,7 +181,7 @@ def perform_delegate_email_batches(entry_id, course_id, task_input, action_name) # inefficient OUTER JOIN query that would read the whole user table. combined_set = recipient_qsets[0].union(*recipient_qsets[1:]) if len(recipient_qsets) > 1 \ else recipient_qsets[0] - recipient_fields = ['profile__name', 'email', 'username'] + recipient_fields = ['profile__name', 'email', 'username', 'password'] log.info("Task %s: Preparing to queue subtasks for sending emails for course %s, email %s", task_id, course_id, email_id) @@ -350,6 +350,21 @@ def _filter_optouts_from_recipients(to_list, course_id): return to_list, num_optout +def _filter_disabled_users_from_recipients(to_list, course_key_str): + """ + Filters a user if its account is disabled + """ + user_list = [] + disabled_count = 0 + for user in to_list: + if user['password'].startswith('!'): + log.info(f"Bulk Email User is disabled {user['email']} in course {course_key_str}") + disabled_count += 1 + else: + user_list.append(user) + return user_list, disabled_count + + def _get_source_address(course_id, course_title, course_language, truncate=True): """ Calculates an email address to be used as the 'from-address' for sent emails. @@ -485,7 +500,8 @@ def _send_course_email(entry_id, email_id, to_list, global_email_context, subtas # in the Optout list. if subtask_status.get_retry_count() == 0: to_list, num_optout = _filter_optouts_from_recipients(to_list, course_email.course_id) - subtask_status.increment(skipped=num_optout) + to_list, num_disabled = _filter_disabled_users_from_recipients(to_list, str(course_email.course_id)) + subtask_status.increment(skipped=num_optout + num_disabled) course_title = global_email_context['course_title'] course_language = global_email_context['course_language'] diff --git a/lms/djangoapps/bulk_email/tests/test_tasks.py b/lms/djangoapps/bulk_email/tests/test_tasks.py index 96c48e0d9e..c4080b872b 100644 --- a/lms/djangoapps/bulk_email/tests/test_tasks.py +++ b/lms/djangoapps/bulk_email/tests/test_tasks.py @@ -479,3 +479,15 @@ class TestBulkEmailInstructorTask(InstructorTaskCourseTestCase): # we should expect only one email to be sent as the other learner is not eligible to receive the message # based on their last_login date self._test_run_with_task(send_bulk_course_email, 'emailed', 1, 1) + + def test_email_is_not_sent_to_disabled_user(self): + """ + Tests if disabled user are skipped when sending bulk email + """ + user_1 = self.create_student(username="user1", email="user1@example.com") + user_1.set_unusable_password() + user_1.save() + self.create_student(username="user2", email="user2@example.com") + with patch('lms.djangoapps.bulk_email.tasks.get_connection', autospec=True) as get_conn: + get_conn.return_value.send_messages.side_effect = cycle([None]) + self._test_run_with_task(send_bulk_course_email, 'emailed', 3, 2, skipped=1)